From: "Zhou, Yun" <yun.zhou@windriver.com>
To: David Howells <dhowells@redhat.com>,
Christian Brauner <christian@brauner.io>
Cc: Paulo Alcantara <pc@manguebit.org>,
Christoph Hellwig <hch@infradead.org>,
netfs@lists.linux.dev, linux-afs@lists.infradead.org,
linux-cifs@vger.kernel.org, ceph-devel@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
syzbot+0da43efa72f88bd3a8af@syzkaller.appspotmail.com,
Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH 4/4] netfs: Fix folio_queue ENOMEM in writeback by adding a mempool
Date: Tue, 28 Jul 2026 16:36:30 +0800 [thread overview]
Message-ID: <51275778-eec9-4c87-8ee6-97e4c4e6d884@windriver.com> (raw)
In-Reply-To: <20260727130716.1099906-5-dhowells@redhat.com>
On 7/27/2026 9:07 PM, David Howells wrote:
> Fix the handling of folio_queue allocation failure in writeback by adding a
> mempool and passing in gfp_t flags to the rolling buffer functions that
> allocate memory, using the mempool if gfp != GFP_KERNEL.
>
> This is then extended upwards and the gfp to be used for a request is stored
> in the netfs_io_request struct and is then used for both requests and
> subrequests, eliminating the sleeping loops there.
>
Have you considered splitting this into multiple patches based on
different functionalities?
> @@ -200,12 +205,14 @@ struct netfs_io_subrequest *netfs_alloc_subrequest(struct netfs_io_request *rreq
> mempool_t *mempool = rreq->netfs_ops->subrequest_pool ?: &netfs_subrequest_pool;
> struct kmem_cache *cache = mempool->pool_data;
>
> - for (;;) {
> - subreq = mempool_alloc(rreq->netfs_ops->subrequest_pool ?: &netfs_subrequest_pool,
> - GFP_KERNEL);
> - if (subreq)
> - break;
> - msleep(10);
> + if (rreq->gfp == GFP_KERNEL) {
Direct equality checks are not reliable; using !(rreq->gfp & GFP_NOFS)
for the check is more robust.
> + subreq = mempool->alloc(rreq->gfp, mempool->pool_data);
> + if (!subreq)
> + return ERR_PTR(-ENOMEM);
Since some callers check for success by testing !NULL, we cannot return
ENOMEM.
221 subreq = netfs_alloc_subrequest(rreq);
222 if (!subreq) {
223 ret = -ENOMEM;
224 break;
> + } else {
> + subreq = mempool_alloc(mempool, rreq->gfp);
> + if (!subreq)
> + return NULL;
Redundant check. If it is added to mirror the GFP_KERNEL path above,
returning the same error code would be better for the caller to handle.
Or perhaps this is an intentional design?
prev parent reply other threads:[~2026-07-28 8:37 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-27 13:07 [PATCH 0/4] netfs: Miscellaneous fixes David Howells
2026-07-27 13:07 ` [PATCH 1/4] netfs: clear PG_private_2 on copy-to-cache append failure David Howells
2026-07-27 13:07 ` [PATCH 2/4] netfs: handle single writeback rolling buffer allocation failure David Howells
2026-07-27 13:07 ` [PATCH 3/4] netfs: release readahead folios on iterator preparation failure David Howells
2026-07-27 13:07 ` [PATCH 4/4] netfs: Fix folio_queue ENOMEM in writeback by adding a mempool David Howells
2026-07-28 8:36 ` Zhou, Yun [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51275778-eec9-4c87-8ee6-97e4c4e6d884@windriver.com \
--to=yun.zhou@windriver.com \
--cc=ceph-devel@vger.kernel.org \
--cc=christian@brauner.io \
--cc=dhowells@redhat.com \
--cc=hch@infradead.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netfs@lists.linux.dev \
--cc=pc@manguebit.org \
--cc=syzbot+0da43efa72f88bd3a8af@syzkaller.appspotmail.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.