| Message ID | 20241009105709.887510-3-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Lazy preemption leftovers | expand |
On Wed, 9 Oct 2024 12:50:56 +0200 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > The scheduler added NEED_RESCHED_LAZY scheduling. Record this state as > part of trace flags and expose it in the need_resched field. > > Record and expose NEED_RESCHED_LAZY. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org> -- Steve > --- > include/linux/trace_events.h | 1 + > kernel/trace/trace.c | 2 ++ > kernel/trace/trace_output.c | 14 +++++++++++++- > 3 files changed, 16 insertions(+), 1 deletion(-)
Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes: > The scheduler added NEED_RESCHED_LAZY scheduling. Record this state as > part of trace flags and expose it in the need_resched field. > > Record and expose NEED_RESCHED_LAZY. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > include/linux/trace_events.h | 1 + > kernel/trace/trace.c | 2 ++ > kernel/trace/trace_output.c | 14 +++++++++++++- > 3 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > index d5c0fcf20f024..4cae6f2581379 100644 > --- a/include/linux/trace_events.h > +++ b/include/linux/trace_events.h > @@ -185,6 +185,7 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status); > enum trace_flag_type { > TRACE_FLAG_IRQS_OFF = 0x01, > TRACE_FLAG_NEED_RESCHED = 0x02, > + TRACE_FLAG_NEED_RESCHED_LAZY = 0x04, > TRACE_FLAG_HARDIRQ = 0x08, > TRACE_FLAG_SOFTIRQ = 0x10, > TRACE_FLAG_PREEMPT_RESCHED = 0x20, > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 1c69ca1f10886..29d7703751aa9 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) > trace_flags |= TRACE_FLAG_NEED_RESCHED; > if (test_preempt_need_resched()) > trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; > + if (tif_test_bit(TIF_NEED_RESCHED_LAZY)) TIF_NEED_RESCHED_LAZY falls back to TIF_NEED_RESCHED without CONFIG_ARCH_HAS_PREEMPT_LAZY. So, you might need to add an explicit check for that as well. With that, Reviewed-by: Ankur Arora <ankur.a.arora@oracle.com> Ankur > + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; > return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | > (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; > } > diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c > index 829daa0764dd9..23ca2155306b1 100644 > --- a/kernel/trace/trace_output.c > +++ b/kernel/trace/trace_output.c > @@ -463,17 +463,29 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry) > !IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' : > '.'; > > - switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | > + switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | > TRACE_FLAG_PREEMPT_RESCHED)) { > + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: > + need_resched = 'B'; > + break; > case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_PREEMPT_RESCHED: > need_resched = 'N'; > break; > + case TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: > + need_resched = 'L'; > + break; > + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY: > + need_resched = 'b'; > + break; > case TRACE_FLAG_NEED_RESCHED: > need_resched = 'n'; > break; > case TRACE_FLAG_PREEMPT_RESCHED: > need_resched = 'p'; > break; > + case TRACE_FLAG_NEED_RESCHED_LAZY: > + need_resched = 'l'; > + break; > default: > need_resched = '.'; > break; -- ankur
On Wed, 09 Oct 2024 10:30:28 -0700 Ankur Arora <ankur.a.arora@oracle.com> wrote: > > +++ b/kernel/trace/trace.c > > @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) > > trace_flags |= TRACE_FLAG_NEED_RESCHED; > > if (test_preempt_need_resched()) > > trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; > > + if (tif_test_bit(TIF_NEED_RESCHED_LAZY)) > > TIF_NEED_RESCHED_LAZY falls back to TIF_NEED_RESCHED without > CONFIG_ARCH_HAS_PREEMPT_LAZY. So, you might need to add an explicit > check for that as well. > > With that, > Reviewed-by: Ankur Arora <ankur.a.arora@oracle.com> > > Ankur > So this should be: if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && tif_test_bit(TIF_NEED_RESCHED_LAZY)) ? -- Steve > > + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; > > return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | > > (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; > > } > > diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c > > index 829daa0764dd9..23ca2155306b1 100644 > > --- a/kernel/trace/trace_output.c > > +++ b/kernel/trace/trace_output.c > > @@ -463,17 +463,29 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry) > > !IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' : > > '.'; > > > > - switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | > > + switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | > > TRACE_FLAG_PREEMPT_RESCHED)) { > > + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: > > + need_resched = 'B'; > > + break; > > case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_PREEMPT_RESCHED: > > need_resched = 'N'; > > break; > > + case TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: > > + need_resched = 'L'; > > + break; > > + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY: > > + need_resched = 'b'; > > + break; > > case TRACE_FLAG_NEED_RESCHED: > > need_resched = 'n'; > > break; > > case TRACE_FLAG_PREEMPT_RESCHED: > > need_resched = 'p'; > > break; > > + case TRACE_FLAG_NEED_RESCHED_LAZY: > > + need_resched = 'l'; > > + break; > > default: > > need_resched = '.'; > > break;
Steven Rostedt <rostedt@goodmis.org> writes: > On Wed, 09 Oct 2024 10:30:28 -0700 > Ankur Arora <ankur.a.arora@oracle.com> wrote: > >> > +++ b/kernel/trace/trace.c >> > @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) >> > trace_flags |= TRACE_FLAG_NEED_RESCHED; >> > if (test_preempt_need_resched()) >> > trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; >> > + if (tif_test_bit(TIF_NEED_RESCHED_LAZY)) >> >> TIF_NEED_RESCHED_LAZY falls back to TIF_NEED_RESCHED without >> CONFIG_ARCH_HAS_PREEMPT_LAZY. So, you might need to add an explicit >> check for that as well. >> >> With that, >> Reviewed-by: Ankur Arora <ankur.a.arora@oracle.com> >> >> Ankur >> > > So this should be: > > if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && > tif_test_bit(TIF_NEED_RESCHED_LAZY)) > > ? Yeah, exactly that. -- ankur
On 2024-10-09 11:49:41 [-0700], Ankur Arora wrote: > > So this should be: > > > > if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && > > tif_test_bit(TIF_NEED_RESCHED_LAZY)) > > > > ? > > Yeah, exactly that. Okay, this makes sense, this is what I ended up with: diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index d5c0fcf20f024..4cae6f2581379 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -185,6 +185,7 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status); enum trace_flag_type { TRACE_FLAG_IRQS_OFF = 0x01, TRACE_FLAG_NEED_RESCHED = 0x02, + TRACE_FLAG_NEED_RESCHED_LAZY = 0x04, TRACE_FLAG_HARDIRQ = 0x08, TRACE_FLAG_SOFTIRQ = 0x10, TRACE_FLAG_PREEMPT_RESCHED = 0x20, diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 1c69ca1f10886..fb839d00aad12 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) trace_flags |= TRACE_FLAG_NEED_RESCHED; if (test_preempt_need_resched()) trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; + if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && tif_test_bit(TIF_NEED_RESCHED_LAZY)) + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; } diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c index 829daa0764dd9..23ca2155306b1 100644 --- a/kernel/trace/trace_output.c +++ b/kernel/trace/trace_output.c @@ -463,17 +463,29 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry) !IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' : '.'; - switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | + switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED)) { + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: + need_resched = 'B'; + break; case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_PREEMPT_RESCHED: need_resched = 'N'; break; + case TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: + need_resched = 'L'; + break; + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY: + need_resched = 'b'; + break; case TRACE_FLAG_NEED_RESCHED: need_resched = 'n'; break; case TRACE_FLAG_PREEMPT_RESCHED: need_resched = 'p'; break; + case TRACE_FLAG_NEED_RESCHED_LAZY: + need_resched = 'l'; + break; default: need_resched = '.'; break;
On Thu, 10 Oct 2024 12:56:11 +0200 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > +++ b/kernel/trace/trace.c > @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) > trace_flags |= TRACE_FLAG_NEED_RESCHED; > if (test_preempt_need_resched()) > trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; > + if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && tif_test_bit(TIF_NEED_RESCHED_LAZY)) > + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; > return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | > (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; > } This is the only update you made from your previous version right? If so, my reviewed-by stands. -- Steve
On 2024-10-10 10:52:14 [-0400], Steven Rostedt wrote: > On Thu, 10 Oct 2024 12:56:11 +0200 > Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > > +++ b/kernel/trace/trace.c > > @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) > > trace_flags |= TRACE_FLAG_NEED_RESCHED; > > if (test_preempt_need_resched()) > > trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; > > + if (IS_ENABLED(CONFIG_ARCH_HAS_PREEMPT_LAZY) && tif_test_bit(TIF_NEED_RESCHED_LAZY)) > > + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; > > return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | > > (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; > > } > > This is the only update you made from your previous version right? Yes. > If so, my reviewed-by stands. Let me add your's and Ankur's. > -- Steve Sebastian
On 10/9/24 16:20, Sebastian Andrzej Siewior wrote: > The scheduler added NEED_RESCHED_LAZY scheduling. Record this state as > part of trace flags and expose it in the need_resched field. > > Record and expose NEED_RESCHED_LAZY. > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > include/linux/trace_events.h | 1 + > kernel/trace/trace.c | 2 ++ > kernel/trace/trace_output.c | 14 +++++++++++++- > 3 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h > index d5c0fcf20f024..4cae6f2581379 100644 > --- a/include/linux/trace_events.h > +++ b/include/linux/trace_events.h > @@ -185,6 +185,7 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status); > enum trace_flag_type { > TRACE_FLAG_IRQS_OFF = 0x01, > TRACE_FLAG_NEED_RESCHED = 0x02, > + TRACE_FLAG_NEED_RESCHED_LAZY = 0x04, > TRACE_FLAG_HARDIRQ = 0x08, > TRACE_FLAG_SOFTIRQ = 0x10, > TRACE_FLAG_PREEMPT_RESCHED = 0x20, > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 1c69ca1f10886..29d7703751aa9 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) > trace_flags |= TRACE_FLAG_NEED_RESCHED; > if (test_preempt_need_resched()) > trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; > + if (tif_test_bit(TIF_NEED_RESCHED_LAZY)) > + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; > return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | > (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; > } > diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c > index 829daa0764dd9..23ca2155306b1 100644 > --- a/kernel/trace/trace_output.c > +++ b/kernel/trace/trace_output.c > @@ -463,17 +463,29 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry) > !IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' : > '.'; > > - switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | > + switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | > TRACE_FLAG_PREEMPT_RESCHED)) { > + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: > + need_resched = 'B'; > + break; > case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_PREEMPT_RESCHED: > need_resched = 'N'; > break; > + case TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: > + need_resched = 'L'; > + break; > + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY: > + need_resched = 'b'; > + break; > case TRACE_FLAG_NEED_RESCHED: > need_resched = 'n'; > break; > case TRACE_FLAG_PREEMPT_RESCHED: > need_resched = 'p'; > break; > + case TRACE_FLAG_NEED_RESCHED_LAZY: > + need_resched = 'l'; > + break; > default: > need_resched = '.'; > break; Could you please update in Documentation/trace/ftrace.rst as well?
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h index d5c0fcf20f024..4cae6f2581379 100644 --- a/include/linux/trace_events.h +++ b/include/linux/trace_events.h @@ -185,6 +185,7 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status); enum trace_flag_type { TRACE_FLAG_IRQS_OFF = 0x01, TRACE_FLAG_NEED_RESCHED = 0x02, + TRACE_FLAG_NEED_RESCHED_LAZY = 0x04, TRACE_FLAG_HARDIRQ = 0x08, TRACE_FLAG_SOFTIRQ = 0x10, TRACE_FLAG_PREEMPT_RESCHED = 0x20, diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 1c69ca1f10886..29d7703751aa9 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -2544,6 +2544,8 @@ unsigned int tracing_gen_ctx_irq_test(unsigned int irqs_status) trace_flags |= TRACE_FLAG_NEED_RESCHED; if (test_preempt_need_resched()) trace_flags |= TRACE_FLAG_PREEMPT_RESCHED; + if (tif_test_bit(TIF_NEED_RESCHED_LAZY)) + trace_flags |= TRACE_FLAG_NEED_RESCHED_LAZY; return (trace_flags << 16) | (min_t(unsigned int, pc & 0xff, 0xf)) | (min_t(unsigned int, migration_disable_value(), 0xf)) << 4; } diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c index 829daa0764dd9..23ca2155306b1 100644 --- a/kernel/trace/trace_output.c +++ b/kernel/trace/trace_output.c @@ -463,17 +463,29 @@ int trace_print_lat_fmt(struct trace_seq *s, struct trace_entry *entry) !IS_ENABLED(CONFIG_TRACE_IRQFLAGS_SUPPORT) ? 'X' : '.'; - switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | + switch (entry->flags & (TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED)) { + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: + need_resched = 'B'; + break; case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_PREEMPT_RESCHED: need_resched = 'N'; break; + case TRACE_FLAG_NEED_RESCHED_LAZY | TRACE_FLAG_PREEMPT_RESCHED: + need_resched = 'L'; + break; + case TRACE_FLAG_NEED_RESCHED | TRACE_FLAG_NEED_RESCHED_LAZY: + need_resched = 'b'; + break; case TRACE_FLAG_NEED_RESCHED: need_resched = 'n'; break; case TRACE_FLAG_PREEMPT_RESCHED: need_resched = 'p'; break; + case TRACE_FLAG_NEED_RESCHED_LAZY: + need_resched = 'l'; + break; default: need_resched = '.'; break;
The scheduler added NEED_RESCHED_LAZY scheduling. Record this state as part of trace flags and expose it in the need_resched field. Record and expose NEED_RESCHED_LAZY. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- include/linux/trace_events.h | 1 + kernel/trace/trace.c | 2 ++ kernel/trace/trace_output.c | 14 +++++++++++++- 3 files changed, 16 insertions(+), 1 deletion(-)