On Sun, Sep 27, 2020 at 07:05:34PM -0700, psoda...@codeaurora.org wrote:
> On 2020-09-24 11:21, Thomas Gleixner wrote:
> > On Thu, Sep 24 2020 at 08:33, Greg KH wrote:
> > > On Wed, Sep 23, 2020 at 05:08:32PM -0700, Prasad Sodagudi wrote:
> > > > +config CONSOLE_FLUSH_ON_HOTPLUG
> > > > + bool "Enable console flush configurable in hot plug code path"
> > > > + depends on HOTPLUG_CPU
> > > > + def_bool n
> > >
> > > n is the default, no need to list it.
> > >
> > > > + help
> > > > + In cpu hot plug path console lock acquire and release causes the
> > > > + console to flush. If console lock is not free hot plug latency
> > > > + increases. So make console flush configurable in hot plug path
> > > > + and default disabled to help in cpu hot plug latencies.
> > >
> > > Why would you not want this option?
> > >
> > > Why isn't this just a bugfix?
> >
> > Because it's the normal behaviour of console lock and there are
> > gazillion other ways to delay stuff in the hotplug path.
> >
> > CPU hotplug is not meant to be a high speed operation and if people
> > think they need it to be fast then its pretty much guaranteed that they
> > want it for the completely wrong reasons.
> >
> > This #ifdef tinkering is just digusting especially as it just tackles an
> > obvious way how to delay timer migration, but does not address the
> > underlying root cause.
> >
>
> Hi tglx,
>
> Yes. I agree with you that there are other conditions, which could delay the
> hotplug operation. But this console
> flushing is not needed in the hotplug path. In the hotplug path, a core is
> trying printing messages
> from other core(by design of printk), delays the whole hotplug operation and
> timers migration. As timers
> migration gets delayed, it would impact the systems stability in device
> stability testing.
> To avoid timers delay in the timer migration in debug builds has to choose
> this option.
>
> I thought of changing the timers and irq migration as priority callbacks in
> the hotplug out operation
> but I observed some comments like shown below. I was under impression that,
> it is hard to find all this
> type of conditions, so started tinkering hotplug path by changing the log
> levels.
> These changes helped on Qualcomm platforms testing.
> /*
> * On the tear-down path, timers_dead_cpu() must be invoked
> * before blk_mq_queue_reinit_notify() from notify_dead(),
> * otherwise a RCU stall occurs.
> */
> [CPUHP_TIMERS_PREPARE] = {
> .name = "timers:prepare",
> .startup.single = timers_prepare_cpu,
> .teardown.single = timers_dead_cpu,
> },
>
> Another reason for adding #ifdef is that, I was not clear why console flush
> is need cpuhp callback and thought
> there might be some use cases and console flush use case might not be valid
> for all the users of cpu hotplug.
> I will try to explore the changing the callback order to complete the timers
> and irq migration early in the hotplug operation.
>
> Let me put some use cases of hotplug and why hotplug and hotplug latency is
> important from testing point of view.
> 1) Secondary cpus are hotplug out during the device suspend and hotplug in
> during the resume. So cpu hotplug operation is important production devices
> point of view as user presses the power key many times.
But what does suspend/resume have to do with this? Why not do just an
offline operation instead of unplugging the whole cpu?
> 2) sysfs nodes (/sys/devices/ststem/cpu/cpu4/oneline) are present from
> linux
> kernel, so test team wants to test cpu hotplug. There could be issues with
> in generic kernel, device drivers or firmware(psci calls handling from
> firmware). There could be issues with device drivers or firmware and test
> teams can not leave the hotplug untested in builds.
Your change isn't for testing things, speed doesn't matter when writing
to sysfs nodes, right?
> 3) Linux kernel also gave provision to register call backs with cpu hotplug
> framework(CPUHP_AP_ONLINE_DYN) dynamic callbacks.
> 3002 ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> "printk:online",
> 3003 console_cpu_notify, NULL);
> So test team wants to test if any in tree or out of tree modules have
> any
> issues with registered call backs or not.
Again, how is this a speed issue?
> 4) Tracing of the cpuhp operation is important to find whether upstream
> changes or out of tree modules(or firmware changes) caused latency
> regression or not.
But cpu hotplug is not deterministic, so how does latency matter here?
confused as to the real problem here...
greg k-h
