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

Vlastimil Babka
2025-11-06T14:57:15Z

On 11/6/25 15:48, Christoph Hellwig wrote:
> 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?

Maybe, guess I don't care that much, as long as it's not causing overhead on
every iteration when disabled, which it shouldn't.

>> > 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?

Yeah, but I guess we could keep trying the pool for the single allocation
case as that's simple enough, just not for the bulk.

>> >> > * 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.