Re: [linus:master] [mm] efa7df3e3b: kernel_BUG_at_include/linux/page_ref.h

Yang Shi
2024-06-04T17:35:17Z

> >
> > I chased it further to:
> >
> > commit 8375ad98cc1defc36adf4a77d9ea1e71db51a371
> > Author: Paul E. McKenney <paulmck@kernel.org>
> > Date: Mon Apr 29 15:06:13 2013 -0700
> >
> > vm: adjust ifdef for TINY_RCU
> > There is an ifdef in page_cache_get_speculative() that checks for !SMP
> > and TREE_RCU, which has been an impossible combination since the advent
> > of TINY_RCU. The ifdef enables a fastpath that is valid when preemption
> > is disabled by rcu_read_lock() in UP systems, which is the case when
> > TINY_RCU is enabled. This commit therefore adjusts the ifdef to
> > generate the fastpath when TINY_RCU is enabled.
> >
> >
> > Where Paul explicitly restored that fastpath for TINY_RCU instead of removing that code.
> >
> > So maybe Paul can comment if that is still worth having. CCing him.
>
> It is currently an atomic operation either way, though the folio_ref_add()
> avoids full ordering, but that is immaterial on x86. Some say that it is
> in the noise on server-class ARMv8 as well, though they have also said
> a great many things in the past. But if that is true, the big benefit
> of the TINY_RCU check is that folio_ref_try_add_rcu() is guaranted not
> to fail in that case (single CPU with preemption disabled). Except that
> everyone has to check the return value anyway, right?
>
> So the usual advice, unsatisfying though it might be, is to remove that
> #ifdef and see if anyone notices.
>
> After all, both 2013 and 2008 were quite some time ago. ;-)

Thanks, Paul.

I will submit a patch to remove the #ifdef as the fix for the bug
report. And do the clean up in a separate patch which is preferred by
David.

>
> Thanx, Paul