On Tue, Nov 21, 2023 at 04:00:32PM -0500, Josef Bacik wrote:
> On Tue, Nov 21, 2023 at 03:25:51PM +0200, Amir Goldstein wrote:
> > We want to move kiocb_start_write() into vfs_iocb_iter_write(), after
> > the permission hook, but leave kiocb_end_write() in the write completion
> > handler of the callers of vfs_iocb_iter_write().
> >
> > After this change, there will be no way of knowing in completion handler,
> > if write has failed before or after calling kiocb_start_write().
> >
> > Add a flag IOCB_WRITE_STARTED, which is set and cleared internally by
> > kiocb_{start,end}_write(), so that kiocb_end_write() could be called for
> > cleanup of async write, whether it was successful or whether it failed
> > before or after calling kiocb_start_write().
> >
> > This flag must not be copied by stacked filesystems (e.g. overlayfs)
> > that clone the iocb to another iocb for io request on a backing file.
> >
> > Link: https://lore.kernel.org/r/CAOQ4uxihfJJRxxUhAmOwtD97Lg8PL8RgXw88rH1UfEeP8AtP+w@xxxxxxxxxxxxxx/
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> This is only a problem for cachefiles and overlayfs, and really just for
> cachefiles because of the error handling thing.
>
> What if instead we made vfs_iocb_iter_write() call kiocb_end_write() in the ret
> != EIOCBQUEUED case, that way it is in charge of the start and the end, and the
> only case where the file system has to worry about is the actual io completion
> path when the kiocb is completed.
>
> The result is something like what I've pasted below, completely uncompiled and
> untested. Thanks,
I like this a lot better than an internal flag that allows
unbalanced start/end calls to proliferate. I find it much easier to
read the code, determine the correct cleanup is being done and
maintain the code in future when calls need to be properly
paired....
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
