[EMAIL PROTECTED] wrote on Fri, 08 Feb 2008 18:27 -0600:
> The attached patch tries to cleanup some of our server request code, so
> that adding a new server request+state machine doesn't mean searching for
> all the places where we perform a switch/case on the server op enum. It
> was motivated by the need for a more generic prelude state machine and
> request scheduler, but its been something that I've been itching to knock
> out for a while now. I think there are still improvements that can be made
> here, especially to the encoding/decoding and request scheduler, but this
> gives me the functionality that I needed, so I decided its a good place to
> stop.
This looks like a very worthwhile cleanup. The patch includes maybe
3 or 4 independent changes, but they are all tied to this one theme.
They are: mode change, ref count moving, rename parent_handle some
places, server req params. Even though I find it easier to digest
small bits, it's all quite nice.
Some minor comments.
>+PINT_GET_OBJECT_REF_DEFINE(flush);
Instead of defining a static inline function for each one of these
(which won't be inlined of course), can you just add a single
generic PINT_get_object_ref_generic() that they all can use? Then
we only see special functions for the oddball cases like mkdir, and
have less code in the binary. Maybe the same idea applies for
PINT_server_req_access_io and _small_io; could be a generic function
somewhere.
>+ SERVERBINOBJS := $(patsubst %.c,%-server.o, $(filter %.c,$(SERVERBINSRC)))
>+ SERVERBINDEPENDS := $(patsubst %.c,%.d, $(filter %.c,$(SERVERBINSRC)))
Probably want to add these to the clean and deps generation lists
too, respectively. It is a bit clunky how we do this. A nice
makefile cleanup for somebody in the future perhaps.
I bet that the new file pvfs2-server-req.c is as brilliant as the
rest of this patch, wherever it may be.
-- Pete
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
