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]
Message-ID: <cfe8835fced30f09285004c6e92ece9c073d8948.camel@infradead.org>
Date:   Thu, 09 Dec 2021 18:52:54 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     paulmck@...nel.org, Neeraj Upadhyay <quic_neeraju@...cinc.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        rcu@...r.kernel.org, mimoja@...oja.de, hewenliang4@...wei.com,
        hushiyuan@...wei.com, luolongjun@...wei.com, hejingxian@...wei.com
Subject: Re: [PATCH 02/11] rcu: Kill rnp->ofl_seq and use only
 rcu_state.ofl_lock for exclusion

On Thu, 2021-12-09 at 09:18 -0800, Paul E. McKenney wrote:
> On Thu, Dec 09, 2021 at 03:09:29PM +0000, David Woodhouse wrote:
> > From: David Woodhouse <
> > dwmw@...zon.co.uk
> > >
> > 
> > If we allow architectures to bring APs online in parallel, then we end
> > up requiring rcu_cpu_starting() to be reentrant. But currently, the
> > manipulation of rnp->ofl_seq is not thread-safe.
> > 
> > However, rnp->ofl_seq is also fairly much pointless anyway since both
> > rcu_cpu_starting() and rcu_report_dead() hold rcu_state.ofl_lock for
> > fairly much the whole time that rnp->ofl_seq is set to an odd number
> > to indicate that an operation is in progress.
> > 
> > So drop rnp->ofl_seq completely, and use only rcu_state.ofl_lock.
> > 
> > This has a couple of minor complexities: lockdep will complain when we
> > take rcu_state.ofl_lock, and currently accepts the 'excuse' of having
> > an odd value in rnp->ofl_seq. So switch it to an arch_spinlock_t to
> > avoid that false positive complaint. Since we're killing rnp->ofl_seq
> > of course that 'excuse' has to be changed too, so make it check for
> > arch_spin_is_locked(rcu_state.ofl_lock).
> > 
> > There's no arch_spin_lock_irqsave() so we have to manually save and
> > restore local interrupts around the locking.
> > 
> > At Paul's request, make rcu_gp_init not just wait but *exclude* any
> > CPU online/offline activity, which was fairly much true already by
> > virtue of it holding rcu_state.ofl_lock.
> 
> Looks good!
> 
> Could you please also make the first clause read something like this?
> 
> 	"At Paul's request based on Neeraj's analysis, ..."
> 
> I am going to pull this into -rcu for more intensive testing of this
> code for non-concurrent CPU-online operations (making the above change
> to the commit log).  At some point, rcutorture needs to learn how to
> do concurrent CPU-online operations, but it would be good to bake the
> RCU-specific patches for a bit beforehand.
> 
> Depending on timing, you might wish to send this patch with the
> rest of this group, so:
> 
> Acked-by: Paul E. McKenney <paulmck@...nel.org>
> 
> If testing goes well and if you don't get it there first, I expect
> to push this during the v5.18 merge window.


Thanks.

I think the best option is probably to let all the cleanups and fixes
get merged separately through whatever tree is most appropriate, and
then we can just merge that final patch to actually *enable* parallel
boot for x86 last of all.

I'll address Neeraj's feedback and repost this one.


Download attachment "smime.p7s" of type "application/pkcs7-signature" (5174 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ