On 2/25/25 7:21 PM, Uladzislau Rezki wrote:
>>
> WQ_MEM_RECLAIM-patch fixes this for me:
Sounds good, can you send a formal patch then?
Some nits below:
> <snip>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 4030907b6b7d..1b5ed5512782 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1304,6 +1304,8 @@ module_param(rcu_min_cached_objs, int, 0444);
> static int rcu_delay_page_cache_fill_msec = 5000;
> module_param(rcu_delay_page_cache_fill_msec, int, 0444);
>
> +static struct workqueue_struct *rcu_reclaim_wq;
> +
> /* Maximum number of jiffies to wait before draining a batch. */
> #define KFREE_DRAIN_JIFFIES (5 * HZ)
> #define KFREE_N_BATCHES 2
> @@ -1632,10 +1634,10 @@ __schedule_delayed_monitor_work(struct kfree_rcu_cpu *krcp)
> if (delayed_work_pending(&krcp->monitor_work)) {
> delay_left = krcp->monitor_work.timer.expires - jiffies;
> if (delay < delay_left)
> - mod_delayed_work(system_unbound_wq, &krcp->monitor_work, delay);
> + mod_delayed_work(rcu_reclaim_wq, &krcp->monitor_work, delay);
> return;
> }
> - queue_delayed_work(system_unbound_wq, &krcp->monitor_work, delay);
> + queue_delayed_work(rcu_reclaim_wq, &krcp->monitor_work, delay);
> }
>
> static void
> @@ -1733,7 +1735,7 @@ kvfree_rcu_queue_batch(struct kfree_rcu_cpu *krcp)
> // "free channels", the batch can handle. Break
> // the loop since it is done with this CPU thus
> // queuing an RCU work is _always_ success here.
> - queued = queue_rcu_work(system_unbound_wq, &krwp->rcu_work);
> + queued = queue_rcu_work(rcu_reclaim_wq, &krwp->rcu_work);
> WARN_ON_ONCE(!queued);
> break;
> }
> @@ -1883,7 +1885,7 @@ run_page_cache_worker(struct kfree_rcu_cpu *krcp)
> if (rcu_scheduler_active == RCU_SCHEDULER_RUNNING &&
> !atomic_xchg(&krcp->work_in_progress, 1)) {
> if (atomic_read(&krcp->backoff_page_cache_fill)) {
> - queue_delayed_work(system_unbound_wq,
> + queue_delayed_work(rcu_reclaim_wq,
> &krcp->page_cache_work,
> msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
> } else {
> @@ -2120,6 +2122,10 @@ void __init kvfree_rcu_init(void)
> int i, j;
> struct shrinker *kfree_rcu_shrinker;
>
> + rcu_reclaim_wq = alloc_workqueue("rcu_reclaim",
Should we name it "kvfree_rcu_reclaim"? rcu_reclaim sounds too generic
as if it's part of rcu itself?
> + WQ_UNBOUND | WQ_MEM_RECLAIM, 0);
Do we want WQ_SYSFS? Or maybe only when someone asks, with a use case?
Thanks,
Vlastimil
> + WARN_ON(!rcu_reclaim_wq);
> +
> /* Clamp it to [0:100] seconds interval. */
> if (rcu_delay_page_cache_fill_msec < 0 ||
> rcu_delay_page_cache_fill_msec > 100 * MSEC_PER_SEC) {
> <snip>
>
> it passes:
>
> <snip>
> [ 15.972416] KTAP version 1
> [ 15.972421] 1..1
> [ 15.973467] KTAP version 1
> [ 15.973470] # Subtest: slub_test
> [ 15.973472] # module: slub_kunit
> [ 15.973474] 1..10
> [ 15.974483] ok 1 test_clobber_zone
> [ 15.974927] ok 2 test_next_pointer
> [ 15.975308] ok 3 test_first_word
> [ 15.975672] ok 4 test_clobber_50th_byte
> [ 15.976035] ok 5 test_clobber_redzone_free
> [ 15.976128] stackdepot: allocating hash table of 1048576 entries via kvcalloc
> [ 15.979505] ok 6 test_kmalloc_redzone_access
> [ 16.014408] ok 7 test_kfree_rcu
> [ 17.726602] ok 8 test_kfree_rcu_wq_destroy
> [ 17.750323] ok 9 test_leak_destroy
> [ 17.750883] ok 10 test_krealloc_redzone_zeroing
> [ 17.750887] # slub_test: pass:10 fail:0 skip:0 total:10
> [ 17.750890] # Totals: pass:10 fail:0 skip:0 total:10
> [ 17.750891] ok 1 slub_test
> <snip>
>
> --
> Uladzislau Rezki
