Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 5 Oct 2021 16:02:52 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Jan Engelhardt <jengelh@inai.de>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Paul <paulmck@linux.vnet.ibm.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        "Joel Fernandes, Google" <joel@joelfernandes.org>,
        Pablo Neira Ayuso <pablo@netfilter.org>,
        Jozsef Kadlecsik <kadlec@netfilter.org>,
        Florian Westphal <fw@strlen.de>,
        "David S. Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>, rcu <rcu@vger.kernel.org>,
        netfilter-devel <netfilter-devel@vger.kernel.org>,
        coreteam <coreteam@netfilter.org>,
        netdev <netdev@vger.kernel.org>
Subject: Re: [RFC][PATCH] rcu: Use typeof(p) instead of typeof(*p) *
Message-ID: <20211005160252.54640350@gandalf.local.home>
In-Reply-To: <CAHk-=wiL+wyCOTedh48Oz0cNOKJq2GNwtxg6423hf-1FuGrv_A@mail.gmail.com>
References: <20211005094728.203ecef2@gandalf.local.home>
        <ef5b1654-1f75-da82-cab8-248319efbe3f@rasmusvillemoes.dk>
        <639278914.2878.1633457192964.JavaMail.zimbra@efficios.com>
        <826o327o-3r46-3oop-r430-8qr0ssp537o3@vanv.qr>
        <20211005144002.34008ea0@gandalf.local.home>
        <srqsppq-p657-43qq-np31-pq5pp03271r6@vanv.qr>
        <20211005154029.46f9c596@gandalf.local.home>
        <CAHk-=wiL+wyCOTedh48Oz0cNOKJq2GNwtxg6423hf-1FuGrv_A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 5 Oct 2021 12:46:43 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Oct 5, 2021 at 12:40 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > I may try it, because exposing the structure I want to hide, is pulling out
> > a lot of other crap with it :-p  
> 
> One option is just "don't do rcu_access of a pointer that you're not
> supposed to touch in a file that isn't supposed to touch it".

The problem is, the RCU isn't for touching it, it is for knowing it exists.

> 
> IOW, why are you doing that
> 
>      pid_list = rcu_dereference_sched(tr->function_pids);
> 
> in a place that isn't supposed to look at the pid_list in the first place?
> 
> Yeah, yeah, I see how you just pass it to trace_ignore_this_task() as
> an argument, but maybe the real fix is to just pass that trace_array
> pointer instead?
> 
> IOW, if you want to keep that structure private, maybe you really just
> shouldn't have non-private users of it randomly doing RCU lookups of
> it?
>

Ideally, I wanted to keep the logic of the pid lists separate, and not have
it know about the trace array at all.

And this was the best "incremental" approach I had, as the code is
currently all just open coded.

The RCU lookups are not an internal use functionality of the pid lists. The
updates to the pid list are done by allocating a new pid_list, copying the
old pid_list with the new updates and then swapping the pointers. The logic
of the pid_list is orthogonal to the RCU update. It's just "allocate some
random thing" and use RCU to swap it with the old random thing.

That is, the logic to synchronize updates is left to the user not the pid
list itself.

I also want to limit function calls, as this is called from the function
tracer (every function being traced) and if the pid_list pointer is NULL it
skips it. Hence, I want to avoid calling a function to know if the pointer
is NULL or not.

-- Steve