On Mon, Nov 20, 2023 at 7:05 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Mon, Nov 20, 2023 at 6:56 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote: > > > > On Mon, Nov 20, 2023 at 12:14:24PM +0200, Amir Goldstein wrote: > > > We want to move kiocb_start_write() into vfs_iocb_iter_write(), but > > > first we need to move it passed cachefiles_inject_write_error() and > > > prevent calling kiocb_end_write() if error was injected. > > > > > > We set the IOCB_WRITE flag after cachefiles_inject_write_error() > > > and use it as indication that kiocb_start_write() was called in the > > > cleanup/completion handler. > > > > > > Link: https://lore.kernel.org/r/CAOQ4uxihfJJRxxUhAmOwtD97Lg8PL8RgXw88rH1UfEeP8AtP+w@xxxxxxxxxxxxxx/ > > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > > > Sorry Amir I meant to respond on Saturday but I got busy with other things. > > > > I was thinking instead, for your series, you could do something like > > > > ret = cachefiles_inject_write_error(); > > if (ret) { > > /* Start kiocb so the error handling is done below. */ > > kiocb_start_write(&ki->iocb); > > } else { > > ret = vfs_iocb_iter_write(file, &ki->iocb, iter); > > } > > > > which seems a bit cleaner than messing with the flags everywhere. > > I think that both our options are pretty ugly ;-) > > I'll use whatever the maintainers of cachefiles and vfs prefer. Looking closer, that's a self NACK. My patch moves kiocb_start_write() to after the permission hook, so calling vfs_iocb_iter_write() is no guarantee that kiocb_start_write() was called... I will send a new patch that does not change cachefiles. Thanks, Amir.
