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.
