Re: [PATCH 3/9] mempool: add mempool_{alloc,free}_bulk

Christoph Hellwig
2025-11-06T14:48:51Z

On Thu, Nov 06, 2025 at 03:27:35PM +0100, Vlastimil Babka wrote:
> >> Would it be enough to do this failure injection attempt once and not in
> >> every iteration?
> >
> > Well, that would only test failure handling for the first element. Or
> > you mean don't call it again if called once?
>
> I mean since this is (due to the semantics of mempools) not really causing a
> failure to the caller (unlike the typical failure injection usage), but
> forcing preallocated objecs use, I'm not sure we get much benefit (in terms
> of testing caller's error paths) from the fine grained selection of the
> first element where we inject fail, and failing immediately or never should
> be sufficient.

I guess. OTOH testing multiple failures could be useful?

> > Yes, this looks like broken copy and paste. The again I'm not even
> > sure who calls into mempool without __GFP_DIRECT_RECLAIM reset, as
> > that's kinda pointless.
>
> Hm yeah would have to be some special case where something limits how many
> such outstanding allocations can there be, otherwise it's just a cache to
> make success more likely but not guaranteed.

I think the only reason mempool_alloc even allows !__GFP_DIRECT_RECLAIM
is to avoid special casing that in callers that have a non-constant
gfp mask. So maybe the best thing would be to never actually go to
the pool for them and just give up if alloc_fn fails?

> >> > * This function only sleeps if the free_fn callback sleeps.
> >>
> >> This part now only applies to mempool_free() ?
> >
> > Both mempool_free and mempool_free_bulk.
>
> But mempool_free_bulk() doesn't use the callback, it's up to the caller to
> free anything the mempool didn't use for its refill.

You're right. So mempool_free_bulk itself will indeed never sleep and
I'll fix that up.