From: Zhouyi Zhou <zhouzhouyi@gmail.com> To: Thomas Gleixner <tglx@linutronix.de> Cc: fweisbec@gmail.com, mingo@kernel.org, dave@stgolabs.net, paulmck@kernel.org, josh@joshtriplett.org, mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu Date: Sun, 27 Nov 2022 10:45:34 +0800 [thread overview] Message-ID: <CAABZP2xNTbrx9iV+KH3VZx1c9Yi97+izNA=XSJQBuOJ4WENFZg@mail.gmail.com> (raw) In-Reply-To: <87y1rxwsse.ffs@tglx> Thank Thomas for your guidance On Sun, Nov 27, 2022 at 1:05 AM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, Nov 21 2022 at 11:51, Zhouyi Zhou wrote: > > During CPU-hotplug torture (CONFIG_NO_HZ_FULL=y), if we try to > > offline tick_do_timer_cpu, the operation will fail because in > > function tick_nohz_cpu_down: > > ``` > > if (tick_nohz_full_running && tick_do_timer_cpu == cpu) > > return -EBUSY; > > ``` > > Above bug was first discovered in torture tests performed in PPC VM > > How is this a bug? Yes, this is a false positive instead. > > > of Open Source Lab of Oregon State University, and reproducable in RISC-V > > and X86-64 (with additional kernel commandline cpu0_hotplug). > > > > In this patch, we avoid offline tick_do_timer_cpu by distribute > > the offlining cpu among remaining cpus. > > Please read Documentation/process. Search for 'this patch'... Documentation/process/submitting-patches.rst says: "Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour." So, I should construct my patch as: We avoid ... by ... > > > > > Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com> > > --- > > include/linux/tick.h | 1 + > > kernel/time/tick-common.c | 1 + > > kernel/time/tick-internal.h | 1 - > > kernel/torture.c | 10 ++++++++++ > > 4 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/tick.h b/include/linux/tick.h > > index bfd571f18cfd..23cc0b205853 100644 > > --- a/include/linux/tick.h > > +++ b/include/linux/tick.h > > @@ -14,6 +14,7 @@ > > #include <linux/rcupdate.h> > > > > #ifdef CONFIG_GENERIC_CLOCKEVENTS > > +extern int tick_do_timer_cpu __read_mostly; > > extern void __init tick_init(void); > > /* Should be core only, but ARM BL switcher requires it */ > > extern void tick_suspend_local(void); > > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > > index 46789356f856..87b9b9afa320 100644 > > --- a/kernel/time/tick-common.c > > +++ b/kernel/time/tick-common.c > > @@ -48,6 +48,7 @@ ktime_t tick_next_period; > > * procedure also covers cpu hotplug. > > */ > > int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT; > > +EXPORT_SYMBOL_GPL(tick_do_timer_cpu); > > No. We are not exporting this just to make a bogus test case happy. > > Fix the torture code to handle -EBUSY correctly. I am going to do a study on this, for now, I do a grep in the kernel tree: find . -name "*.c"|xargs grep cpuhp_setup_state|wc -l The result of the grep command shows that there are 268 cpuhp_setup_state* cases. which may make our task more complicated. After my study, should we also take Frederic's proposal as a possible option? (construct a function for this) https://lore.kernel.org/lkml/20221123223658.GC1395324@lothringen/ I learned a lot during this process Many thanks Zhouyi > > Thanks, > > tglx
WARNING: multiple messages have this Message-ID (diff)
From: Zhouyi Zhou <zhouzhouyi@gmail.com> To: Thomas Gleixner <tglx@linutronix.de> Cc: dave@stgolabs.net, paulmck@kernel.org, josh@joshtriplett.org, linux-kernel@vger.kernel.org, fweisbec@gmail.com, linuxppc-dev@lists.ozlabs.org, mingo@kernel.org Subject: Re: [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu Date: Sun, 27 Nov 2022 10:45:34 +0800 [thread overview] Message-ID: <CAABZP2xNTbrx9iV+KH3VZx1c9Yi97+izNA=XSJQBuOJ4WENFZg@mail.gmail.com> (raw) In-Reply-To: <87y1rxwsse.ffs@tglx> Thank Thomas for your guidance On Sun, Nov 27, 2022 at 1:05 AM Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, Nov 21 2022 at 11:51, Zhouyi Zhou wrote: > > During CPU-hotplug torture (CONFIG_NO_HZ_FULL=y), if we try to > > offline tick_do_timer_cpu, the operation will fail because in > > function tick_nohz_cpu_down: > > ``` > > if (tick_nohz_full_running && tick_do_timer_cpu == cpu) > > return -EBUSY; > > ``` > > Above bug was first discovered in torture tests performed in PPC VM > > How is this a bug? Yes, this is a false positive instead. > > > of Open Source Lab of Oregon State University, and reproducable in RISC-V > > and X86-64 (with additional kernel commandline cpu0_hotplug). > > > > In this patch, we avoid offline tick_do_timer_cpu by distribute > > the offlining cpu among remaining cpus. > > Please read Documentation/process. Search for 'this patch'... Documentation/process/submitting-patches.rst says: "Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour." So, I should construct my patch as: We avoid ... by ... > > > > > Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com> > > --- > > include/linux/tick.h | 1 + > > kernel/time/tick-common.c | 1 + > > kernel/time/tick-internal.h | 1 - > > kernel/torture.c | 10 ++++++++++ > > 4 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/tick.h b/include/linux/tick.h > > index bfd571f18cfd..23cc0b205853 100644 > > --- a/include/linux/tick.h > > +++ b/include/linux/tick.h > > @@ -14,6 +14,7 @@ > > #include <linux/rcupdate.h> > > > > #ifdef CONFIG_GENERIC_CLOCKEVENTS > > +extern int tick_do_timer_cpu __read_mostly; > > extern void __init tick_init(void); > > /* Should be core only, but ARM BL switcher requires it */ > > extern void tick_suspend_local(void); > > diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c > > index 46789356f856..87b9b9afa320 100644 > > --- a/kernel/time/tick-common.c > > +++ b/kernel/time/tick-common.c > > @@ -48,6 +48,7 @@ ktime_t tick_next_period; > > * procedure also covers cpu hotplug. > > */ > > int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT; > > +EXPORT_SYMBOL_GPL(tick_do_timer_cpu); > > No. We are not exporting this just to make a bogus test case happy. > > Fix the torture code to handle -EBUSY correctly. I am going to do a study on this, for now, I do a grep in the kernel tree: find . -name "*.c"|xargs grep cpuhp_setup_state|wc -l The result of the grep command shows that there are 268 cpuhp_setup_state* cases. which may make our task more complicated. After my study, should we also take Frederic's proposal as a possible option? (construct a function for this) https://lore.kernel.org/lkml/20221123223658.GC1395324@lothringen/ I learned a lot during this process Many thanks Zhouyi > > Thanks, > > tglx
next prev parent reply other threads:[~2022-11-27 2:45 UTC|newest] Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-21 3:51 [PATCH linux-next][RFC]torture: avoid offline tick_do_timer_cpu Zhouyi Zhou 2022-11-22 1:37 ` Paul E. McKenney 2022-11-22 1:37 ` Paul E. McKenney 2022-11-23 2:23 ` Zhouyi Zhou 2022-11-23 2:23 ` Zhouyi Zhou 2022-11-23 18:49 ` Paul E. McKenney 2022-11-23 18:49 ` Paul E. McKenney 2022-11-24 2:35 ` Zhouyi Zhou 2022-11-24 2:35 ` Zhouyi Zhou 2022-11-23 22:25 ` Frederic Weisbecker 2022-11-23 22:25 ` Frederic Weisbecker 2022-11-23 23:00 ` Paul E. McKenney 2022-11-23 23:00 ` Paul E. McKenney 2022-11-23 22:36 ` Frederic Weisbecker 2022-11-23 22:36 ` Frederic Weisbecker 2022-11-24 2:18 ` Zhouyi Zhou 2022-11-24 2:18 ` Zhouyi Zhou 2022-11-26 17:05 ` Thomas Gleixner 2022-11-27 2:45 ` Zhouyi Zhou [this message] 2022-11-27 2:45 ` Zhouyi Zhou 2022-11-27 12:40 ` Thomas Gleixner 2022-11-27 12:40 ` Thomas Gleixner 2022-11-27 17:53 ` Paul E. McKenney 2022-11-27 17:53 ` Paul E. McKenney 2022-11-28 3:00 ` Zhouyi Zhou 2022-11-28 3:00 ` Zhouyi Zhou 2022-11-28 8:12 ` Thomas Gleixner 2022-11-28 8:12 ` Thomas Gleixner 2022-11-28 15:16 ` Paul E. McKenney 2022-11-28 15:16 ` Paul E. McKenney 2023-07-06 7:09 ` Christophe Leroy 2023-07-06 8:13 ` Zhouyi Zhou 2023-07-06 8:13 ` Zhouyi Zhou
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='CAABZP2xNTbrx9iV+KH3VZx1c9Yi97+izNA=XSJQBuOJ4WENFZg@mail.gmail.com' \ --to=zhouzhouyi@gmail.com \ --cc=dave@stgolabs.net \ --cc=fweisbec@gmail.com \ --cc=josh@joshtriplett.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mingo@kernel.org \ --cc=mpe@ellerman.id.au \ --cc=paulmck@kernel.org \ --cc=tglx@linutronix.de \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.