On Mon, Sep 25, 2023 at 9:21 AM Jens Axboe <axboe@xxxxxxxxx> wrote: > > overlayfs copies the kiocb flags when it sets up a new kiocb to handle > a write, but it doesn't properly support dealing with the deferred > caller completions of the kiocb. This means it doesn't get the final > write completion value, and hence will complete the write with '0' as > the result. > > We could support the caller completions in overlayfs, but for now let's > just disable them in the generated write kiocb. > > Reported-by: Zorro Lang <zlang@xxxxxxxxxx> > Link: https://lore.kernel.org/io-uring/20230924142754.ejwsjen5pvyc32l4@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > Fixes: 8c052fb3002e ("iomap: support IOCB_DIO_CALLER_COMP") > Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> > Thanks for fixing this Jens! If you or Christian want to send this fix to Linus, you have my ACK. On the bright side, I am glad that you are aware of the overlayfs "kiocb_clone" use case, which delegates/forwards the io request to another file in another fs. I have already posted an RFC [1] for moving this functionality to common vfs code. My main goal was to expose it to other filesystem (fuse), but a very desired side effect is that this functionality gets more vfs reviewer eyes and then the chances of catching a regression like this one during review of vfs changes hopefully increases. As for test coverage, I need to check why my tests did not catch this - I suspect fsx may not have been rebuilt with io_uring support, but not sure (not near workstation atm). If you would like to add overlayfs to your test coverage, as Zorro explained, it is as simple as running ./check -overlay with your existing fstests config. ./check -overlay is a relatively faster test run because many of the tests do _notrun on overlayfs. I don't have to tell you that io_uring code will end up running on overlayfs in many container workloads, so it is not a niche setup. Thanks, Amir. [1] https://lore.kernel.org/linux-fsdevel/20230912185408.3343163-1-amir73il@xxxxxxxxx/ > --- > > diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c > index 4193633c4c7a..693971d20280 100644 > --- a/fs/overlayfs/file.c > +++ b/fs/overlayfs/file.c > @@ -391,6 +391,12 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) > if (!ovl_should_sync(OVL_FS(inode->i_sb))) > ifl &= ~(IOCB_DSYNC | IOCB_SYNC); > > + /* > + * Overlayfs doesn't support deferred completions, don't copy > + * this property in case it is set by the issuer. > + */ > + ifl &= ~IOCB_DIO_CALLER_COMP; > + > old_cred = ovl_override_creds(file_inode(file)->i_sb); > if (is_sync_kiocb(iocb)) { > file_start_write(real.file); > > -- > Jens Axboe > >
