On Sat, Oct 29, 2016 at 05:12:30PM +0100, Al Viro wrote:

Eww...  IOW, as soon as we'd submitted an async iocb, nobody can so much as
look at struct file *or* iocb, right?  Or underlying inode, or any fs-private
data structures attached to it...

Yeah.

I certainly agree that it's a bug, but IMO you are just papering over it.
Just look at e.g.
        written = mapping->a_ops->direct_IO(iocb, &data);

        /*
         * Finally, try again to invalidate clean pages which might have been
         * cached by non-direct readahead, or faulted in by get_user_pages()
         * if the source of the write was an mmap'ed region of the file
         * we're writing.  Either one is a pretty crazy thing to do,
         * so we don't support it 100%.  If this invalidation
         * fails, tough, the write still worked...
         */
        if (mapping->nrpages) {
                invalidate_inode_pages2_range(mapping,
                                              pos >> PAGE_SHIFT, end);
        }
in generic_file_direct_write() - who said that mapping doesn't point into
freed memory by that point?

True, somewhat unlike due to inode caching, but for sure possible
and something that needs to be deal with.

Why do we play that kind of insane games, anyway?  Why not e.g. refcount
the (async) iocb and have kiocb_free() drop the reference, with io_submit_one()
holding one across the call of aio_run_iocb()?  Cacheline bouncing issues?
Anything more subtle?

There shouldn't really be a need to refcount the iocb itself - nothing
worth looking at.  The one we consider was struct file, and I didn't
like it because of the cacheline bouncing if we decrement if from both
the submitter and completion context.  But it starts to sounds like
the sane version more and more.