On 02/07/2018 09:05 AM, William Allen Simpson wrote:
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.
Already fixed. I did my research after sending the last mail.
I'm assuming this code will be committed sometime in the near future.
I wasn't planning on committing this as is, but rather waiting until it
was more complete.
"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).
It all can, but I prefer simple in parameters to be passed as
parameters. I don't like functions that take only a single giant
struct, especially with optional entries. I was not planning on putting
anything in it that was not needed in the callback (and the contents of
this struct are still evolving as I write code).
And some of this will need to be done to remove op_ctx dependencies.
This won't convince me. I'm a fan of op_ctx.
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.
Daniel
------------------------------------------------------------------------------
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
