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

[Harry Yoo]

On Mon, Nov 09, 2026 at 12:48:31PM +0100, Uladzislau Rezki wrote:
> On Fri, Nov 06, 2026 at 12:21:28PM +0000, Harry Yoo wrote:
> > 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?
> > 
> Yes. It is now in milliseconds.

Thanks for clarifying.

> So starting with 1 - means that it can be configured at least from 1 msec
> whereas 0 is forbidden. Also we can drop the extra "if condition" proposed
> in the patch as it never can be zero.

But can we explain why forbiding 0 msec can be justified (when there's
an alternative approach to fix it?)

-- 
Cheers,
Harry / Hyeonggon