All of lore.kernel.org
 help / color / mirror / Atom feed
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?

      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.