On Tue, Sep 26, 2023 at 7:19 PM Jens Axboe <axboe@xxxxxxxxx> wrote: > > On 9/26/23 9:48 AM, Amir Goldstein wrote: > > On Tue, Sep 26, 2023 at 6:31?PM Jens Axboe <axboe@xxxxxxxxx> wrote: > >> > >> On 9/25/23 4:43 AM, Amir Goldstein wrote: > >>> Jens, > >>> > >>> Are there any IOCB flags that overlayfs (or backing_aio) need > >>> to set or clear, besides IOCB_DIO_CALLER_COMP, that > >>> would prevent calling completion from interrupt context? > >> > >> There are a few flags that may get set (like WAITQ or ALLOC_CACHE), but > >> I think that should all work ok as-is as the former is just state in > >> that iocb and that is persistent (and only for the read path), and > >> ALLOC_CACHE should just be propagated. > >> > >>> Or is the proper way to deal with this is to defer completion > >>> to workqueue in the common backing_aio helpers that > >>> I am re-factoring from overlayfs? > >> > >> No, deferring to a workqueue would defeat the purpose of the flag, which > >> is telling you that the caller will ensure that the end_io callback will > >> happen from task context and need not be deferred to a workqueue. I can > >> take a peek at how to wire it up properly for overlayfs, have some > >> travel coming up in a few days. > >> > > > > No worries, this is not urgent. > > I queued a change to overlayfs to take a spin lock on completion > > for the 6.7 merge window, so if I can get a ACK/NACK until then > > It would be nice. > > > > https://lore.kernel.org/linux-unionfs/20230912173653.3317828-2-amir73il@xxxxxxxxx/ > > That's not going to work for ovl_copyattr(), as ->ki_complete() may very > well be called from interrupt context in general. > > >>> IIUC, that could also help overlayfs support > >>> IOCB_DIO_CALLER_COMP? > >>> > >>> Is my understanding correct? > >> > >> If you peek at fs.h and find the CALLER_COMP references, it'll tell you > >> a bit about how it works. This is new with the 6.6-rc kernel, there's a > >> series of patches from me that went in through the iomap tree that > >> hooked that up. Those commits have an explanation as well. > >> > > > > Sorry, I think my question wasn't clear. > > I wasn't asking specifically about CALLER_COMP. > > > > Zhang Tianci commented in review (above) that I am not allowed > > to take the inode spinlock in the ovl io completion context, because > > it may be called from interrupt. > > That is correct, the inode spinlock is not IRQ safe. > > > I wasn't sure if his statement was correct, so this is what I am > > asking - whether overlayfs can set any IOCB flags that will force > > the completion to be called from task context - this is kind of the > > opposite of CALLER_COMP. > > > > Let me know if I wasn't able to explain myself. > > I am not that fluent in aio jargon. > > Ah gotcha. I don't think that'd really work for your case as you don't > need to propagate it, you can just punt your completion handling to a > context that is sane for you, like a workqueue. That is provided that > you don't need any task context there, which presumably you don't since > eg copyattr is already called from IRQ context. > > From that context you could then grab the inode lock. > Yes, that's what I thought. Thanks for confirming! Amir.
