public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Wander Lairson Costa <wander@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"open list:SCHEDULER" <linux-kernel@vger.kernel.org>,
	"open list:TRACING" <linux-trace-kernel@vger.kernel.org>,
	acme@kernel.org, williams@redhat.com, gmonaco@redhat.com
Subject: Re: [PATCH v3 1/4] tracing/preemptirq: Optimize preempt_disable/enable() tracepoint overhead
Date: Wed, 11 Mar 2026 20:35:03 +0100	[thread overview]
Message-ID: <20260311193503.GS606826@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20260311125021.197638-2-wander@redhat.com>

On Wed, Mar 11, 2026 at 09:50:15AM -0300, Wander Lairson Costa wrote:

> +extern void __trace_preempt_on(void);
> +extern void __trace_preempt_off(void);
> +
> +DECLARE_TRACEPOINT(preempt_enable);
> +DECLARE_TRACEPOINT(preempt_disable);
> +
> +#define __preempt_trace_enabled(type, val) \
> +	(tracepoint_enabled(preempt_##type) && preempt_count() == (val))
> +
> +static __always_inline void preempt_count_add(int val)
> +{
> +	__preempt_count_add(val);
> +
> +	if (__preempt_trace_enabled(disable, val))
> +		__trace_preempt_off();
> +}
> +
> +static __always_inline void preempt_count_sub(int val)
> +{
> +	if (__preempt_trace_enabled(enable, val))
> +		__trace_preempt_on();
> +
> +	__preempt_count_sub(val);
> +}
>  #else
>  #define preempt_count_add(val)	__preempt_count_add(val)
>  #define preempt_count_sub(val)	__preempt_count_sub(val)
>  #define preempt_count_dec_and_test() __preempt_count_dec_and_test()
>  #endif
>  
> +#if defined(CONFIG_DEBUG_PREEMPT) || defined(CONFIG_TRACE_PREEMPT_TOGGLE)
> +#define preempt_count_dec_and_test() \
> +	({ preempt_count_sub(1); should_resched(0); })
> +#endif

Why!?!

Why can't you simply have:

static __always_inline bool preempt_count_dec_and_test(void)
{
	if (__preempt_trace_enabled(enable, 1))
		__trace_preempt_on();

	return __preempt_count_dec_and_test();
}

Also, given how !x86 architectures were just complaining about how
terrible their preempt_emable() is, I'm really not liking this much at
all.

Currently the x86 preempt_disable() is _1_ instruction and
preempt_enable() is all of 3. Adding in these tracepoints will bloat
every single such site by at least another 4-5.

That's significant bloat, for really very little gain. Realistically
nobody is going to need these.

  reply	other threads:[~2026-03-11 19:43 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11 12:50 [PATCH v3 0/4] tracing/preemptirq: Optimize disabled tracepoint overhead Wander Lairson Costa
2026-03-11 12:50 ` [PATCH v3 1/4] tracing/preemptirq: Optimize preempt_disable/enable() " Wander Lairson Costa
2026-03-11 19:35   ` Peter Zijlstra [this message]
2026-03-12 17:19     ` Wander Lairson Costa
2026-03-13  9:04       ` Peter Zijlstra
2026-03-13 15:36         ` Wander Lairson Costa
2026-03-11 12:50 ` [PATCH v3 2/4] trace/preemptirq: make TRACE_PREEMPT_TOGGLE user-selectable Wander Lairson Costa
2026-03-11 12:50 ` [PATCH v3 3/4] trace/preemptirq: add TRACE_IRQFLAGS_TOGGLE Wander Lairson Costa
2026-03-11 12:50 ` [PATCH v3 4/4] trace/preemptirq: Implement trace_irqflags hooks Wander Lairson Costa
2026-03-11 19:43   ` Peter Zijlstra
2026-03-11 19:48     ` Steven Rostedt
2026-03-11 19:53       ` Peter Zijlstra
2026-03-11 20:07         ` Steven Rostedt
2026-03-11 20:46           ` Peter Zijlstra
2026-03-11 23:16             ` Steven Rostedt
2026-03-12 17:09     ` Wander Lairson Costa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260311193503.GS606826@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=gmonaco@redhat.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mgorman@suse.de \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=wander@redhat.com \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox