Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH 2/2] printk: Make the console flush configurable in hotplug path | Date | Thu, 24 Sep 2020 20:21:07 +0200 |
| |
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.
>> +#ifdef CONFIG_CONSOLE_FLUSH_ON_HOTPLUG > > #ifdef in .c code is a mess to maintain. > >> ret = cpuhp_setup_state_nocalls(CPUHP_PRINTK_DEAD, "printk:dead", NULL, >> console_cpu_notify); >> WARN_ON(ret < 0); >> ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "printk:online", >> console_cpu_notify, NULL); >> WARN_ON(ret < 0); >> - return 0; >> +#endif > > What happens if we don't make these calls entirely? Why not just remove > them as who wants extra latency for their system?
That's just wrong. If you don't want output, then adjust your loglevel, but delaying printks up to the point where by chance another printk happens is just silly.
CPU hotplug is not about latency. It's slow by design and again, the timer migration is simply happening at the wrong place. But fixing that needs more thoughts than modifying log levels and sprinkling a few #ifdefs into the code.
Thanks,
tglx
| |