On Wed, 30 Mar 2022 08:09:59 -0700
Alexei Starovoitov [off-list ref] wrote:

On Wed, Mar 30, 2022 at 2:15 AM Masami Hiramatsu [off-list ref] wrote:

quoted

On Wed, 30 Mar 2022 13:52:17 +0900
Masami Hiramatsu [off-list ref] wrote:

quoted

On Tue, 29 Mar 2022 18:51:22 -0700
Alexei Starovoitov [off-list ref] wrote:

quoted

On Tue, Mar 29, 2022 at 6:41 PM Jakub Kicinski [off-list ref] wrote:

quoted

On Tue, 29 Mar 2022 16:49:24 -0700 Alexei Starovoitov wrote:

quoted

Hi David, hi Jakub,

The following pull-request contains BPF updates for your *net* tree.

We've added 16 non-merge commits during the last 1 day(s) which contain
a total of 24 files changed, 354 insertions(+), 187 deletions(-).

The main changes are:

1) x86 specific bits of fprobe/rethook, from Masami and Peter.

2) ice/xsk fixes, from Maciej and Magnus.

3) Various small fixes, from Andrii, Yonghong, Geliang and others.

There are some new sparse warnings here that look semi-legit.
As in harmless but not erroneous.

Both are new warnings and not due to these patches, right?

quoted

kernel/trace/rethook.c:68:9: error: incompatible types in comparison expression (different address spaces):
kernel/trace/rethook.c:68:9:    void ( [noderef] __rcu * )( ... )
kernel/trace/rethook.c:68:9:    void ( * )( ... )

66 void rethook_free(struct rethook *rh)
67 {
68         rcu_assign_pointer(rh->handler, NULL);
69
70         call_rcu(&rh->rcu, rethook_free_rcu);
71 }

Looks like this should be a WRITE_ONCE() ?

Masami, please take a look.

Yeah, I think we should make this rcu pointer (and read side must use rcu_dereference())
because this rh->handler becomes the key to disable this rethook.
Let me fix that.

Sorry, please ignore this. Since the handler pointed by rh->handler never
be removed (unless removed by modules, but this will not happen while
the rethook is running), YES, WRITE_ONCE() is enough.
Please add below.

From 92c9c784458f03900823360981812220ce3c7bf3 Mon Sep 17 00:00:00 2001
From: Masami Hiramatsu <mhiramat@kernel.org>
Date: Wed, 30 Mar 2022 18:13:42 +0900
Subject: [PATCH] rethook: Fix to use WRITE_ONCE() for rethook::handler

Since the function pointered by rethook::handler never be removed when
the rethook is alive, it doesn't need to use rcu_assign_pointer() to
update it. Just use WRITE_ONCE().

Reported-by: Alexei Starovoitov <redacted>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Could you please send it as a proper patch so it registers in patchwork?

Sure, I sent the patch. BTW, I marked it as "bpf" instead of "bpf-next",
was that OK? (It seems bpf-next was not updated yet)

Thank you,

quoted

---
 kernel/trace/rethook.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index ab463a4d2b23..b56833700d23 100644
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c

@@ -65,7 +65,7 @@ static void rethook_free_rcu(struct rcu_head *head)
  */
 void rethook_free(struct rethook *rh)
 {
-       rcu_assign_pointer(rh->handler, NULL);
+       WRITE_ONCE(rh->handler, NULL);

        call_rcu(&rh->rcu, rethook_free_rcu);
 }
--

2.25.1
--
Masami Hiramatsu [off-list ref]

-- 
Masami Hiramatsu [off-list ref]