[Pvfs2-developers] Hint patch

[Sam Lang]

Woops.  Forgot the attachment.

-sam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: hints.patch
Type: application/octet-stream
Size: 501778 bytes
Desc: not available
Url : http://www.beowulf-underground.org/pipermail/pvfs2-developers/attachments/20070914/dcb83556/hints-0001.obj
-------------- next part --------------

On Jun 26, 2007, at 10:27 AM, Sam Lang wrote:

>
> 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
>