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:   Fri, 18 Mar 2022 18:07:50 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Ingo Molnar <mingo@kernel.org>, Will Deacon <will@kernel.org>,
        Waiman Long <longman@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Byungchul Park <byungchul.park@lge.com>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Radoslaw Burny <rburny@google.com>,
        linux-arch <linux-arch@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH 2/2] locking: Apply contention tracepoints in the slow
 path
Message-ID: <20220318180750.744f08d4@gandalf.local.home>
In-Reply-To: <CAM9d7cjUR6shddKM2h9uFXgQf+0F504fnJmKRSfc3+PG3TmEyg@mail.gmail.com>
References: <20220316224548.500123-1-namhyung@kernel.org>
        <20220316224548.500123-3-namhyung@kernel.org>
        <YjSBRNxzaE9c+F/1@boqun-archlinux>
        <YjS2rlezTh9gdlDh@hirez.programming.kicks-ass.net>
        <CAM9d7cjUR6shddKM2h9uFXgQf+0F504fnJmKRSfc3+PG3TmEyg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 18 Mar 2022 14:55:27 -0700
Namhyung Kim <namhyung@kernel.org> wrote:

> > > This looks a littl ugly ;-/ Maybe we can rename __down_common() to
> > > ___down_common() and implement __down_common() as:
> > >
> > >       static inline int __sched __down_common(...)
> > >       {
> > >               int ret;
> > >               trace_contention_begin(sem, 0);
> > >               ret = ___down_common(...);
> > >               trace_contention_end(sem, ret);
> > >               return ret;
> > >       }
> > >
> > > Thoughts?  
> >
> > Yeah, that works, except I think he wants a few extra
> > __set_current_state()'s like so:  
> 
> Not anymore, I decided not to because of noise in the task state.
> 
> Also I'm considering two tracepoints for the return path to reduce
> the buffer size as Mathieu suggested.  Normally it'd return with 0
> so we can ignore it in the contention_end.  For non-zero cases,
> we can add a new tracepoint to save the return value.

I don't think you need two tracepoints, but one that you can override.

We have eprobes that let you create a trace event on top of a current trace
event that can limit or extend what is traced in the buffer.

And I also have custom events that can be placed on top of any existing
tracepoint that has full access to what is sent into the tracepoint on not
just what is available to the trace event:

  https://lore.kernel.org/all/20220312232551.181178712@goodmis.org/

-- Steve