linux-kernel - Re: [linus:master] [mm] efa7df3e3b: kernel_BUG_at_include/linux/page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 4 Jun 2024 10:35:04 -0700
From: Yang Shi <shy828301@...il.com>
To: paulmck@...nel.org
Cc: David Hildenbrand <david@...hat.com>, Peter Xu <peterx@...hat.com>, 
	kernel test robot <oliver.sang@...el.com>, Jason Gunthorpe <jgg@...dia.com>, 
	Vivek Kasireddy <vivek.kasireddy@...el.com>, Rik van Riel <riel@...riel.com>, oe-lkp@...ts.linux.dev, 
	lkp@...el.com, linux-kernel@...r.kernel.org, 
	Andrew Morton <akpm@...ux-foundation.org>, Matthew Wilcox <willy@...radead.org>, 
	Christopher Lameter <cl@...ux.com>, linux-mm@...ck.org
Subject: Re: [linus:master] [mm] efa7df3e3b: kernel_BUG_at_include/linux/page_ref.h

> >
> > I chased it further to:
> >
> > commit 8375ad98cc1defc36adf4a77d9ea1e71db51a371
> > Author: Paul E. McKenney <paulmck@...nel.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