Re: [PATCH] ovl: punt write aio completion to workqueue

[Matthew Wilcox <willy@infradead.org>]

On Thu, Sep 28, 2023 at 12:15:00PM +0300, Amir Goldstein wrote:
> On Thu, Sep 28, 2023 at 10:08 AM Jens Axboe <axboe@kernel.dk> wrote:
> > On 9/28/23 12:46 AM, Amir Goldstein wrote:
> > >               ret = -ENOMEM;
> > >               aio_req = kmem_cache_zalloc(ovl_aio_request_cachep, GFP_KERNEL);
> > >               if (!aio_req)
> >
> > Unrelated to this patch, but is this safe? You're allocating an aio_req
> > from within the ->write_iter() handler, yet it's GFP_KERNEL? Seems like
> > that should at least be GFP_NOFS, no?
> 
> I could be wrong, but since overlayfs does not have any page cache
> of its own, I don't think memory reclaim poses a risk.

Use the scoped APIs, people!  GFP_NOFS needs to die.  If your filesystem
cannot tolerate being reentered, call memalloc_nofs_save() / restore()
when it can tolerate being reentered.

> > That aside, punting to a workqueue is a very heavy handed solution to
> > the problem. Maybe it's the only one you have, didn't look too closely
> > at it, but it's definitely not going to increase your performance...
> 
> I bet it won't... but I need to worry about correctness.
> 
> What I would like to know, and that is something that I tried
> to ask you in the Link: discussion, but perhaps I wasn't clear -
> Are there any IOCB flags that the completion caller may set,
> that will hint the submitter that completion is not from interrupt
> context and that punting to workqueue is not needed?

I'd really like page cache write completions to not be handled in
the interrupt handler.  Then we could make the i_pages lock not an
interrupt-disabling lock any more.  I think that'd best be handled in a
workqueue too, but maybe there's a better solution nowadays.