Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 6 Jul 2023 09:56:24 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc:     Jiri Olsa <jolsa@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
        lkml <linux-kernel@vger.kernel.org>,
        linux-trace-kernel@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH] fprobe: Ensure running fprobe_exit_handler() finished
 before calling rethook_free()
Message-ID: <20230706095624.3a846b8e@gandalf.local.home>
In-Reply-To: <20230706141012.c1a0ae0901e0fdec7b3078c7@kernel.org>
References: <20230628012305.978e34d44f1a53fe20327fde@kernel.org>
        <168796344232.46347.7947681068822514750.stgit@devnote2>
        <20230705212657.5968daf7@gandalf.local.home>
        <20230706141012.c1a0ae0901e0fdec7b3078c7@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 6 Jul 2023 14:10:12 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> With only Jiri's patch, following flow can happen;
> 
> ------
>  CPU1                              CPU2
>  call unregister_fprobe()
>  ...
>                                    __fprobe_handler()
>                                    rethook_hook() on probed function
>  unregister_ftrace_function()
>                                    return from probed function
>                                    rethook hooks
>                                    find rh->handler == fprobe_exit_handler
>                                    call fprobe_exit_handler()
>  rethook_free():
>    set rh->handler = NULL;
>  return from unreigster_fprobe;
>                                    call fp->exit_handler() <- (*)
> 
> (*) In this point, the exit handler is called after returning from 
> unregister_fprobe().
> ------
> 
> So, this patch changes it as following;
> ------
>  CPU1                              CPU2
>  call unregister_fprobe()
>  ...
>  rethook_stop():
>    set rh->handler = NULL;
>                                    __fprobe_handler()
>                                    rethook_hook() on probed function
>  unregister_ftrace_function()
>                                    return from probed function
>                                    rethook hooks
>                                    find rh->handler == NULL
>                                    return from rethook
>  rethook_free()
>  return from unreigster_fprobe;
> ------
> 
> I can also just put a synchronize_sched_rcu() right after rethook_free()
> to wait for all running fprobe_exit_handler() too.
> 

This makes more sense. Can you please add the above to the change log.

Thanks,

-- Steve