Messages in this thread Patch in this message | | | Date | Thu, 28 Nov 2019 16:56:12 +0000 | From | Qais Yousef <> | Subject | Re: [PATCH v2 12/14] torture: Replace cpu_up/down with device_online/offline |
| |
On 11/27/19 13:47, Paul E. McKenney wrote: > On Mon, Nov 25, 2019 at 11:27:52AM +0000, Qais Yousef wrote: > > The core device API performs extra housekeeping bits that are missing > > from directly calling cpu_up/down. > > > > See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and > > serialization during LPM") for an example description of what might go > > wrong. > > > > This also prepares to make cpu_up/down a private interface for anything > > but the cpu subsystem. > > > > Signed-off-by: Qais Yousef <qais.yousef@arm.com> > > CC: Davidlohr Bueso <dave@stgolabs.net> > > CC: "Paul E. McKenney" <paulmck@kernel.org> > > CC: Josh Triplett <josh@joshtriplett.org> > > CC: linux-kernel@vger.kernel.org > > Looks fine from an rcutorture viewpoint, but why not provide an API > that pulled lock_device_hotplug() and unlock_device_hotplug() into the > online/offline calls?
I *think* the right way to do what you say is by doing lock_device_hotplug() inside device_{online, offline}() - which affects all drivers not just the CPU.
And even then, I think we need to refcount it so nested calls won't deadlock.
I don't know if this can break any rule or not. If Greg thinks it's okay I'd be happy to post some patches that do just that.
Thanks
-- Qais Yousef
> > Thanx, Paul > > > --- > > kernel/torture.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/kernel/torture.c b/kernel/torture.c > > index 7c13f5558b71..12115024feb2 100644 > > --- a/kernel/torture.c > > +++ b/kernel/torture.c > > @@ -97,7 +97,9 @@ bool torture_offline(int cpu, long *n_offl_attempts, long *n_offl_successes, > > torture_type, cpu); > > starttime = jiffies; > > (*n_offl_attempts)++; > > - ret = cpu_down(cpu); > > + lock_device_hotplug(); > > + ret = device_offline(get_cpu_device(cpu)); > > + unlock_device_hotplug(); > > if (ret) { > > if (verbose) > > pr_alert("%s" TORTURE_FLAG > > @@ -148,7 +150,9 @@ bool torture_online(int cpu, long *n_onl_attempts, long *n_onl_successes, > > torture_type, cpu); > > starttime = jiffies; > > (*n_onl_attempts)++; > > - ret = cpu_up(cpu); > > + lock_device_hotplug(); > > + ret = device_online(get_cpu_device(cpu)); > > + unlock_device_hotplug(); > > if (ret) { > > if (verbose) > > pr_alert("%s" TORTURE_FLAG > > @@ -192,17 +196,20 @@ torture_onoff(void *arg) > > for_each_online_cpu(cpu) > > maxcpu = cpu; > > WARN_ON(maxcpu < 0); > > - if (!IS_MODULE(CONFIG_TORTURE_TEST)) > > + if (!IS_MODULE(CONFIG_TORTURE_TEST)) { > > + lock_device_hotplug(); > > for_each_possible_cpu(cpu) { > > if (cpu_online(cpu)) > > continue; > > - ret = cpu_up(cpu); > > + ret = device_online(get_cpu_device(cpu)); > > if (ret && verbose) { > > pr_alert("%s" TORTURE_FLAG > > "%s: Initial online %d: errno %d\n", > > __func__, torture_type, cpu, ret); > > } > > } > > + unlock_device_hotplug(); > > + } > > > > if (maxcpu == 0) { > > VERBOSE_TOROUT_STRING("Only one CPU, so CPU-hotplug testing is disabled"); > > -- > > 2.17.1 > >
| |