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@xxxxxxxxxx> > > > > Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx> > > > > Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx> > > > > Cc: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> > > > > Cc: Neeraj Upadhyay <neeraju@xxxxxxxxxxxxxx> > > > > Cc: Boqun Feng <boqun.feng@xxxxxxxxx> > > > > > > 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@xxxxxxxxx> Thanks a lot for checking that!
