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.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
