lkml.org 
[lkml]   [2021]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 10/13] rcu/nocb: Delete bypass_timer upon nocb_gp wakeup
On Mon, Mar 15, 2021 at 11:56:33PM +0100, Frederic Weisbecker wrote:
> 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@kernel.org>
> > > > > Cc: Josh Triplett <josh@joshtriplett.org>
> > > > > Cc: Lai Jiangshan <jiangshanlai@gmail.com>
> > > > > Cc: Joel Fernandes <joel@joelfernandes.org>
> > > > > Cc: Neeraj Upadhyay <neeraju@codeaurora.org>
> > > > > Cc: Boqun Feng <boqun.feng@gmail.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@gmail.com>
>
> Thanks a lot for checking that!

Applied to 10/13, thank you both!

Thanx, Paul

\
 
 \ /
  Last update: 2021-03-16 01:03    [W:0.097 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site