Hi Julian,
I went ahead and made the hints changes I described in the previous
email. Let me know what you think.
-sam
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
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
