Re: RFC: [PATCH] staging/lustre/llite: fix O_TMPFILE/O_LOV_DELAY_CREATE conflict

[Al Viro <viro@ZenIV.linux.org.uk>]

On Mon, Feb 10, 2014 at 10:10:30PM +0000, Al Viro wrote:

> Ugh...  Sorry, I misread that code.  Why the devil do you have the
> pos argument passed to lustre_generic_file_{read,write}() by address,
> when both proceed to dereference it and pass the value on?

Egads.  Correct me if I'm wrong, please, but it looks like you have

1) ->aio_write() stash its arguments in an off-stack struct and pass it
and &iocb->ki_pos to

2) ll_file_io_generic() which calls

3) cl_io_rw_init() which copied ki_pos value passed to it into
io->u.ci_rw.crw_pos (in another off-stack struct) and calls

4) cl_io_init() which calls

5) cl_io_init0() which possibly calls a bunch of instances of ->coo_io_init(),
which may or may not return 0; I _hope_ that in this case that's what'll
happen and we get back to ll_file_io_generic(), where

3') we stash iocb and iov/iovlen into the third off-stack structure (cio)
and call

4') cl_io_loop() where we (in a loop) call cl_io_iter_init(), cl_io_lock() and

5') cl_io_start() which a calls a bunch (how large in that case?) of

6') ->cio_start() instances.  Hopefully that'll be vvp_io_write_start()
which will pass the value of io->u.ci_rw.crw_pos (picked via an overlapping
field of union) to generic_file_aio_write().  Which, BTW, updates ->ki_pos.
Then we return into cl_io_loop(), where

4'') we call cl_io_end(), cl_io_unlock() and

5'') cl_io_rw_advance() which increments ....crw_pos, hopefully in sync with
what we have in iocb->ki_pos.  And calls a bunch of methods.  And return
into cl_io_loop(), where

4''') we call cl_io_iter_fini() (_another_ pile of methods called) and possibly
repeat everything from (4') on (apparently only if nothing had been done so
far).  Eventually we return into ll_file_io_generic() and there

3''') we copy ....crw_pos into iocb->ki_pos.  WTF do we need that?  Hadn't
generic_file_aio_write() been good enough?

Is that correct?  I have *not* traced it into all methods that might've
been called in process - stuff called from cl_io_loop() is chock-full of
those.  Have I missed anything relevant wrt file position handling in
there?

You guys really should be forced to hand-draw a call graph for that thing.
Both as a punishment and a deterrent...