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

[Christoph Hellwig <hch@lst.de>]

On Wed, Nov 05, 2025 at 04:04:53PM +0100, Vlastimil Babka wrote:
> > +	for (; i < count; i++) {
> > +		if (!elem[i]) {
> > +			if (should_fail_ex(&fail_mempool_alloc, 1,
> > +					FAULT_NOWARN)) {
> > +				pr_info("forcing pool usage for pool %pS\n",
> > +					(void *)caller_ip);
> > +				goto use_pool;
> > +			}
> 
> 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?

> >  	/*
> > @@ -445,10 +463,12 @@ void *mempool_alloc_noprof(mempool_t *pool, gfp_t gfp_mask)
> >  	/* We must not sleep if !__GFP_DIRECT_RECLAIM */
> >  	if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {
> >  		spin_unlock_irqrestore(&pool->lock, flags);
> > -		return NULL;
> > +		if (i > 0)
> > +			mempool_free_bulk(pool, elem + i, count - i);
> 
> I don't understand why we are trying to free from i to count and not from 0
> to i? Seems buggy, there will likely be NULLs which might go through
> add_element() which assumes they are not NULL.

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.

> Assuming this is fixed we might still have confusing API. We might be
> freeing away elements that were already in the array when
> mempool_alloc_bulk() was called. OTOH the pool might be missing less than i
> elements and mempool_free_bulk() will not do anything with the rest.
> Anything beyond i is untouched. The caller has no idea what's in the array
> after getting this -ENOMEM. (alloc_pages_bulk() returns the number of pages
> there).
> Maybe it's acceptable (your usecase I think doesn't even add a caller that
> can't block), but needs documenting clearly.

I'm tempted to just disallow !__GFP_DIRECT_RECLAIM bulk allocations.
That feature seems to being a lot of trouble for no real gain, as
we can't use mempool as a guaranteed allocator there, so it's kinda
pointless.

> So in theory callers waiting for many objects might wait indefinitely to
> find enough objects in the pool, while smaller callers succeed their
> allocations and deplete the pool. Mempools never provided some fair ordering
> of waiters, but this might make it worse deterministically instead of
> randomly. Guess it's not such a problem if all callers are comparable in
> number of objects.

Yeah, which is the use case.

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