| Message ID | 20240628041826.68587-1-takakura@valinux.co.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rcu: Let rcu_dump_task() be used without preemption disabled | expand |
On Fri, Jun 28, 2024 at 01:18:26PM +0900, takakura@valinux.co.jp wrote: > From: Ryo Takakura <takakura@valinux.co.jp> > > The commit 2d7f00b2f0130 ("rcu: Suppress smp_processor_id() complaint > in synchronize_rcu_expedited_wait()") disabled preemption around > dump_cpu_task() to suppress warning on its usage within preemtible context. > > Calling dump_cpu_task() doesn't required to be in non-preemptible context > except for suppressing the smp_processor_id() warning. > As the smp_processor_id() is evaluated along with in_hardirq() > to check if it's in interrupt context, this patch removes the need > for its preemtion disablement by reordering the condition so that > smp_processor_id() only gets evaluated when it's in interrupt context. > > Signed-off-by: Ryo Takakura <takakura@valinux.co.jp> Hearing no objections, I pulled this in for further review and testing. I had to hand-apply this due to a recent conflicting change in the -rcu tree, so could you please check the version below in case I messed something up? Thanx, Paul ------------------------------------------------------------------------ commit ad6647a70f239aa9f2741b2f5a828a4483122a26 Author: Ryo Takakura <takakura@valinux.co.jp> Date: Fri Jun 28 13:18:26 2024 +0900 rcu: Let rcu_dump_task() be used without preemption disabled The commit 2d7f00b2f0130 ("rcu: Suppress smp_processor_id() complaint in synchronize_rcu_expedited_wait()") disabled preemption around dump_cpu_task() to suppress warning on its usage within preemtible context. Calling dump_cpu_task() doesn't required to be in non-preemptible context except for suppressing the smp_processor_id() warning. As the smp_processor_id() is evaluated along with in_hardirq() to check if it's in interrupt context, this patch removes the need for its preemtion disablement by reordering the condition so that smp_processor_id() only gets evaluated when it's in interrupt context. Signed-off-by: Ryo Takakura <takakura@valinux.co.jp> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h index d4be644afb50..c5d9a7eb0803 100644 --- a/kernel/rcu/tree_exp.h +++ b/kernel/rcu/tree_exp.h @@ -597,9 +597,7 @@ static void synchronize_rcu_expedited_stall(unsigned long jiffies_start, unsigne mask = leaf_node_cpu_bit(rnp, cpu); if (!(READ_ONCE(rnp->expmask) & mask)) continue; - preempt_disable(); // For smp_processor_id() in dump_cpu_task(). dump_cpu_task(cpu); - preempt_enable(); } rcu_exp_print_detail_task_stall_rnp(rnp); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 05afa2932b5e..bdb0e0328f6a 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -11485,7 +11485,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { void dump_cpu_task(int cpu) { - if (cpu == smp_processor_id() && in_hardirq()) { + if (in_hardirq() && cpu == smp_processor_id()) { struct pt_regs *regs; regs = get_irq_regs();
Hi Paul, On Mon, 8 July 2024, Paul E. McKenney wrote: >On Fri, Jun 28, 2024 at 01:18:26PM +0900, takakura@valinux.co.jp wrote: >> From: Ryo Takakura <takakura@valinux.co.jp> >> >> The commit 2d7f00b2f0130 ("rcu: Suppress smp_processor_id() complaint >> in synchronize_rcu_expedited_wait()") disabled preemption around >> dump_cpu_task() to suppress warning on its usage within preemtible context. >> >> Calling dump_cpu_task() doesn't required to be in non-preemptible context >> except for suppressing the smp_processor_id() warning. >> As the smp_processor_id() is evaluated along with in_hardirq() >> to check if it's in interrupt context, this patch removes the need >> for its preemtion disablement by reordering the condition so that >> smp_processor_id() only gets evaluated when it's in interrupt context. >> >> Signed-off-by: Ryo Takakura <takakura@valinux.co.jp> > >Hearing no objections, I pulled this in for further review and testing. > >I had to hand-apply this due to a recent conflicting change in the >-rcu tree, so could you please check the version below in case I messed >something up? > > Thanx, Paul Thanks for preparing the patch! I checked it on the rcu tree and looks good to me. If possible, could you replace the title with s/rcu_dump_task()/dump_cpu_task()/ when applying? I made a mistake with the title where dump_cpu_task() is the one being modified, not rcu_dump_task(). I'm sorry for the confusion. Sincerely, Ryo Takakura > >------------------------------------------------------------------------ > >commit ad6647a70f239aa9f2741b2f5a828a4483122a26 >Author: Ryo Takakura <takakura@valinux.co.jp> >Date: Fri Jun 28 13:18:26 2024 +0900 > > rcu: Let rcu_dump_task() be used without preemption disabled > > The commit 2d7f00b2f0130 ("rcu: Suppress smp_processor_id() complaint > in synchronize_rcu_expedited_wait()") disabled preemption around > dump_cpu_task() to suppress warning on its usage within preemtible context. > > Calling dump_cpu_task() doesn't required to be in non-preemptible context > except for suppressing the smp_processor_id() warning. > As the smp_processor_id() is evaluated along with in_hardirq() > to check if it's in interrupt context, this patch removes the need > for its preemtion disablement by reordering the condition so that > smp_processor_id() only gets evaluated when it's in interrupt context. > > Signed-off-by: Ryo Takakura <takakura@valinux.co.jp> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > >diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h >index d4be644afb50..c5d9a7eb0803 100644 >--- a/kernel/rcu/tree_exp.h >+++ b/kernel/rcu/tree_exp.h >@@ -597,9 +597,7 @@ static void synchronize_rcu_expedited_stall(unsigned long jiffies_start, unsigne > mask = leaf_node_cpu_bit(rnp, cpu); > if (!(READ_ONCE(rnp->expmask) & mask)) > continue; >- preempt_disable(); // For smp_processor_id() in dump_cpu_task(). > dump_cpu_task(cpu); >- preempt_enable(); > } > rcu_exp_print_detail_task_stall_rnp(rnp); > } >diff --git a/kernel/sched/core.c b/kernel/sched/core.c >index 05afa2932b5e..bdb0e0328f6a 100644 >--- a/kernel/sched/core.c >+++ b/kernel/sched/core.c >@@ -11485,7 +11485,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { > > void dump_cpu_task(int cpu) > { >- if (cpu == smp_processor_id() && in_hardirq()) { >+ if (in_hardirq() && cpu == smp_processor_id()) { > struct pt_regs *regs; > > regs = get_irq_regs();
On Tue, Jul 09, 2024 at 02:34:26PM +0900, takakura@valinux.co.jp wrote: > Hi Paul, > > On Mon, 8 July 2024, Paul E. McKenney wrote: > >On Fri, Jun 28, 2024 at 01:18:26PM +0900, takakura@valinux.co.jp wrote: > >> From: Ryo Takakura <takakura@valinux.co.jp> > >> > >> The commit 2d7f00b2f0130 ("rcu: Suppress smp_processor_id() complaint > >> in synchronize_rcu_expedited_wait()") disabled preemption around > >> dump_cpu_task() to suppress warning on its usage within preemtible context. > >> > >> Calling dump_cpu_task() doesn't required to be in non-preemptible context > >> except for suppressing the smp_processor_id() warning. > >> As the smp_processor_id() is evaluated along with in_hardirq() > >> to check if it's in interrupt context, this patch removes the need > >> for its preemtion disablement by reordering the condition so that > >> smp_processor_id() only gets evaluated when it's in interrupt context. > >> > >> Signed-off-by: Ryo Takakura <takakura@valinux.co.jp> > > > >Hearing no objections, I pulled this in for further review and testing. > > > >I had to hand-apply this due to a recent conflicting change in the > >-rcu tree, so could you please check the version below in case I messed > >something up? > > > > Thanx, Paul > > Thanks for preparing the patch! > I checked it on the rcu tree and looks good to me. > > If possible, could you replace the title with s/rcu_dump_task()/dump_cpu_task()/ > when applying? > I made a mistake with the title where dump_cpu_task() is the one being modified, > not rcu_dump_task(). I'm sorry for the confusion. Thank you for calling my attention to this. Done locally, and it will show up on my next rebase. Thanx, Paul > Sincerely, > Ryo Takakura > > > > >------------------------------------------------------------------------ > > > >commit ad6647a70f239aa9f2741b2f5a828a4483122a26 > >Author: Ryo Takakura <takakura@valinux.co.jp> > >Date: Fri Jun 28 13:18:26 2024 +0900 > > > > rcu: Let rcu_dump_task() be used without preemption disabled > > > > The commit 2d7f00b2f0130 ("rcu: Suppress smp_processor_id() complaint > > in synchronize_rcu_expedited_wait()") disabled preemption around > > dump_cpu_task() to suppress warning on its usage within preemtible context. > > > > Calling dump_cpu_task() doesn't required to be in non-preemptible context > > except for suppressing the smp_processor_id() warning. > > As the smp_processor_id() is evaluated along with in_hardirq() > > to check if it's in interrupt context, this patch removes the need > > for its preemtion disablement by reordering the condition so that > > smp_processor_id() only gets evaluated when it's in interrupt context. > > > > Signed-off-by: Ryo Takakura <takakura@valinux.co.jp> > > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > > >diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h > >index d4be644afb50..c5d9a7eb0803 100644 > >--- a/kernel/rcu/tree_exp.h > >+++ b/kernel/rcu/tree_exp.h > >@@ -597,9 +597,7 @@ static void synchronize_rcu_expedited_stall(unsigned long jiffies_start, unsigne > > mask = leaf_node_cpu_bit(rnp, cpu); > > if (!(READ_ONCE(rnp->expmask) & mask)) > > continue; > >- preempt_disable(); // For smp_processor_id() in dump_cpu_task(). > > dump_cpu_task(cpu); > >- preempt_enable(); > > } > > rcu_exp_print_detail_task_stall_rnp(rnp); > > } > >diff --git a/kernel/sched/core.c b/kernel/sched/core.c > >index 05afa2932b5e..bdb0e0328f6a 100644 > >--- a/kernel/sched/core.c > >+++ b/kernel/sched/core.c > >@@ -11485,7 +11485,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { > > > > void dump_cpu_task(int cpu) > > { > >- if (cpu == smp_processor_id() && in_hardirq()) { > >+ if (in_hardirq() && cpu == smp_processor_id()) { > > struct pt_regs *regs; > > > > regs = get_irq_regs();
diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h index 8a1d9c8bd9f7..80c461b26144 100644 --- a/kernel/rcu/tree_exp.h +++ b/kernel/rcu/tree_exp.h @@ -617,9 +617,7 @@ static void synchronize_rcu_expedited_wait(void) mask = leaf_node_cpu_bit(rnp, cpu); if (!(READ_ONCE(rnp->expmask) & mask)) continue; - preempt_disable(); // For smp_processor_id() in dump_cpu_task(). dump_cpu_task(cpu); - preempt_enable(); } rcu_exp_print_detail_task_stall_rnp(rnp); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index bcf2c4cc0522..38efbdafe3d4 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -11485,7 +11485,7 @@ struct cgroup_subsys cpu_cgrp_subsys = { void dump_cpu_task(int cpu) { - if (cpu == smp_processor_id() && in_hardirq()) { + if (in_hardirq() && cpu == smp_processor_id()) { struct pt_regs *regs; regs = get_irq_regs();