[Pvfs2-developers] Hint patch

[Sam Lang]

Hi Julian,

Thanks for the patch!  I had a chance to look at it and have some  
comments.  Overall I think its great.  It provides functionality  
we've been needing for a while now -- the hints may become a good way  
to 'stage' parameters that we can make more permanent later on.  I  
would like to see this added to the tree quickly, and so while my  
comments may seem nit-picky and anal, I want to make sure it fits in  
well with the rest of the code.

I think we may need to copy the hint structure within the PVFS_isys_*  
calls.  We can't guarantee that the caller won't destroy his hints  
structure after the non-blocking call completes.

I would like to see the encoding and decoding for the  
'transfer_to_server' hints be done in binary instead of a string.   
I'm thinking of something along the lines where a static struct would  
define encode/decode functions for a request-id hint, that would  
encode it into a 32bit value, instead of a string.  The decoding  
could also just be to a 32bit int.  This would require an internal  
representation of a hint, different from the exposed string form, but  
would save us shipping a bunch of bytes just because the integer  
value is large.

Related to that, the hint struct in pvfs2-hint.h doesn't have to be  
exposed.  Since we're adding hints to the hint list with functions,  
we could keep the actual fields of the struct internal.  This would  
allow us to convert a known hint immediately to some internal form.

Your calculating the size of the hints for each request encode, which  
goes against the pre-calculated request size stuff.  Maybe it makes  
sense at this point to get rid of that pre-calculated request sizes  
code, but since its there right now, we should probably follow that  
model.  That means the max_size_array needs to account for the  
maximum number of hints possible, and in lebf_initialize you'll have  
to fill in the hints struct with the maximum possible hints so the  
size is calculated correctly.  Alternatively, if the hint struct is  
internal, it could keep track of the number of hints with a separate  
field, and you could just set that to the max for the initial max  
size calculation.

The PINT_hint_encode/decode calls and PINT_hint_add_environment_hints  
should be in an internal header.  PINT_hint_calc_size as well.

pvfs2_hint_type should be PVFS_hint_type.

Separate from the patch -- the way MPI hints are converted to PVFS  
hints seems kludgy.  I would rather move the PVFS_hint_get_type call  
into the PVFS_add_hint call.  If PVFS doesn't recognize the hint, the  
PVFS_add_hint call can return an error -PVFS_ENOENT or something,  
which the MPI code would probably just ignore.  Maybe the ROMIO folks  
have better ideas for that though.

Thanks again,

-sam

On Jun 19, 2007, at 4:11 PM, Julian Martin Kunkel wrote:

> Hi,
> I attached a patch which implements the hint mechanism for PVFS2  
> the way we
> use it right now.
> For people not reading all the messages from the mailing list: We  
> had a
> discussion about hints a couple of month ago. A hint allows to  
> transfer a
> string through the layers to the server and down to Flow/Trove and  
> parly BMI
> (where it makes sense).
>
> I had to stripe quite some functionality to reduce the patch to its  
> core just
> providing the hints (and a handful of really small modifications)  
> and to
> merge the functionality to the recent CVS version (with concurrent
> statemachines).
> Therefore, I hope I did not break something, basic operations
> seem to work. Most the time the patch is really straightforward.
> I left the "Request ID" hint and the kernel code modifications to  
> transfer it.
> However, the patch contains a handful redundant variables....
> Well, I sent it even if it is not perfect to reopen a potential  
> discussion
> about the future (and hopefully final) implementation ;-)
>
> Best regards,
> Julian
> <hint.patch>
> _______________________________________________
> Pvfs2-developers mailing list
> Pvfs2-developers at beowulf-underground.org
> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers