1. Patchew
  2. linux
  3. [PATCH v2 0/3] Lazy preemption leftovers
  4. View patch

[PATCH v2 1/3] tracing: Replace TRACE_FLAG_IRQS_NOSUPPORT with its config option.

Sebastian Andrzej Siewior posted 3 patches 1 month ago
[PATCH v2 1/3] tracing: Replace TRACE_FLAG_IRQS_NOSUPPORT with its config option.
Posted by Sebastian Andrzej Siewior 1 month ago 
From: Thomas Gleixner <tglx@linutronix.de>

The TRACE_FLAG_IRQS_NOSUPPORT flag is used by tracing_gen_ctx.*() to
signal that CONFIG_TRACE_IRQFLAGS_SUPPORT is not enabled and tracing IRQ
flags is not supported.

This could be replaced by using the 0 as flags and then deducting that
there is no IRQFLAGS_SUPPORT based on the config option. The downside is
that without CONFIG_TRACE_IRQFLAGS_SUPPORT we can not distinguish
between no-IRQ passed flags and callers which passed 0. On the upside we
have room for one additional flags which could be used for LAZY_PREEMPTION.

Remove TRACE_FLAG_IRQS_NOSUPPORT and set it flags are 0 and
CONFIG_TRACE_IRQFLAGS_SUPPORT is not set.

[bigeasy: Commit descrption.]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/trace_events.h |    7 +++----
 kernel/trace/trace_output.c  |    2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -184,8 +184,7 @@ unsigned int tracing_gen_ctx_irq_test(un
 
 enum trace_flag_type {
 	TRACE_FLAG_IRQS_OFF		= 0x01,
-	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
-	TRACE_FLAG_NEED_RESCHED		= 0x04,
+	TRACE_FLAG_NEED_RESCHED		= 0x02,
 	TRACE_FLAG_HARDIRQ		= 0x08,
 	TRACE_FLAG_SOFTIRQ		= 0x10,
 	TRACE_FLAG_PREEMPT_RESCHED	= 0x20,
@@ -211,11 +210,11 @@ static inline unsigned int tracing_gen_c
 
 static inline unsigned int tracing_gen_ctx_flags(unsigned long irqflags)
 {
-	return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
+	return tracing_gen_ctx_irq_test(0);
 }
 static inline unsigned int tracing_gen_ctx(void)
 {
-	return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
+	return tracing_gen_ctx_irq_test(0);
 }
 #endif
 
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -460,7 +460,7 @@ int trace_print_lat_fmt(struct trace_seq
 		(entry->flags & TRACE_FLAG_IRQS_OFF && bh_off) ? 'D' :
 		(entry->flags & TRACE_FLAG_IRQS_OFF) ? 'd' :
 		bh_off ? 'b' :
-		(entry->flags & TRACE_FLAG_IRQS_NOSUPPORT) ? 'X' :
+		!IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' :
 		'.';
 
 	switch (entry->flags & (TRACE_FLAG_NEED_RESCHED |
Re: [PATCH v2 1/3] tracing: Replace TRACE_FLAG_IRQS_NOSUPPORT with its config option.
Posted by Steven Rostedt 1 month ago 
On Mon, 21 Oct 2024 17:08:40 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> From: Thomas Gleixner <tglx@linutronix.de>
> 
> The TRACE_FLAG_IRQS_NOSUPPORT flag is used by tracing_gen_ctx.*() to
> signal that CONFIG_TRACE_IRQFLAGS_SUPPORT is not enabled and tracing IRQ
> flags is not supported.
> 
> This could be replaced by using the 0 as flags and then deducting that
> there is no IRQFLAGS_SUPPORT based on the config option. The downside is
> that without CONFIG_TRACE_IRQFLAGS_SUPPORT we can not distinguish
> between no-IRQ passed flags and callers which passed 0. On the upside we
> have room for one additional flags which could be used for LAZY_PREEMPTION.
> 
> Remove TRACE_FLAG_IRQS_NOSUPPORT and set it flags are 0 and
> CONFIG_TRACE_IRQFLAGS_SUPPORT is not set.

We could also add that we have:

#
# Minimum requirements an architecture has to meet for us to
# be able to offer generic tracing facilities:
#
config TRACING_SUPPORT
        bool
        depends on TRACE_IRQFLAGS_SUPPORT
        depends on STACKTRACE_SUPPORT
        default y

So this can't even be built without TRACE_IRQFLAGS_SUPPORT!

> 
> [bigeasy: Commit descrption.]
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/trace_events.h |    7 +++----
>  kernel/trace/trace_output.c  |    2 +-
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -184,8 +184,7 @@ unsigned int tracing_gen_ctx_irq_test(un
>  
>  enum trace_flag_type {
>  	TRACE_FLAG_IRQS_OFF		= 0x01,
> -	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
> -	TRACE_FLAG_NEED_RESCHED		= 0x04,
> +	TRACE_FLAG_NEED_RESCHED		= 0x02,

These flags are user visible (I probably should move them into uapi).
They are parsed by libtraceevent.

Please just remove NOSUPPORT and do not touch NEED_RESCHED.


>  	TRACE_FLAG_HARDIRQ		= 0x08,
>  	TRACE_FLAG_SOFTIRQ		= 0x10,
>  	TRACE_FLAG_PREEMPT_RESCHED	= 0x20,
> @@ -211,11 +210,11 @@ static inline unsigned int tracing_gen_c
>  
>  static inline unsigned int tracing_gen_ctx_flags(unsigned long irqflags)
>  {
> -	return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
> +	return tracing_gen_ctx_irq_test(0);
>  }
>  static inline unsigned int tracing_gen_ctx(void)
>  {
> -	return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
> +	return tracing_gen_ctx_irq_test(0);
>  }
>  #endif
>  
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -460,7 +460,7 @@ int trace_print_lat_fmt(struct trace_seq
>  		(entry->flags & TRACE_FLAG_IRQS_OFF && bh_off) ? 'D' :
>  		(entry->flags & TRACE_FLAG_IRQS_OFF) ? 'd' :
>  		bh_off ? 'b' :
> -		(entry->flags & TRACE_FLAG_IRQS_NOSUPPORT) ? 'X' :
> +		!IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' :

Probably can even remove this check.

-- Steve

>  		'.';
>  
>  	switch (entry->flags & (TRACE_FLAG_NEED_RESCHED |
Re: [PATCH v2 1/3] tracing: Replace TRACE_FLAG_IRQS_NOSUPPORT with its config option.
Posted by Sebastian Andrzej Siewior 1 month ago 
On 2024-10-22 03:14:18 [-0400], Steven Rostedt wrote:
> On Mon, 21 Oct 2024 17:08:40 +0200
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
> > From: Thomas Gleixner <tglx@linutronix.de>
> > 
> > The TRACE_FLAG_IRQS_NOSUPPORT flag is used by tracing_gen_ctx.*() to
> > signal that CONFIG_TRACE_IRQFLAGS_SUPPORT is not enabled and tracing IRQ
> > flags is not supported.
> > 
> > This could be replaced by using the 0 as flags and then deducting that
> > there is no IRQFLAGS_SUPPORT based on the config option. The downside is
> > that without CONFIG_TRACE_IRQFLAGS_SUPPORT we can not distinguish
> > between no-IRQ passed flags and callers which passed 0. On the upside we
> > have room for one additional flags which could be used for LAZY_PREEMPTION.
> > 
> > Remove TRACE_FLAG_IRQS_NOSUPPORT and set it flags are 0 and
> > CONFIG_TRACE_IRQFLAGS_SUPPORT is not set.
> 
> We could also add that we have:
> 
> #
> # Minimum requirements an architecture has to meet for us to
> # be able to offer generic tracing facilities:
> #
> config TRACING_SUPPORT
>         bool
>         depends on TRACE_IRQFLAGS_SUPPORT
>         depends on STACKTRACE_SUPPORT
>         default y
> 
> So this can't even be built without TRACE_IRQFLAGS_SUPPORT!

Good point. So we could TRACE_FLAG_IRQS_NOSUPPORT since it can't be
used. This is since commit 0ea5ee035133a ("tracing: Remove PPC32 wart
from config TRACING_SUPPORT").

> > +++ b/include/linux/trace_events.h
> > @@ -184,8 +184,7 @@ unsigned int tracing_gen_ctx_irq_test(un
> >  
> >  enum trace_flag_type {
> >  	TRACE_FLAG_IRQS_OFF		= 0x01,
> > -	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
> > -	TRACE_FLAG_NEED_RESCHED		= 0x04,
> > +	TRACE_FLAG_NEED_RESCHED		= 0x02,
> 
> These flags are user visible (I probably should move them into uapi).
> They are parsed by libtraceevent.
> 
> Please just remove NOSUPPORT and do not touch NEED_RESCHED.

Then I put the lazy bit where we have not NOSUPPORT.

> >  	TRACE_FLAG_HARDIRQ		= 0x08,
> >  	TRACE_FLAG_SOFTIRQ		= 0x10,
> >  	TRACE_FLAG_PREEMPT_RESCHED	= 0x20,
> > --- a/kernel/trace/trace_output.c
> > +++ b/kernel/trace/trace_output.c
> > @@ -460,7 +460,7 @@ int trace_print_lat_fmt(struct trace_seq
> >  		(entry->flags & TRACE_FLAG_IRQS_OFF && bh_off) ? 'D' :
> >  		(entry->flags & TRACE_FLAG_IRQS_OFF) ? 'd' :
> >  		bh_off ? 'b' :
> > -		(entry->flags & TRACE_FLAG_IRQS_NOSUPPORT) ? 'X' :
> > +		!IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' :
> 
> Probably can even remove this check.

Yes.

> -- Steve

Sebastian
Re: [PATCH v2 1/3] tracing: Replace TRACE_FLAG_IRQS_NOSUPPORT with its config option.
Posted by Steven Rostedt 1 month ago 
On Tue, 22 Oct 2024 11:52:41 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> > 
> > Please just remove NOSUPPORT and do not touch NEED_RESCHED.  
> 
> Then I put the lazy bit where we have not NOSUPPORT.

I'm afraid user space will confuse this with the NOSUPPORT.

-- Steve
Re: [PATCH v2 1/3] tracing: Replace TRACE_FLAG_IRQS_NOSUPPORT with its config option.
Posted by Sebastian Andrzej Siewior 1 month ago 
On 2024-10-22 06:19:59 [-0400], Steven Rostedt wrote:
> On Tue, 22 Oct 2024 11:52:41 +0200
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
> > > 
> > > Please just remove NOSUPPORT and do not touch NEED_RESCHED.  
> > 
> > Then I put the lazy bit where we have not NOSUPPORT.
> 
> I'm afraid user space will confuse this with the NOSUPPORT.

So? We don't ignore this for now and recycle that bit? If I got it
right, only PPC32 was using that NOSUPPORT bit.
What are the options given that it has to be an 8 bit field?

> -- Steve

Sebastian
[PATCH] tracing: Remove TRACE_FLAG_IRQS_NOSUPPORT
Posted by Sebastian Andrzej Siewior 1 month ago 
It was possible to enable tracing with no IRQ tracing support. The
tracing infrastructure would then record TRACE_FLAG_IRQS_NOSUPPORT as
the only tracing flag and show an 'X' in the output.

The last user of this feature was PPC32 which managed to implement it
during PowerPC merge in 2009. Since then, it was unused and the PPC32
dependency was finally removed in commit 0ea5ee035133a ("tracing: Remove
PPC32 wart from config TRACING_SUPPORT").
Since the PowerPC merge the code behind !CONFIG_TRACE_IRQFLAGS_SUPPORT
with TRACING enabled can no longer be selected used and the 'X' is not
displayed or recorded.

Remove the CONFIG_TRACE_IRQFLAGS_SUPPORT from the tracing code. Remove
TRACE_FLAG_IRQS_NOSUPPORT.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
On 2024-10-22 03:14:18 [-0400], Steven Rostedt wrote:
> So this can't even be built without TRACE_IRQFLAGS_SUPPORT!

What about this as a first step? This bit was not used since 2009.

 Documentation/trace/ftrace.rst |  3 ---
 include/linux/trace_events.h   | 13 -------------
 kernel/trace/trace_output.c    |  1 -
 3 files changed, 17 deletions(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index 4073ca48af4ad..74d5bd801b1a8 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -1031,9 +1031,6 @@ explains which is which.
   CPU#: The CPU which the process was running on.
 
   irqs-off: 'd' interrupts are disabled. '.' otherwise.
-	.. caution:: If the architecture does not support a way to
-		read the irq flags variable, an 'X' will always
-		be printed here.
 
   need-resched:
 	- 'N' both TIF_NEED_RESCHED and PREEMPT_NEED_RESCHED is set,
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 42bedcddd5113..467dadb990cbe 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -184,7 +184,6 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status);
 
 enum trace_flag_type {
 	TRACE_FLAG_IRQS_OFF		= 0x01,
-	TRACE_FLAG_IRQS_NOSUPPORT	= 0x02,
 	TRACE_FLAG_NEED_RESCHED		= 0x04,
 	TRACE_FLAG_HARDIRQ		= 0x08,
 	TRACE_FLAG_SOFTIRQ		= 0x10,
@@ -193,7 +192,6 @@ enum trace_flag_type {
 	TRACE_FLAG_BH_OFF		= 0x80,
 };
 
-#ifdef CONFIG_TRACE_IRQFLAGS_SUPPORT
 static inline unsigned int tracing_gen_ctx_flags(unsigned long irqflags)
 {
 	unsigned int irq_status = irqs_disabled_flags(irqflags) ?
@@ -207,17 +205,6 @@ static inline unsigned int tracing_gen_ctx(void)
 	local_save_flags(irqflags);
 	return tracing_gen_ctx_flags(irqflags);
 }
-#else
-
-static inline unsigned int tracing_gen_ctx_flags(unsigned long irqflags)
-{
-	return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
-}
-static inline unsigned int tracing_gen_ctx(void)
-{
-	return tracing_gen_ctx_irq_test(TRACE_FLAG_IRQS_NOSUPPORT);
-}
-#endif
 
 static inline unsigned int tracing_gen_ctx_dec(void)
 {
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 868f2f912f280..2ee6613dce6dc 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -460,7 +460,6 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry)
 		(entry->flags & TRACE_FLAG_IRQS_OFF && bh_off) ? 'D' :
 		(entry->flags & TRACE_FLAG_IRQS_OFF) ? 'd' :
 		bh_off ? 'b' :
-		(entry->flags & TRACE_FLAG_IRQS_NOSUPPORT) ? 'X' :
 		'.';
 
 	switch (entry->flags & (TRACE_FLAG_NEED_RESCHED |
-- 
2.45.2

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.