On 2/6/18 10:40 AM, Daniel Gryniewicz wrote:
On 02/06/2018 10:26 AM, William Allen Simpson wrote:
On 2/6/18 8:25 AM, Daniel Gryniewicz wrote:
Hi, all.

I've worked up a sample API for async/vector for FSAL ops.  The example op is read(), and I've "implemented" it for all FSALs, so that I can verify that it does, in fact, work for some definition of work.

I'm a bit surprised it works, as the alloca needs the sizeof struct PLUS the
sizeof iov * iov_count.  Right now it's writing off the end.

I believe the empty array construct includes a size of 1.  If I'm wrong, then 
it's an easy fix (and this code will go away anyway, and never be committed).

No, it's zero.  Yes, an easy fix.

I'm assuming this code will be committed sometime in the near future.


"asynchronous: has an 'h' in it.

"it's" means "it is".  Most places should be "its".

To be async, need to move the status return into the arg struct, and pass
NULL for the caller's parameter at the top level.

Return is it's own argument to the callback.

I'd prefer to have the new struct contain all the common arguments.

Every level needs to be able to set the status, so putting the result in
the struct makes the code cleaner than copying stuff in every wrapper.


Why not move the other arguments into the struct?
   * bypass
   * state
   * offset

Because those are pure in arguments, and were unchanged, so minimal code 
changes.  The iov was put into the arg to avoid multiple mallocs, and I put 
iov_count with iov.  The rest are out arguments.

Obviously, all [in] arguments can be in the struct.  Set and forget once
at the top....

Even [out] pointer arguments can be in the struct.

Removing long parameter lists makes the code cleaner (and faster).

And some of this will need to be done to remove op_ctx dependencies.


Also it will be the same for write, so we can just name it
struct fsal_cb_arg -- and the function typedef fsal_cb to match.

It may be.  I didn't look at write, this is a proof-of-concept for read, and 
not in any way intended to be final.

Yeah, as we talked earlier, I was looking at the bigger picture.  This is a
nice clean proof-of-concept.  I like it.  Now I'm talking details.



Why not get rid of fsal_read2(), and call the function directly in
the 3 places it's used?

I'm considering it.  That was a good point to break this particular 
proof-of-concept, but much must change for async to be plumbed all the way back 
to the top of the protocols.

Yes, again I'm forward looking.  If we do it now, then we don't have to
undo anything later.  Makes the patches easier to understand.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Reply via email to