Re: [PATCH 2/2] mm/slab: don't queue the page cache work under krcp->lock

[Harry Yoo]

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@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > ---
> >  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