RCU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] rcu: remove surplus instrumentation_end in rcu_nmi_enter
@ 2021-01-11  1:08 Zhouyi Zhou
  2021-01-12 17:50 ` Paul E. McKenney
  0 siblings, 1 reply; 3+ messages in thread
From: Zhouyi Zhou @ 2021-01-11  1:08 UTC (permalink / raw)
  To: 340442286, paulmck, josh, rostedt, mathieu.desnoyers,
	jiangshanlai, joel, rcu, linux-kernel
  Cc: Zhouyi Zhou

In function rcu_nmi_enter, there is a surplus instrumentation_end
in second branch of if statement, although objtool check -f vmlinux.o will
not complain because of its inability to correctly cover all cases
(objtool will visit the third branch first, which markes
 following trace_rcu_dyntick as visited), I think remove the surplus
instrumentation_end will make the code better.


Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
---
 kernel/rcu/tree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 40e5e3dd253e..eaec6f6032c2 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1021,7 +1021,6 @@ noinstr void rcu_nmi_enter(void)
 	} else if (!in_nmi()) {
 		instrumentation_begin();
 		rcu_irq_enter_check_tick();
-		instrumentation_end();
 	} else  {
 		instrumentation_begin();
 	}
-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rcu: remove surplus instrumentation_end in rcu_nmi_enter
  2021-01-11  1:08 [PATCH] rcu: remove surplus instrumentation_end in rcu_nmi_enter Zhouyi Zhou
@ 2021-01-12 17:50 ` Paul E. McKenney
  2021-01-13  3:46   ` Zhouyi Zhou
  0 siblings, 1 reply; 3+ messages in thread
From: Paul E. McKenney @ 2021-01-12 17:50 UTC (permalink / raw)
  To: Zhouyi Zhou
  Cc: 340442286, josh, rostedt, mathieu.desnoyers, jiangshanlai, joel,
	rcu, linux-kernel

On Mon, Jan 11, 2021 at 09:08:59AM +0800, Zhouyi Zhou wrote:
> In function rcu_nmi_enter, there is a surplus instrumentation_end
> in second branch of if statement, although objtool check -f vmlinux.o will
> not complain because of its inability to correctly cover all cases
> (objtool will visit the third branch first, which markes
>  following trace_rcu_dyntick as visited), I think remove the surplus
> instrumentation_end will make the code better.
> 
> 
> Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>

Good catch, applied, thank you!  As usual, I edited a bit, so please
check below to see if I messed anything up.

I did add Neeraj Upadhyay's Reported-by because he noted this, though you
beat him with a public posting, though mostly because I asked him whether
he could make this happen.  I also added the Fixes, but could you please
do that in the future?  You can use "git blame" or either "git log" and
"gitk" with appropriate options to find the offending commit.

							Thanx, Paul

------------------------------------------------------------------------

commit 3f91ff7fd4f76e7eb48bad79666c466bc3530324
Author: Zhouyi Zhou <zhouzhouyi@gmail.com>
Date:   Mon Jan 11 09:08:59 2021 +0800

    rcu: Remove spurious instrumentation_end() in rcu_nmi_enter()
    
    In rcu_nmi_enter(), there is an erroneous instrumentation_end() in the
    second branch of the "if" statement.  Oddly enough, "objtool check -f
    vmlinux.o" fails to complain because it is unable to correctly cover
    all cases.  Instead, objtool visits the third branch first, which marks
    following trace_rcu_dyntick() as visited.  This commit therefore removes
    the spurious instrumentation_end().
    
    Fixes: 04b25a495bd6 ("rcu: Mark rcu_nmi_enter() call to rcu_cleanup_after_idle() noinstr")
    Reported-by Neeraj Upadhyay <neeraju@codeaurora.org>
    Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 63c6dba..e04e336 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1050,7 +1050,6 @@ noinstr void rcu_nmi_enter(void)
 	} else if (!in_nmi()) {
 		instrumentation_begin();
 		rcu_irq_enter_check_tick();
-		instrumentation_end();
 	} else  {
 		instrumentation_begin();
 	}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rcu: remove surplus instrumentation_end in rcu_nmi_enter
  2021-01-12 17:50 ` Paul E. McKenney
@ 2021-01-13  3:46   ` Zhouyi Zhou
  0 siblings, 0 replies; 3+ messages in thread
From: Zhouyi Zhou @ 2021-01-13  3:46 UTC (permalink / raw)
  To: paulmck; +Cc: rcu, neeraju

Thank Paul for your encouragement and valuable advice

I achieve great self improvement in studying and participating Linux RCU

Also thank Neeraj for mutual interest in Linux RCU, your contribution
makes me believe
that even a small enhancement of code can still attract interest and
draw favorable attention
from peers.

Cheers
Zhouyi
On Wed, Jan 13, 2021 at 1:50 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> Good catch, applied, thank you!  As usual, I edited a bit, so please
> check below to see if I messed anything up.
>
> I did add Neeraj Upadhyay's Reported-by because he noted this, though you
> beat him with a public posting, though mostly because I asked him whether
> he could make this happen.  I also added the Fixes, but could you please
> do that in the future?  You can use "git blame" or either "git log" and
> "gitk" with appropriate options to find the offending commit.
>
>                                                         Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 3f91ff7fd4f76e7eb48bad79666c466bc3530324
> Author: Zhouyi Zhou <zhouzhouyi@gmail.com>
> Date:   Mon Jan 11 09:08:59 2021 +0800
>
>     rcu: Remove spurious instrumentation_end() in rcu_nmi_enter()
>
>     In rcu_nmi_enter(), there is an erroneous instrumentation_end() in the
>     second branch of the "if" statement.  Oddly enough, "objtool check -f
>     vmlinux.o" fails to complain because it is unable to correctly cover
>     all cases.  Instead, objtool visits the third branch first, which marks
>     following trace_rcu_dyntick() as visited.  This commit therefore removes
>     the spurious instrumentation_end().
>
>     Fixes: 04b25a495bd6 ("rcu: Mark rcu_nmi_enter() call to rcu_cleanup_after_idle() noinstr")
>     Reported-by Neeraj Upadhyay <neeraju@codeaurora.org>
>     Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
>     Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 63c6dba..e04e336 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1050,7 +1050,6 @@ noinstr void rcu_nmi_enter(void)
>         } else if (!in_nmi()) {
>                 instrumentation_begin();
>                 rcu_irq_enter_check_tick();
> -               instrumentation_end();
>         } else  {
>                 instrumentation_begin();
>         }

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11  1:08 [PATCH] rcu: remove surplus instrumentation_end in rcu_nmi_enter Zhouyi Zhou
2021-01-12 17:50 ` Paul E. McKenney
2021-01-13  3:46   ` Zhouyi Zhou

RCU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/rcu/0 rcu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 rcu rcu/ https://lore.kernel.org/rcu \
		rcu@vger.kernel.org
	public-inbox-index rcu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.rcu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git