On Mon, Feb 6, 2023 at 8:15 PM Zhang, Qiang1 <qiang1.zhang@xxxxxxxxx> wrote: > > On Mon, Feb 6, 2023 at 12:19 PM Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> wrote: > > > > On Sun, Feb 5, 2023 at 10:09 PM Zhang, Qiang1 <qiang1.zhang@xxxxxxxxx> wrote: > > > > > > > > > >Recent discussion triggered due to a patch linked below, from Qiang, > > > >shed light on the need to accelerate from QS reporting paths. > > > > > > > >Update the comments to capture this piece of knowledge. > > > > > > > >Link: https://lore.kernel.org/all/20230118073014.2020743-1-qiang1.zhang@xxxxxxxxx/ > > > >Cc: Qiang Zhang <Qiang1.zhang@xxxxxxxxx> > > > >Cc: Frederic Weisbecker <frederic@xxxxxxxxxx> > > > >Signed-off-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx> > > > > > > > >--- > > > > kernel/rcu/tree.c | 13 ++++++++++++- > > > > 1 file changed, 12 insertions(+), 1 deletion(-) > > > > > > > >diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > > >index 93eb03f8ed99..713eb6ca6902 100644 > > > >--- a/kernel/rcu/tree.c > > > >+++ b/kernel/rcu/tree.c > > > >@@ -1983,7 +1983,12 @@ rcu_report_qs_rdp(struct rcu_data *rdp) > > > > } else { > > > > /* > > > > * This GP can't end until cpu checks in, so all of our > > > >- * callbacks can be processed during the next GP. > > > >+ * callbacks can be processed during the next GP. Do > > > >+ * the acceleration from here otherwise there may be extra > > > >+ * grace period delays, as any accelerations from rcu_core() > > > > > > > > > Does the extra grace period delays means that if not accelerate callback, > > > the grace period will take more time to end ? or refers to a delay in the > > > start time of a new grace period? > > > > Yes, so IMO it is like this if we don't accelerate: > > 1. Start GP 1 > > 2. CPU1 queues callback C1 (not accelerated yet) > > 3. CPU1 reports QS for GP1 (not accelerating anything). > > 4. GP1 ends > > 5. CPU1's note_gp_changes() is called, accelerate happens, now the CB > > will execute after GP3 (or alternately, rcu_core() on CPU1 does > > accelerate). > > 6. GP2 ends. > > 7. GP3 starts. > > 8. GP3 ends. > > 9. CB is invoked > > > > Instead, what we will get the following thanks to the acceleration here is: > > 1. Start GP 1 > > 2. CPU1 queues callback C1 (not accelerated yet) > > 3. CPU1 reports QS for GP1 and acceleration happens as done by the > > code this patch adds comments for. > > 4. GP1 ends > > 5. CPU1's note_gp_changes() is called > > 6. GP2 ends. > > 7. CB is invoked > > > >Sorry I missed some steps, here is the update: > >1. Start GP 1 > >2. CPU1 queues callback C1 (not accelerated yet) > >3. CPU1 reports QS for GP1 (not accelerating anything). > >4. GP1 ends > >5. GP2 starts for some other reason from some other CPU. > >6. CPU1's note_gp_changes() is called, acceleration happens, now the CB > >will execute after GP3. > >7. GP2 ends. > >8. GP3 starts. > >9. GP3 ends. > >10. CB is invoked > > > >Instead, what we will get the following thanks to the acceleration here is: > >1. Start GP 1 > >2. CPU1 queues callback C1 (not accelerated yet) > >3. CPU1 reports QS for GP1 and acceleration happens as done by the > >code this patch adds comments for. > >4. GP1 ends > >5. GP2 starts > >6. GP2 ends. > >7. CB is invoked > > > >Does that make sense or is there a subtlety I missed? > > > > Thanks for detailed description, that is to say, the grace period delays means that > if there is no acceleration, the invocation of callback may be delayed by one or > more grace periods. > > Can you re-describe the meaning of "grace period delays "in the comments? Yes, good point. I should change it to "one or more delays". Thank you for the suggestion! Sorry for my late reply as I was OOO this week. - Joel > > Thanks > Zqiang > > > > > > >Thanks, > > > > - Joel
