Linux block layer
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-block@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	Vlastimil Babka <vbabka@kernel.org>, Harry Yoo <harry@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Hao Li <hao.li@linux.dev>, Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	Roman Gushchin <roman.gushchin@linux.dev>
Subject: Re: [PATCH 2/3] mm: support fallible mempool_alloc_bulk()
Date: Tue, 11 Aug 2026 13:13:37 -0700	[thread overview]
Message-ID: <anuCcb-Cma6JOAb0@infradead.org> (raw)
In-Reply-To: <20260810182215.GA270444@google.com>

On Mon, Aug 10, 2026 at 06:22:15PM +0000, Eric Biggers wrote:
> On Mon, Aug 10, 2026 at 10:21:15AM -0700, Christoph Hellwig wrote:
> > On Mon, Aug 10, 2026 at 09:14:04AM -0700, Eric Biggers wrote:
> > > On Mon, Aug 10, 2026 at 09:00:11AM -0700, Christoph Hellwig wrote:
> > > > On Thu, Aug 06, 2026 at 03:10:30PM -0700, Eric Biggers wrote:
> > > > > To fix a deadlock, blk-crypto-fallback needs to be able to make fallible
> > > > > mempool_alloc_bulk() allocations.
> > > > 
> > > > It doesn't.  fallible mempool allocations are a concept that doesn't
> > > > make much sense.  Please just go straight to the backing page allocator
> > > > instead for callers that do not need the mempool guarantees.
> > > 
> > > It does make sense.  When alloc_pages_bulk() doesn't completely succeed,
> > > there still might be pages available in the mempool.
> > 
> > But they should not go to a caller that does not need the mempool.
> 
> The caller does need the mempool.

If it does not specify a blocking GFP_* mask, it by definition does not.

> Now, as I explained in this patchset, whether this code can wait forever
> for the mempool actually depends on whether it's a recursive bio
> submission or not.  If it is, then it cannot wait, but ultimately it
> does still need to use the mempool to get a guaranteed allocation.
> 
> Falling back to the rescuer kthread (which can wait on the mempool)
> solves that.  But before taking that slow fallback, it's much more
> efficient to check the mempool directly first, since pages may be
> available there (and in fact it's fairly likely that they will be, since
> regular allocations are always used first).  The whole point of the
> mempool is that it can be used when the regular allocation fails.

What is actually efficient is do skip the mempool entirely from the
original submission context and do a non-mempool GFP_NOIO allocation,
and only in the extremely unlikely case that this fails fall back to
the rescruer thread.

> This is also the only caller of mempool_alloc_bulk().  So I'm kind of
> confused why it would not be allowed to implement the behavior that is
> desired here, especially when the non-bulk API offers it already.

Because passing GFP_ flags with all their warts is a bad idea where
we can avoid, as is using up mempool resources where not needed, and as
is having a possible failure path from a function that can only happen
for non-standard flags.  (having a return value at all is also stupid,
but that is forced on us by the braindead alloc_hooks).


  reply	other threads:[~2026-08-11 20:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 22:10 [PATCH 0/3] Fix a deadlock in blk-crypto-fallback Eric Biggers
2026-08-06 22:10 ` [PATCH 1/3] mm: make mempool_alloc_from_pool() return bool Eric Biggers
2026-08-10 15:59   ` Christoph Hellwig
2026-08-06 22:10 ` [PATCH 2/3] mm: support fallible mempool_alloc_bulk() Eric Biggers
2026-08-10 16:00   ` Christoph Hellwig
2026-08-10 16:14     ` Eric Biggers
2026-08-10 17:21       ` Christoph Hellwig
2026-08-10 18:22         ` Eric Biggers
2026-08-11 20:13           ` Christoph Hellwig [this message]
2026-08-11 21:03             ` Eric Biggers
2026-08-06 22:10 ` [PATCH 3/3] blk-crypto-fallback: Fix deadlock when encrypting large bio in dm layer Eric Biggers

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=anuCcb-Cma6JOAb0@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=cl@gentwo.org \
    --cc=ebiggers@kernel.org \
    --cc=hao.li@linux.dev \
    --cc=harry@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=vbabka@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox