Re: [PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Paul E. McKenney]

On Mon, May 14, 2018 at 06:01:31PM -0700, Joel Fernandes wrote:
> On Mon, May 14, 2018 at 12:20:28PM -0700, Paul E. McKenney wrote:
> > On Sun, May 13, 2018 at 08:15:40PM -0700, Joel Fernandes (Google) wrote:
> > > Currently the tree RCU clean up code records a CleanupMore trace event
> > > even if the GP was already in progress. This makes CleanupMore show up
> > > twice for no reason. Avoid it.
> > > 
> > > Signed-off-by: Joel Fernandes (Google) 
> > 
> > Good catch, and I applied this patch.  I did rework the commit log
> > a bit, so please look it over to make sure I didn't mess it up.
> > 
> > Thanx, Paul
> > 
> > 
> > 
> > commit 52c4e689efd975f5383895b1bc1b91bc90fdd372
> > Author: Joel Fernandes (Google) 
> > Date:   Sun May 13 20:15:40 2018 -0700
> > 
> > rcu: Produce last "CleanupMore" trace only if late-breaking request
> > 
> > Currently the tree RCU clean-up code records a "CleanupMore" trace
> > event in response to late-arriving grace-period requests even if the
> > grace period was already requested. This makes "CleanupMore" show up an
> > extra time (in addition to once for each rcu_node structure that was
> > previously marked with the request) for no good reason.  This commit
> > therefore avoids emitting this trace message unless the only if the only
> > request for this next grace period arrived during or after the cleanup
> > scan of the rcu_node structures.
> 
> Yes, this is fine except "unless the only if the only" should be "unless the".

I did update this after sending, and I still have "the the".  Will fix on
next rebase.  As my daughter recently reminded me, the Law of Conservation
of Bugs.  ;-)

Thanx, Paul

> thanks,
> 
> - Joel
> 
> > 
> > Signed-off-by: Joel Fernandes (Google) 
> > Signed-off-by: Paul E. McKenney 
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 8063a0478870..de6447dd73de 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2072,7 +2072,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
> > rsp->gp_state = RCU_GP_IDLE;
> > /* Check for GP requests since above loop. */
> > rdp = this_cpu_ptr(rsp->rda);
> > -   if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
> > +   if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
> > trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
> >   TPS("CleanupMore"));
> > needgp = true;
> > 
> 

Re: [PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Paul E. McKenney]

On Mon, May 14, 2018 at 06:01:31PM -0700, Joel Fernandes wrote:
> On Mon, May 14, 2018 at 12:20:28PM -0700, Paul E. McKenney wrote:
> > On Sun, May 13, 2018 at 08:15:40PM -0700, Joel Fernandes (Google) wrote:
> > > Currently the tree RCU clean up code records a CleanupMore trace event
> > > even if the GP was already in progress. This makes CleanupMore show up
> > > twice for no reason. Avoid it.
> > > 
> > > Signed-off-by: Joel Fernandes (Google) 
> > 
> > Good catch, and I applied this patch.  I did rework the commit log
> > a bit, so please look it over to make sure I didn't mess it up.
> > 
> > Thanx, Paul
> > 
> > 
> > 
> > commit 52c4e689efd975f5383895b1bc1b91bc90fdd372
> > Author: Joel Fernandes (Google) 
> > Date:   Sun May 13 20:15:40 2018 -0700
> > 
> > rcu: Produce last "CleanupMore" trace only if late-breaking request
> > 
> > Currently the tree RCU clean-up code records a "CleanupMore" trace
> > event in response to late-arriving grace-period requests even if the
> > grace period was already requested. This makes "CleanupMore" show up an
> > extra time (in addition to once for each rcu_node structure that was
> > previously marked with the request) for no good reason.  This commit
> > therefore avoids emitting this trace message unless the only if the only
> > request for this next grace period arrived during or after the cleanup
> > scan of the rcu_node structures.
> 
> Yes, this is fine except "unless the only if the only" should be "unless the".

I did update this after sending, and I still have "the the".  Will fix on
next rebase.  As my daughter recently reminded me, the Law of Conservation
of Bugs.  ;-)

Thanx, Paul

> thanks,
> 
> - Joel
> 
> > 
> > Signed-off-by: Joel Fernandes (Google) 
> > Signed-off-by: Paul E. McKenney 
> > 
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 8063a0478870..de6447dd73de 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2072,7 +2072,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
> > rsp->gp_state = RCU_GP_IDLE;
> > /* Check for GP requests since above loop. */
> > rdp = this_cpu_ptr(rsp->rda);
> > -   if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
> > +   if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
> > trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
> >   TPS("CleanupMore"));
> > needgp = true;
> > 
> 

Re: [PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Joel Fernandes]

On Mon, May 14, 2018 at 12:20:28PM -0700, Paul E. McKenney wrote:
> On Sun, May 13, 2018 at 08:15:40PM -0700, Joel Fernandes (Google) wrote:
> > Currently the tree RCU clean up code records a CleanupMore trace event
> > even if the GP was already in progress. This makes CleanupMore show up
> > twice for no reason. Avoid it.
> > 
> > Signed-off-by: Joel Fernandes (Google) 
> 
> Good catch, and I applied this patch.  I did rework the commit log
> a bit, so please look it over to make sure I didn't mess it up.
> 
>   Thanx, Paul
> 
> 
> 
> commit 52c4e689efd975f5383895b1bc1b91bc90fdd372
> Author: Joel Fernandes (Google) 
> Date:   Sun May 13 20:15:40 2018 -0700
> 
> rcu: Produce last "CleanupMore" trace only if late-breaking request
> 
> Currently the tree RCU clean-up code records a "CleanupMore" trace
> event in response to late-arriving grace-period requests even if the
> grace period was already requested. This makes "CleanupMore" show up an
> extra time (in addition to once for each rcu_node structure that was
> previously marked with the request) for no good reason.  This commit
> therefore avoids emitting this trace message unless the only if the only
> request for this next grace period arrived during or after the cleanup
> scan of the rcu_node structures.

Yes, this is fine except "unless the only if the only" should be "unless the".

thanks,

- Joel

> 
> Signed-off-by: Joel Fernandes (Google) 
> Signed-off-by: Paul E. McKenney 
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 8063a0478870..de6447dd73de 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2072,7 +2072,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
>   rsp->gp_state = RCU_GP_IDLE;
>   /* Check for GP requests since above loop. */
>   rdp = this_cpu_ptr(rsp->rda);
> - if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
> + if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
>   trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
> TPS("CleanupMore"));
>   needgp = true;
> 

Re: [PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Joel Fernandes]

On Mon, May 14, 2018 at 12:20:28PM -0700, Paul E. McKenney wrote:
> On Sun, May 13, 2018 at 08:15:40PM -0700, Joel Fernandes (Google) wrote:
> > Currently the tree RCU clean up code records a CleanupMore trace event
> > even if the GP was already in progress. This makes CleanupMore show up
> > twice for no reason. Avoid it.
> > 
> > Signed-off-by: Joel Fernandes (Google) 
> 
> Good catch, and I applied this patch.  I did rework the commit log
> a bit, so please look it over to make sure I didn't mess it up.
> 
>   Thanx, Paul
> 
> 
> 
> commit 52c4e689efd975f5383895b1bc1b91bc90fdd372
> Author: Joel Fernandes (Google) 
> Date:   Sun May 13 20:15:40 2018 -0700
> 
> rcu: Produce last "CleanupMore" trace only if late-breaking request
> 
> Currently the tree RCU clean-up code records a "CleanupMore" trace
> event in response to late-arriving grace-period requests even if the
> grace period was already requested. This makes "CleanupMore" show up an
> extra time (in addition to once for each rcu_node structure that was
> previously marked with the request) for no good reason.  This commit
> therefore avoids emitting this trace message unless the only if the only
> request for this next grace period arrived during or after the cleanup
> scan of the rcu_node structures.

Yes, this is fine except "unless the only if the only" should be "unless the".

thanks,

- Joel

> 
> Signed-off-by: Joel Fernandes (Google) 
> Signed-off-by: Paul E. McKenney 
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 8063a0478870..de6447dd73de 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2072,7 +2072,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
>   rsp->gp_state = RCU_GP_IDLE;
>   /* Check for GP requests since above loop. */
>   rdp = this_cpu_ptr(rsp->rda);
> - if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
> + if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
>   trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
> TPS("CleanupMore"));
>   needgp = true;
> 

Re: [PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Paul E. McKenney]

On Sun, May 13, 2018 at 08:15:40PM -0700, Joel Fernandes (Google) wrote:
> Currently the tree RCU clean up code records a CleanupMore trace event
> even if the GP was already in progress. This makes CleanupMore show up
> twice for no reason. Avoid it.
> 
> Signed-off-by: Joel Fernandes (Google) 

Good catch, and I applied this patch.  I did rework the commit log
a bit, so please look it over to make sure I didn't mess it up.

Thanx, Paul



commit 52c4e689efd975f5383895b1bc1b91bc90fdd372
Author: Joel Fernandes (Google) 
Date:   Sun May 13 20:15:40 2018 -0700

rcu: Produce last "CleanupMore" trace only if late-breaking request

Currently the tree RCU clean-up code records a "CleanupMore" trace
event in response to late-arriving grace-period requests even if the
grace period was already requested. This makes "CleanupMore" show up an
extra time (in addition to once for each rcu_node structure that was
previously marked with the request) for no good reason.  This commit
therefore avoids emitting this trace message unless the only if the only
request for this next grace period arrived during or after the cleanup
scan of the rcu_node structures.

Signed-off-by: Joel Fernandes (Google) 
Signed-off-by: Paul E. McKenney 

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8063a0478870..de6447dd73de 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2072,7 +2072,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
rsp->gp_state = RCU_GP_IDLE;
/* Check for GP requests since above loop. */
rdp = this_cpu_ptr(rsp->rda);
-   if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
+   if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
  TPS("CleanupMore"));
needgp = true;

Re: [PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Paul E. McKenney]

On Sun, May 13, 2018 at 08:15:40PM -0700, Joel Fernandes (Google) wrote:
> Currently the tree RCU clean up code records a CleanupMore trace event
> even if the GP was already in progress. This makes CleanupMore show up
> twice for no reason. Avoid it.
> 
> Signed-off-by: Joel Fernandes (Google) 

Good catch, and I applied this patch.  I did rework the commit log
a bit, so please look it over to make sure I didn't mess it up.

Thanx, Paul



commit 52c4e689efd975f5383895b1bc1b91bc90fdd372
Author: Joel Fernandes (Google) 
Date:   Sun May 13 20:15:40 2018 -0700

rcu: Produce last "CleanupMore" trace only if late-breaking request

Currently the tree RCU clean-up code records a "CleanupMore" trace
event in response to late-arriving grace-period requests even if the
grace period was already requested. This makes "CleanupMore" show up an
extra time (in addition to once for each rcu_node structure that was
previously marked with the request) for no good reason.  This commit
therefore avoids emitting this trace message unless the only if the only
request for this next grace period arrived during or after the cleanup
scan of the rcu_node structures.

Signed-off-by: Joel Fernandes (Google) 
Signed-off-by: Paul E. McKenney 

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8063a0478870..de6447dd73de 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2072,7 +2072,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
rsp->gp_state = RCU_GP_IDLE;
/* Check for GP requests since above loop. */
rdp = this_cpu_ptr(rsp->rda);
-   if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
+   if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
  TPS("CleanupMore"));
needgp = true;

[PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Joel Fernandes (Google)]

Currently the tree RCU clean up code records a CleanupMore trace event
even if the GP was already in progress. This makes CleanupMore show up
twice for no reason. Avoid it.

Signed-off-by: Joel Fernandes (Google) 
---
 kernel/rcu/tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8401a253e7de..25c44328d071 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2083,7 +2083,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
rsp->gp_state = RCU_GP_IDLE;
/* Check for GP requests since above loop. */
rdp = this_cpu_ptr(rsp->rda);
-   if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
+   if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
  TPS("CleanupMore"));
needgp = true;
-- 
2.17.0.441.gb46fe60e1d-goog

[PATCH RFC 7/8] rcu: trace CleanupMore condition only if needed

[Joel Fernandes (Google)]

Currently the tree RCU clean up code records a CleanupMore trace event
even if the GP was already in progress. This makes CleanupMore show up
twice for no reason. Avoid it.

Signed-off-by: Joel Fernandes (Google) 
---
 kernel/rcu/tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8401a253e7de..25c44328d071 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2083,7 +2083,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
rsp->gp_state = RCU_GP_IDLE;
/* Check for GP requests since above loop. */
rdp = this_cpu_ptr(rsp->rda);
-   if (ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
+   if (!needgp && ULONG_CMP_LT(rnp->gp_seq, rnp->gp_seq_needed)) {
trace_rcu_this_gp(rnp, rdp, rnp->gp_seq_needed,
  TPS("CleanupMore"));
needgp = true;
-- 
2.17.0.441.gb46fe60e1d-goog