linux-kernel - Re: [PATCH 10/13] rcu/nocb: Delete bypass_timer upon nocb

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: <20210315225633.GA53908@lothringen>
Date:   Mon, 15 Mar 2021 23:56:33 +0100
From:   Frederic Weisbecker <frederic@...nel.org>
To:     Boqun Feng <boqun.feng@...il.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Neeraj Upadhyay <neeraju@...eaurora.org>,
        Josh Triplett <josh@...htriplett.org>,
        Stable <stable@...r.kernel.org>,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH 10/13] rcu/nocb: Delete bypass_timer upon nocb_gp wakeup

On Mon, Mar 15, 2021 at 10:53:36PM +0800, Boqun Feng wrote:
> On Wed, Mar 10, 2021 at 11:17:02PM +0100, Frederic Weisbecker wrote:
> > On Tue, Mar 02, 2021 at 05:24:56PM -0800, Paul E. McKenney wrote:
> > > On Tue, Feb 23, 2021 at 01:10:08AM +0100, Frederic Weisbecker wrote:
> > > > A NOCB-gp wake up can safely delete the nocb_bypass_timer. nocb_gp_wait()
> > > > is going to check again the bypass state and rearm the bypass timer if
> > > > necessary.
> > > > 
> > > > Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> > > > Cc: Josh Triplett <josh@...htriplett.org>
> > > > Cc: Lai Jiangshan <jiangshanlai@...il.com>
> > > > Cc: Joel Fernandes <joel@...lfernandes.org>
> > > > Cc: Neeraj Upadhyay <neeraju@...eaurora.org>
> > > > Cc: Boqun Feng <boqun.feng@...il.com>
> > > 
> > > Give that you delete this code a couple of patches later in this series,
> > > why not just leave it out entirely?  ;-)
> > 
> > It's not exactly deleted later, it's rather merged within the
> > "del_timer(&rdp_gp->nocb_timer)".
> > 
> > The purpose of that patch is to make it clear that we explicitly cancel
> > the nocb_bypass_timer here before we do it implicitly later with the
> > merge of nocb_bypass_timer into nocb_timer.
> > 
> > We could drop that patch, the resulting code in the end of the patchset
> > will be the same of course but the behaviour detail described here might
> > slip out of the reviewers attention :-)
> > 
> 
> How about merging the timers first and adding those small improvements
> later? i.e. move patch #12 #13 right after #7 (IIUC, #7 is the last
> requirement you need for merging timers)

Hmm, nope, patches 9 and 10 are actually preparation work for timers merge.
In fact they could even be skipped and timers could be merged directly but I
wanted the unified behaviour to be fully explicit for reviewers through those
incremental changes before merging the timers together.

>, and then patch #8~#11 just follow

Patch 8 really need to stay where it is because it is an important limitation
on nocb de-offloading that can be removed right after patch 7 (which itself
removes the sole reason for rdp leader to remain nocb) and it doesn't depend
on the timers unification that comes after.

> 
> Just my 2 cents. The overall patchset looks good to me ;-)
> 
> Feel free to add
> 
> Reviewed-by: Boqun Feng <boqun.feng@...il.com>

Thanks a lot for checking that!