mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
@ 2026-10-28 13:46 Zhan Xusheng
  2026-10-28 13:46 ` [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock Zhan Xusheng
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Zhan Xusheng @ 2026-10-28 13:46 UTC (permalink / raw)
  To: vbabka, harry
  Cc: urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

The move of kvfree_rcu() to mm/slab_common.c dropped the "rcutree."
MODULE_PARAM_PREFIX, so rcu_min_cached_objs and
rcu_delay_page_cache_fill_msec became slab_common.* and the documented
rcutree.* names are silently ignored.  Restore the prefix; the
slab_common.* names, never documented, stop working.

Fixes: bbe658d65802 ("mm/slab: Move kvfree_rcu() into SLAB")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 mm/slab_common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 01c64ef661e7..3729aa7a6156 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1373,6 +1373,10 @@ void __init kvfree_rcu_init(void)
  * per-CPU. Object size is equal to one page. This value
  * can be changed at boot time.
  */
+/* These used to be rcutree parameters; keep their documented names. */
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "rcutree."
+
 static int rcu_min_cached_objs = 5;
 module_param(rcu_min_cached_objs, int, 0444);
 

base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
-- 
2.43.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-10-28 13:46 [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Zhan Xusheng
@ 2026-10-28 13:46 ` Zhan Xusheng
  2026-11-02 14:35   ` Harry Yoo
  2026-11-06  9:35   ` Uladzislau Rezki
  2026-10-28 15:23 ` [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Harry Yoo
  2026-11-02 14:27 ` Harry Yoo (Meta)
  2 siblings, 2 replies; 14+ messages in thread
From: Zhan Xusheng @ 2026-10-28 13:46 UTC (permalink / raw)
  To: vbabka, harry
  Cc: urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
callers that may hold more, such as set_cpus_allowed_force() under
pi_lock, so it kicks the refill work from an hrtimer.  Under memory
pressure it instead queues the work with a delay of
rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
queues it at once, taking pool->lock under krcp->lock.

With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
pi_lock after a drop_caches makes lockdep report:

  Chain exists of:
    &pool->lock --> &p->pi_lock --> krc.lock

Use the hrtimer when the delay is 0.

Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 mm/slab_common.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 3729aa7a6156..ef5ff0a20249 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1987,7 +1987,13 @@ 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)) {
+		/*
+		 * A zero delay would queue the work right here, under
+		 * krcp->lock and possibly the caller's pi_lock, so let the
+		 * hrtimer do it.
+		 */
+		if (atomic_read(&krcp->backoff_page_cache_fill) &&
+		    rcu_delay_page_cache_fill_msec) {
 			queue_delayed_work(rcu_reclaim_wq,
 				&krcp->page_cache_work,
 					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
-- 
2.43.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
  2026-10-28 13:46 [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Zhan Xusheng
  2026-10-28 13:46 ` [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock Zhan Xusheng
@ 2026-10-28 15:23 ` Harry Yoo
  2026-11-02 14:33   ` Harry Yoo
  2026-11-02 14:27 ` Harry Yoo (Meta)
  2 siblings, 1 reply; 14+ messages in thread
From: Harry Yoo @ 2026-10-28 15:23 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: vbabka, urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

On Wed, Oct 28, 2026 at 09:46:46PM +0800, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> The move of kvfree_rcu() to mm/slab_common.c dropped the "rcutree."
> MODULE_PARAM_PREFIX, so rcu_min_cached_objs and
> rcu_delay_page_cache_fill_msec became slab_common.* and the documented
> rcutree.* names are silently ignored.  Restore the prefix; the
> slab_common.* names, never documented, stop working.
> 
> Fixes: bbe658d65802 ("mm/slab: Move kvfree_rcu() into SLAB")

We should add:

Cc: stable@vger.kernel.org

This is a functional regression and in MM we don't automatically
backport fixes to -stable unless without this tag.

> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  mm/slab_common.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 01c64ef661e7..3729aa7a6156 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1373,6 +1373,10 @@ void __init kvfree_rcu_init(void)
>   * per-CPU. Object size is equal to one page. This value
>   * can be changed at boot time.
>   */
> +/* These used to be rcutree parameters; keep their documented names. */
> +#undef MODULE_PARAM_PREFIX
> +#define MODULE_PARAM_PREFIX "rcutree."

This should be moved to before "This rcu parameter is
runtime-read-only ..." part?

And perhaps let's rephrase the comment, something like:

/* Keep the prefix inherited from RCU */

With that adjusted, otherwise looks good to me, so:
Reviewed-by: Harry Yoo (Meta) <harry@kernel.org>

-- 
Cheers,
Harry / Hyeonggon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
  2026-10-28 13:46 [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Zhan Xusheng
  2026-10-28 13:46 ` [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock Zhan Xusheng
  2026-10-28 15:23 ` [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Harry Yoo
@ 2026-11-02 14:27 ` Harry Yoo (Meta)
  2 siblings, 0 replies; 14+ messages in thread
From: Harry Yoo (Meta) @ 2026-11-02 14:27 UTC (permalink / raw)
  To: vbabka, Zhan Xusheng
  Cc: kernel-team, urezki, paulmck, qiang.zhang, akpm, hao.li, cl,
	rientjes, roman.gushchin, linux-mm, rcu, linux-kernel,
	zhanxusheng

On Wed, 28 Oct 2026 21:46:46 +0800, Zhan Xusheng wrote:
> The move of kvfree_rcu() to mm/slab_common.c dropped the "rcutree."
> MODULE_PARAM_PREFIX, so rcu_min_cached_objs and
> rcu_delay_page_cache_fill_msec became slab_common.* and the documented
> rcutree.* names are silently ignored.  Restore the prefix; the
> slab_common.* names, never documented, stop working.
> 
> 
> [...]

Applied to mm/slab.git slab/for-next-fixes, thanks!

[1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
      commit: e6316aa3c7aec549e3ab017a9c49fca52194fd4a
[2/2] mm/slab: don't queue the page cache work under krcp->lock
      commit: 03a176b9248c4513bb705ed3d931af3a7e9c2a08

-- 
Cheers,
Harry / Hyeonggon


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
  2026-10-28 15:23 ` [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Harry Yoo
@ 2026-11-02 14:33   ` Harry Yoo
  2026-11-04  8:17     ` Vlastimil Babka (SUSE)
  2026-11-06  7:59     ` Hao Li
  0 siblings, 2 replies; 14+ messages in thread
From: Harry Yoo @ 2026-11-02 14:33 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: vbabka, urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

I adjusted my feedback. End result:

mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters

The move of kvfree_rcu() to mm/slab_common.c dropped the "rcutree."
MODULE_PARAM_PREFIX, so rcu_min_cached_objs and
rcu_delay_page_cache_fill_msec became slab_common.* and the documented
rcutree.* names are silently ignored.  Restore the prefix; the
slab_common.* names, never documented, stop working.

Fixes: bbe658d65802 ("mm/slab: Move kvfree_rcu() into SLAB")
Cc: stable@vger.kernel.org
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Reviewed-by: Harry Yoo (Meta) <harry@kernel.org>
Link: https://patch.msgid.link/20261028134647.3176080-1-zhanxusheng1024@gmail.com
Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 mm/slab_common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 01c64ef661e7..41dea01a8c1f 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1367,6 +1367,10 @@ void __init kvfree_rcu_init(void)
 
 #else /* CONFIG_KVFREE_RCU_BATCHED */
 
+/* Keep the prefix inherited from RCU */
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "rcutree."
+
 /*
  * This rcu parameter is runtime-read-only. It reflects
  * a minimum allowed number of objects which can be cached
-- 
2.54.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-10-28 13:46 ` [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock Zhan Xusheng
@ 2026-11-02 14:35   ` Harry Yoo
  2026-11-04  8:19     ` Vlastimil Babka (SUSE)
                       ` (2 more replies)
  2026-11-06  9:35   ` Uladzislau Rezki
  1 sibling, 3 replies; 14+ messages in thread
From: Harry Yoo @ 2026-11-02 14:35 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: vbabka, urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

Here I added bit more context and adjusted comment. End result:

mm/slab: don't queue the page cache work under krcp->lock

Commit 56292e8609e3 ("rcu/tree: Defer kvfree_rcu() allocation to a
clean context") deferred page allocations to a workqueue. To avoid
deadlocks with the scheduler, it used hrtimer to queue the work instead
of queueing it directly.

Commit d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory
pressure") later added a backoff mechanism under memory pressure, but
did not consider that queue_delayed_work() with delay = 0 immediately
queues the work and introduced the deadlock commit 56292e8609e3 tried to
avoid.

run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
callers that may hold more, such as set_cpus_allowed_force() under
pi_lock, so it kicks the refill work from an hrtimer. Under memory
pressure it instead queues the work with a delay of
rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
queues it at once, taking pool->lock under krcp->lock.

With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
pi_lock after a drop_caches makes lockdep report:

  Chain exists of:
    &pool->lock --> &p->pi_lock --> krc.lock

Use the hrtimer when the delay is 0.

Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
Cc: stable@vger.kernel.org
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Link: https://patch.msgid.link/20261028134647.3176080-2-zhanxusheng1024@gmail.com
[ harry@kernel.org: add more context to changelog and adjust comment ]
Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
---
 mm/slab_common.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 41dea01a8c1f..051ca63c0b67 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1987,7 +1987,15 @@ 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)) {
+		/*
+		 * Use the hrtimer to avoid a deadlock with the scheduler
+		 * by waking up the worker directly. queue_delayed_work()
+		 * uses a timer to queue the work, so it's fine.
+		 * But with delay = 0 it queues the work immediately.
+		 * Fall back to the hrtimer in that case.
+		 */
+		if (atomic_read(&krcp->backoff_page_cache_fill) &&
+		    rcu_delay_page_cache_fill_msec) {
 			queue_delayed_work(rcu_reclaim_wq,
 				&krcp->page_cache_work,
 					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
-- 
2.54.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
  2026-11-02 14:33   ` Harry Yoo
@ 2026-11-04  8:17     ` Vlastimil Babka (SUSE)
  2026-11-06  7:59     ` Hao Li
  1 sibling, 0 replies; 14+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-11-04  8:17 UTC (permalink / raw)
  To: Harry Yoo, Zhan Xusheng
  Cc: urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

On 11/2/26 15:33, Harry Yoo wrote:
> I adjusted my feedback. End result:
> 
> mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
> 
> The move of kvfree_rcu() to mm/slab_common.c dropped the "rcutree."
> MODULE_PARAM_PREFIX, so rcu_min_cached_objs and
> rcu_delay_page_cache_fill_msec became slab_common.* and the documented
> rcutree.* names are silently ignored.  Restore the prefix; the
> slab_common.* names, never documented, stop working.
> 
> Fixes: bbe658d65802 ("mm/slab: Move kvfree_rcu() into SLAB")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Reviewed-by: Harry Yoo (Meta) <harry@kernel.org>
> Link: https://patch.msgid.link/20261028134647.3176080-1-zhanxusheng1024@gmail.com
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/slab_common.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 01c64ef661e7..41dea01a8c1f 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1367,6 +1367,10 @@ void __init kvfree_rcu_init(void)
>  
>  #else /* CONFIG_KVFREE_RCU_BATCHED */
>  
> +/* Keep the prefix inherited from RCU */
> +#undef MODULE_PARAM_PREFIX
> +#define MODULE_PARAM_PREFIX "rcutree."
> +
>  /*
>   * This rcu parameter is runtime-read-only. It reflects
>   * a minimum allowed number of objects which can be cached


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-11-02 14:35   ` Harry Yoo
@ 2026-11-04  8:19     ` Vlastimil Babka (SUSE)
  2026-11-06  8:00     ` Hao Li
  2026-11-06 11:32     ` Hao Li
  2 siblings, 0 replies; 14+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-11-04  8:19 UTC (permalink / raw)
  To: Harry Yoo, Zhan Xusheng
  Cc: urezki, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng

On 11/2/26 15:35, Harry Yoo wrote:
> Here I added bit more context and adjusted comment. End result:
> 
> mm/slab: don't queue the page cache work under krcp->lock
> 
> Commit 56292e8609e3 ("rcu/tree: Defer kvfree_rcu() allocation to a
> clean context") deferred page allocations to a workqueue. To avoid
> deadlocks with the scheduler, it used hrtimer to queue the work instead
> of queueing it directly.
> 
> Commit d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory
> pressure") later added a backoff mechanism under memory pressure, but
> did not consider that queue_delayed_work() with delay = 0 immediately
> queues the work and introduced the deadlock commit 56292e8609e3 tried to
> avoid.
> 
> run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
> callers that may hold more, such as set_cpus_allowed_force() under
> pi_lock, so it kicks the refill work from an hrtimer. Under memory
> pressure it instead queues the work with a delay of
> rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
> queues it at once, taking pool->lock under krcp->lock.
> 
> With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
> pi_lock after a drop_caches makes lockdep report:
> 
>   Chain exists of:
>     &pool->lock --> &p->pi_lock --> krc.lock
> 
> Use the hrtimer when the delay is 0.
> 
> Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Link: https://patch.msgid.link/20261028134647.3176080-2-zhanxusheng1024@gmail.com
> [ harry@kernel.org: add more context to changelog and adjust comment ]
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/slab_common.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 41dea01a8c1f..051ca63c0b67 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1987,7 +1987,15 @@ 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)) {
> +		/*
> +		 * Use the hrtimer to avoid a deadlock with the scheduler
> +		 * by waking up the worker directly. queue_delayed_work()
> +		 * uses a timer to queue the work, so it's fine.
> +		 * But with delay = 0 it queues the work immediately.
> +		 * Fall back to the hrtimer in that case.
> +		 */
> +		if (atomic_read(&krcp->backoff_page_cache_fill) &&
> +		    rcu_delay_page_cache_fill_msec) {
>  			queue_delayed_work(rcu_reclaim_wq,
>  				&krcp->page_cache_work,
>  					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
  2026-11-02 14:33   ` Harry Yoo
  2026-11-04  8:17     ` Vlastimil Babka (SUSE)
@ 2026-11-06  7:59     ` Hao Li
  1 sibling, 0 replies; 14+ messages in thread
From: Hao Li @ 2026-11-06  7:59 UTC (permalink / raw)
  To: Harry Yoo
  Cc: Zhan Xusheng, vbabka, urezki, paulmck, qiang.zhang, akpm, cl,
	rientjes, roman.gushchin, linux-mm, rcu, linux-kernel,
	zhanxusheng

On Mon, Nov 02, 2026 at 02:33:43PM +0000, Harry Yoo wrote:
> I adjusted my feedback. End result:
> 
> mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters
> 
> The move of kvfree_rcu() to mm/slab_common.c dropped the "rcutree."
> MODULE_PARAM_PREFIX, so rcu_min_cached_objs and
> rcu_delay_page_cache_fill_msec became slab_common.* and the documented
> rcutree.* names are silently ignored.  Restore the prefix; the
> slab_common.* names, never documented, stop working.
> 
> Fixes: bbe658d65802 ("mm/slab: Move kvfree_rcu() into SLAB")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Reviewed-by: Harry Yoo (Meta) <harry@kernel.org>
> Link: https://patch.msgid.link/20261028134647.3176080-1-zhanxusheng1024@gmail.com
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>

Reviewed-by: Hao Li <hao.li@linux.dev>

-- 
Thanks,
Hao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-11-02 14:35   ` Harry Yoo
  2026-11-04  8:19     ` Vlastimil Babka (SUSE)
@ 2026-11-06  8:00     ` Hao Li
  2026-11-06 11:32     ` Hao Li
  2 siblings, 0 replies; 14+ messages in thread
From: Hao Li @ 2026-11-06  8:00 UTC (permalink / raw)
  To: Harry Yoo
  Cc: Zhan Xusheng, vbabka, urezki, paulmck, qiang.zhang, akpm, cl,
	rientjes, roman.gushchin, linux-mm, rcu, linux-kernel,
	zhanxusheng

On Mon, Nov 02, 2026 at 02:35:40PM +0000, Harry Yoo wrote:
> Here I added bit more context and adjusted comment. End result:
> 
> mm/slab: don't queue the page cache work under krcp->lock
> 
> Commit 56292e8609e3 ("rcu/tree: Defer kvfree_rcu() allocation to a
> clean context") deferred page allocations to a workqueue. To avoid
> deadlocks with the scheduler, it used hrtimer to queue the work instead
> of queueing it directly.
> 
> Commit d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory
> pressure") later added a backoff mechanism under memory pressure, but
> did not consider that queue_delayed_work() with delay = 0 immediately
> queues the work and introduced the deadlock commit 56292e8609e3 tried to
> avoid.
> 
> run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
> callers that may hold more, such as set_cpus_allowed_force() under
> pi_lock, so it kicks the refill work from an hrtimer. Under memory
> pressure it instead queues the work with a delay of
> rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
> queues it at once, taking pool->lock under krcp->lock.
> 
> With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
> pi_lock after a drop_caches makes lockdep report:
> 
>   Chain exists of:
>     &pool->lock --> &p->pi_lock --> krc.lock
> 
> Use the hrtimer when the delay is 0.
> 
> Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Link: https://patch.msgid.link/20261028134647.3176080-2-zhanxusheng1024@gmail.com
> [ harry@kernel.org: add more context to changelog and adjust comment ]
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
> ---
>  mm/slab_common.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Hao Li <hao.li@linux.dev>

-- 
Thanks,
Hao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-10-28 13:46 ` [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock Zhan Xusheng
  2026-11-02 14:35   ` Harry Yoo
@ 2026-11-06  9:35   ` Uladzislau Rezki
  2026-11-06 12:18     ` Zhan Xusheng
  2026-11-06 12:21     ` Harry Yoo
  1 sibling, 2 replies; 14+ messages in thread
From: Uladzislau Rezki @ 2026-11-06  9:35 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: vbabka, harry, urezki, paulmck, qiang.zhang, akpm, hao.li, cl,
	rientjes, roman.gushchin, linux-mm, rcu, linux-kernel,
	zhanxusheng

On Wed, Oct 28, 2026 at 09:46:47PM +0800, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
> callers that may hold more, such as set_cpus_allowed_force() under
> pi_lock, so it kicks the refill work from an hrtimer.  Under memory
> pressure it instead queues the work with a delay of
> rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
> queues it at once, taking pool->lock under krcp->lock.
> 
> With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
> pi_lock after a drop_caches makes lockdep report:
> 
>   Chain exists of:
>     &pool->lock --> &p->pi_lock --> krc.lock
> 
> Use the hrtimer when the delay is 0.
> 
> Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  mm/slab_common.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 3729aa7a6156..ef5ff0a20249 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1987,7 +1987,13 @@ 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)) {
> +		/*
> +		 * A zero delay would queue the work right here, under
> +		 * krcp->lock and possibly the caller's pi_lock, so let the
> +		 * hrtimer do it.
> +		 */
> +		if (atomic_read(&krcp->backoff_page_cache_fill) &&
> +		    rcu_delay_page_cache_fill_msec) {
>  			queue_delayed_work(rcu_reclaim_wq,
>  				&krcp->page_cache_work,
>  					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
> -- 
> 2.43.0
> 
> 
Can we just clamp this to 1 - 100 seconds interval?

<snip>
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 7223a7596dab..2c9236236ac9 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -2249,12 +2249,12 @@ void __init kvfree_rcu_init(void)
 			WQ_UNBOUND | WQ_MEM_RECLAIM, 0);
 	WARN_ON(!rcu_reclaim_wq);
 
-	/* Clamp it to [0:100] seconds interval. */
-	if (rcu_delay_page_cache_fill_msec < 0 ||
+	/* Clamp it to [1:100] seconds interval. */
+	if (rcu_delay_page_cache_fill_msec <= 0 ||
 		rcu_delay_page_cache_fill_msec > 100 * MSEC_PER_SEC) {
 
 		rcu_delay_page_cache_fill_msec =
-			clamp(rcu_delay_page_cache_fill_msec, 0,
+			clamp(rcu_delay_page_cache_fill_msec, 1,
 				(int) (100 * MSEC_PER_SEC));
 
 		pr_info("Adjusting rcutree.rcu_delay_page_cache_fill_msec to %d ms.\n",
<snip>

so we forbid zero as value to use?

--
Uladzislau Rezki

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-11-02 14:35   ` Harry Yoo
  2026-11-04  8:19     ` Vlastimil Babka (SUSE)
  2026-11-06  8:00     ` Hao Li
@ 2026-11-06 11:32     ` Hao Li
  2 siblings, 0 replies; 14+ messages in thread
From: Hao Li @ 2026-11-06 11:32 UTC (permalink / raw)
  To: Harry Yoo
  Cc: Zhan Xusheng, vbabka, urezki, paulmck, qiang.zhang, akpm, cl,
	rientjes, roman.gushchin, linux-mm, rcu, linux-kernel,
	zhanxusheng

On Mon, Nov 02, 2026 at 02:35:40PM +0000, Harry Yoo wrote:
> Here I added bit more context and adjusted comment. End result:
> 
> mm/slab: don't queue the page cache work under krcp->lock
> 
> Commit 56292e8609e3 ("rcu/tree: Defer kvfree_rcu() allocation to a
> clean context") deferred page allocations to a workqueue. To avoid
> deadlocks with the scheduler, it used hrtimer to queue the work instead
> of queueing it directly.
> 
> Commit d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory
> pressure") later added a backoff mechanism under memory pressure, but
> did not consider that queue_delayed_work() with delay = 0 immediately
> queues the work and introduced the deadlock commit 56292e8609e3 tried to
> avoid.
> 
> run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
> callers that may hold more, such as set_cpus_allowed_force() under
> pi_lock, so it kicks the refill work from an hrtimer. Under memory
> pressure it instead queues the work with a delay of
> rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
> queues it at once, taking pool->lock under krcp->lock.
> 
> With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
> pi_lock after a drop_caches makes lockdep report:
> 
>   Chain exists of:
>     &pool->lock --> &p->pi_lock --> krc.lock
> 
> Use the hrtimer when the delay is 0.
> 
> Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> Link: https://patch.msgid.link/20261028134647.3176080-2-zhanxusheng1024@gmail.com
> [ harry@kernel.org: add more context to changelog and adjust comment ]
> Signed-off-by: Harry Yoo (Meta) <harry@kernel.org>
> ---
>  mm/slab_common.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 41dea01a8c1f..051ca63c0b67 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1987,7 +1987,15 @@ 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)) {
> +		/*
> +		 * Use the hrtimer to avoid a deadlock with the scheduler
> +		 * by waking up the worker directly. queue_delayed_work()
> +		 * uses a timer to queue the work, so it's fine.
> +		 * But with delay = 0 it queues the work immediately.
> +		 * Fall back to the hrtimer in that case.
> +		 */
> +		if (atomic_read(&krcp->backoff_page_cache_fill) &&
> +		    rcu_delay_page_cache_fill_msec) {
>  			queue_delayed_work(rcu_reclaim_wq,
>  				&krcp->page_cache_work,
>  					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
> -- 
> 2.54.0

Just a thought: once this fix is applied, could we simplify the code further by
always queuing the work via hrtimer? This would make the logic more
understandable. For example:

diff --git a/mm/slab_common.c b/mm/slab_common.c
index ef5ff0a20249..e5213f952e16 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1480,7 +1480,7 @@ struct kfree_rcu_cpu {
 	struct delayed_work monitor_work;
 	bool initialized;
 
-	struct delayed_work page_cache_work;
+	struct work_struct page_cache_work;
 	atomic_t backoff_page_cache_fill;
 	atomic_t work_in_progress;
 	struct hrtimer hrtimer;
@@ -1878,7 +1878,7 @@ static void fill_page_cache_func(struct work_struct *work)
 	struct kvfree_rcu_bulk_data *bnode;
 	struct kfree_rcu_cpu *krcp =
 		container_of(work, struct kfree_rcu_cpu,
-			page_cache_work.work);
+			page_cache_work);
 	unsigned long flags;
 	int nr_pages;
 	bool pushed;
@@ -1974,34 +1974,27 @@ schedule_page_work_fn(struct hrtimer *t)
 	struct kfree_rcu_cpu *krcp =
 		container_of(t, struct kfree_rcu_cpu, hrtimer);
 
-	queue_delayed_work(system_highpri_wq, &krcp->page_cache_work, 0);
+	queue_work(system_highpri_wq, &krcp->page_cache_work);
 	return HRTIMER_NORESTART;
 }
 
 static void
 run_page_cache_worker(struct kfree_rcu_cpu *krcp)
 {
+	ktime_t delay = 0;
+
 	// If cache disabled, bail out.
 	if (!rcu_min_cached_objs)
 		return;
 
 	if (rcu_scheduler_active == RCU_SCHEDULER_RUNNING &&
 			!atomic_xchg(&krcp->work_in_progress, 1)) {
-		/*
-		 * A zero delay would queue the work right here, under
-		 * krcp->lock and possibly the caller's pi_lock, so let the
-		 * hrtimer do it.
-		 */
-		if (atomic_read(&krcp->backoff_page_cache_fill) &&
-		    rcu_delay_page_cache_fill_msec) {
-			queue_delayed_work(rcu_reclaim_wq,
-				&krcp->page_cache_work,
-					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
-		} else {
-			hrtimer_setup(&krcp->hrtimer, schedule_page_work_fn, CLOCK_MONOTONIC,
-				      HRTIMER_MODE_REL);
-			hrtimer_start(&krcp->hrtimer, 0, HRTIMER_MODE_REL);
-		}
+		if (atomic_read(&krcp->backoff_page_cache_fill))
+			delay = ms_to_ktime(rcu_delay_page_cache_fill_msec);
+
+		hrtimer_setup(&krcp->hrtimer, schedule_page_work_fn, CLOCK_MONOTONIC,
+			      HRTIMER_MODE_REL);
+		hrtimer_start(&krcp->hrtimer, delay, HRTIMER_MODE_REL);
 	}
 }
 
@@ -2295,7 +2288,7 @@ void __init kvfree_rcu_init(void)
 			INIT_LIST_HEAD(&krcp->bulk_head[i]);
 
 		INIT_DELAYED_WORK(&krcp->monitor_work, kfree_rcu_monitor);
-		INIT_DELAYED_WORK(&krcp->page_cache_work, fill_page_cache_func);
+		INIT_WORK(&krcp->page_cache_work, fill_page_cache_func);
 		krcp->initialized = true;
 	}
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-11-06  9:35   ` Uladzislau Rezki
@ 2026-11-06 12:18     ` Zhan Xusheng
  2026-11-06 12:21     ` Harry Yoo
  1 sibling, 0 replies; 14+ messages in thread
From: Zhan Xusheng @ 2026-11-06 12:18 UTC (permalink / raw)
  To: Uladzislau Rezki
  Cc: vbabka, harry, paulmck, qiang.zhang, akpm, hao.li, cl, rientjes,
	roman.gushchin, linux-mm, rcu, linux-kernel, zhanxusheng,
	Zhan Xusheng

Hi Uladzislau,

> so we forbid zero as value to use?

Yes, that works as well. The parameter is 0444, so the clamp in
kvfree_rcu_init() covers every case, and with a delay of at least one
jiffy queue_delayed_work() arms a timer instead of queueing the work
under krcp->lock. The check added to run_page_cache_worker() would
then not be needed.

Thanks,
Zhan Xusheng

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock
  2026-11-06  9:35   ` Uladzislau Rezki
  2026-11-06 12:18     ` Zhan Xusheng
@ 2026-11-06 12:21     ` Harry Yoo
  1 sibling, 0 replies; 14+ messages in thread
From: Harry Yoo @ 2026-11-06 12:21 UTC (permalink / raw)
  To: Uladzislau Rezki
  Cc: Zhan Xusheng, vbabka, paulmck, qiang.zhang, akpm, hao.li, cl,
	rientjes, roman.gushchin, linux-mm, rcu, linux-kernel,
	zhanxusheng

On Fri, Nov 06, 2026 at 10:35:13AM +0100, Uladzislau Rezki wrote:
> On Wed, Oct 28, 2026 at 09:46:47PM +0800, Zhan Xusheng wrote:
> > From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> > 
> > run_page_cache_worker() runs under krcp->lock, from kvfree_call_rcu()
> > callers that may hold more, such as set_cpus_allowed_force() under
> > pi_lock, so it kicks the refill work from an hrtimer.  Under memory
> > pressure it instead queues the work with a delay of
> > rcutree.rcu_delay_page_cache_fill_msec, which may be 0, and a zero delay
> > queues it at once, taking pool->lock under krcp->lock.
> > 
> > With the delay set to 0, calling kvfree_rcu() on vmalloc memory under
> > pi_lock after a drop_caches makes lockdep report:
> > 
> >   Chain exists of:
> >     &pool->lock --> &p->pi_lock --> krc.lock
> > 
> > Use the hrtimer when the delay is 0.
> > 
> > Fixes: d0bfa8b3c411 ("kvfree_rcu: Release a page cache under memory pressure")
> > Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> > ---
> >  mm/slab_common.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > index 3729aa7a6156..ef5ff0a20249 100644
> > --- a/mm/slab_common.c
> > +++ b/mm/slab_common.c
> > @@ -1987,7 +1987,13 @@ 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)) {
> > +		/*
> > +		 * A zero delay would queue the work right here, under
> > +		 * krcp->lock and possibly the caller's pi_lock, so let the
> > +		 * hrtimer do it.
> > +		 */
> > +		if (atomic_read(&krcp->backoff_page_cache_fill) &&
> > +		    rcu_delay_page_cache_fill_msec) {
> >  			queue_delayed_work(rcu_reclaim_wq,
> >  				&krcp->page_cache_work,
> >  					msecs_to_jiffies(rcu_delay_page_cache_fill_msec));
> > -- 
> > 2.43.0
> > 
> > 
> Can we just clamp this to 1 - 100 seconds interval?

You mean clamp to [1:100,000] milliseconds interval?

> <snip>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 7223a7596dab..2c9236236ac9 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -2249,12 +2249,12 @@ void __init kvfree_rcu_init(void)
>  			WQ_UNBOUND | WQ_MEM_RECLAIM, 0);
>  	WARN_ON(!rcu_reclaim_wq);
>  
> -	/* Clamp it to [0:100] seconds interval. */
> -	if (rcu_delay_page_cache_fill_msec < 0 ||
> +	/* Clamp it to [1:100] seconds interval. */
> +	if (rcu_delay_page_cache_fill_msec <= 0 ||
>  		rcu_delay_page_cache_fill_msec > 100 * MSEC_PER_SEC) {
>  
>  		rcu_delay_page_cache_fill_msec =
> -			clamp(rcu_delay_page_cache_fill_msec, 0,
> +			clamp(rcu_delay_page_cache_fill_msec, 1,
>  				(int) (100 * MSEC_PER_SEC));
>  
>  		pr_info("Adjusting rcutree.rcu_delay_page_cache_fill_msec to %d ms.\n",
> <snip>
> 
> so we forbid zero as value to use?

IMO in general we should keep existing behaviour unless we could
demonstrate setting a delay < 1ms is fundamentally flawed?

-- 
Cheers,
Harry / Hyeonggon

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-11-06 12:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 13:46 [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Zhan Xusheng
2026-10-28 13:46 ` [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock Zhan Xusheng
2026-11-02 14:35   ` Harry Yoo
2026-11-04  8:19     ` Vlastimil Babka (SUSE)
2026-11-06  8:00     ` Hao Li
2026-11-06 11:32     ` Hao Li
2026-11-06  9:35   ` Uladzislau Rezki
2026-11-06 12:18     ` Zhan Xusheng
2026-11-06 12:21     ` Harry Yoo
2026-10-28 15:23 ` [PATCH 1/2] mm/slab: restore the rcutree. prefix of the kvfree_rcu parameters Harry Yoo
2026-11-02 14:33   ` Harry Yoo
2026-11-04  8:17     ` Vlastimil Babka (SUSE)
2026-11-06  7:59     ` Hao Li
2026-11-02 14:27 ` Harry Yoo (Meta)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®