Messages in this thread | | | Date | Mon, 25 Sep 2023 03:35:43 -0600 | Subject | Re: [PATCH] ovl: disable IOCB_DIO_CALLER_COMP | From | Jens Axboe <> |
| |
On 9/25/23 3:18 AM, Amir Goldstein wrote: > On Mon, Sep 25, 2023 at 9:21?AM Jens Axboe <axboe@kernel.dk> 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@redhat.com> >> Link: https://lore.kernel.org/io-uring/20230924142754.ejwsjen5pvyc32l4@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/ >> Fixes: 8c052fb3002e ("iomap: support IOCB_DIO_CALLER_COMP") >> Signed-off-by: Jens Axboe <axboe@kernel.dk> >> > > Thanks for fixing this Jens! > If you or Christian want to send this fix to Linus, you have my ACK.
No problem - and thanks, maybe Christian can pick this one up? I tentatively queued it up here just so I don't forget it:
https://git.kernel.dk/cgit/linux/log/?h=ovl-kiocb
> 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.
Ah that's great! Yeah it's a bit hidden in there if you don't know about it, and I did grep today when writing this patch to ensure we didn't have any others like it. So I think we're good for now, at least.
> 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).
I'm guessing it's because you don't have liburing installed on the test box, then fsx etc don't get built with io_uring support in xfstests.
> 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.
Will add it to the mix! Thanks for the details.
-- Jens Axboe
| |