在 8/1/2025 12:17 AM, Mikulas Patocka 写道:
On Tue, 29 Jul 2025, Dongsheng Yang wrote:
Just a suggestion for performance improvement: when you hold a
spinlock a you need to allocate some memory, you must drop the
spinlock, allocate it with GFP_NOIO, reacquire the spinlock and
recheck the state. You can improve this logic by allocating with
mempool_alloc(GFP_NOWAIT) while you hold the spinlock (GFP_NOWAIT
will not sleep, so it's allowed), and fallback to dropping the
spinlock only if the GFP_NOWAIT allocation fails.
Sounds a good suggestion, I will try it in V5.
Yes. When you implement it, don't forget to test the code path that drops
the spinlock and uses mempool_alloc(GFP_NOIO), so that it doesn't bitrot
over time.
Sure, the failslab case can cover this path:
https://github.com/DataTravelGuide/dtg-tests/blob/main/pcache.py.data/pcache_failslab.sh
Dongsheng
failslab doesn't test mempool allocation failures - see the function
mempool_alloc_noprof:
it tries to allocate using
element = pool->alloc(gfp_temp, pool->pool_data);
if it fails (the failure may be caused by failslab), it goes down this
path
spin_lock_irqsave(&pool->lock, flags);
if (likely(pool->curr_nr)) {
element = remove_element(pool);
spin_unlock_irqrestore(&pool->lock, flags);
/* paired with rmb in mempool_free(), read comment there */
smp_wmb();
/*
* Update the allocation stack trace as this is more useful
* for debugging.
*/
kmemleak_update_trace(element);
return element;
}
so, it will not simulate a situation where the mempool is depleted.
Yes. During my manual testing, I set mempool's min_nr to 0, triggered
the `GFP_NOIO` path with `failslab`, and inserted some debug messages to
confirm that the execution really went through that path.
I then automated the same procedure in dtg-tests: before failslab test
it applies a patch to change `min_nr`, runs the test and checks `dmesg`
for the expected debug output, and finally reverts the patch.
It may be worthwhile to extend failslab, so that it makes mempool_alloc
randomly return NULL for non-sleeping allocations (i.e. allocations
without __GFP_DIRECT_RECLAIM) and submit the patch to failslab
maintainers.
Indeed, if `mempool` were to support the `fault_inject` feature, the
whole process would be much simpler.
If I come up with a good way to add that support, I’ll send out a patch.
Thanx
Mikulas
