Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Thu, 17 Mar 2022 12:43:05 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To:     rostedt <rostedt@goodmis.org>
Cc:     Namhyung Kim <namhyung@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>, Will Deacon <will@kernel.org>,
        Waiman Long <longman@redhat.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Byungchul Park <byungchul.park@lge.com>,
        paulmck <paulmck@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Radoslaw Burny <rburny@google.com>,
        linux-arch <linux-arch@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>
Message-ID: <1325267700.157591.1647535385417.JavaMail.zimbra@efficios.com>
In-Reply-To: <20220317121055.33812ac1@gandalf.local.home>
References: <20220316224548.500123-1-namhyung@kernel.org> <20220316224548.500123-3-namhyung@kernel.org> <365529974.156362.1647524728813.JavaMail.zimbra@efficios.com> <20220317121055.33812ac1@gandalf.local.home>
Subject: Re: [PATCH 2/2] locking: Apply contention tracepoints in the slow
 path
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: locking: Apply contention tracepoints in the slow path
Thread-Index: XtYNwpD1Kl95+SVBT3UlFooWB7QYrg==
Precedence: bulk

----- On Mar 17, 2022, at 12:10 PM, rostedt rostedt@goodmis.org wrote:

> On Thu, 17 Mar 2022 09:45:28 -0400 (EDT)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
>> > *sem, bool reader)
>> > 		schedule();
>> > 	}
>> > 	__set_current_state(TASK_RUNNING);
>> > +	trace_contention_end(sem, 0);
>> 
>> So for the reader-write locks, and percpu rwlocks, the "trace contention end"
>> will always
>> have ret=0. Likewise for qspinlock, qrwlock, and rtlock. It seems to be a waste
>> of trace
>> buffer space to always have space for a return value that is always 0.
>> 
>> Sorry if I missed prior dicussions of that topic, but why introduce this single
>> "trace contention begin/end" muxer tracepoint with flags rather than
>> per-locking-type
>> tracepoint ? The per-locking-type tracepoint could be tuned to only have the
>> fields
>> that are needed for each locking type.
> 
> per-locking-type tracepoint will also add a bigger footprint.

If you are talking about code and data size footprint in the kernel, yes, we agree
there.

> 
> One extra byte is not an issue.

The implementation uses a 32-bit integer.

But given that this only traces contention, it's probably not as important to
shrink the event size as if it would be for tracing every uncontended lock/unlock.

> This is just the tracepoints. You can still
> attach your own specific LTTng trace events that ignores the zero
> parameter, and can multiplex into specific types of trace events on your
> end.

Indeed, I could, as I do for system call entry/exit tracing. But I suspect it would
not be worth it for contended locks, because I don't expect those events to be frequent
enough in the trace to justify the added code/data footprint, as you pointed out.

> 
> I prefer the current approach as it keeps the tracing footprint down.

Likewise. I just wanted to make sure this was done knowing the trace buffer vs kernel
code/data overhead trade-off.

Thanks,

Mathieu

> 
> -- Steve

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com