On Wed, Sep 23, 2020 at 05:08:32PM -0700, Prasad Sodagudi wrote:
> From: Mohammed Khajapasha <mkh...@codeaurora.org>
> 
> The thread which initiates the hot plug can get scheduled
> out, while trying to acquire the console lock,
> thus increasing the hot plug latency. This option
> allows to selectively disable the console flush and
> in turn reduce the hot plug latency.
> 
> Signed-off-by: Mohammed Khajapasha <mkh...@codeaurora.org>
> Signed-off-by: Prasad Sodagudi <psoda...@codeaurora.org>
> ---
>  init/Kconfig           | 10 ++++++++++
>  kernel/printk/printk.c | 10 ++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index d6a0b31..9ce39ba 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -699,6 +699,16 @@ config LOG_BUF_SHIFT
>                    13 =>  8 KB
>                    12 =>  4 KB
>  
> +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?

> +
>  config LOG_CPU_MAX_BUF_SHIFT
>       int "CPU kernel log buffer size contribution (13 => 8 KB, 17 => 128KB)"
>       depends on SMP
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 9b75f6b..f02d3ef 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2283,6 +2283,8 @@ void resume_console(void)
>       console_unlock();
>  }
>  
> +#ifdef CONFIG_CONSOLE_FLUSH_ON_HOTPLUG
> +
>  /**
>   * console_cpu_notify - print deferred console messages after CPU hotplug
>   * @cpu: unused
> @@ -2302,6 +2304,8 @@ static int console_cpu_notify(unsigned int cpu)
>       return 0;
>  }
>  
> +#endif
> +
>  /**
>   * console_lock - lock the console system for exclusive use.
>   *
> @@ -2974,7 +2978,7 @@ void __init console_init(void)
>  static int __init printk_late_init(void)
>  {
>       struct console *con;
> -     int ret;
> +     int ret = 0;
>  
>       for_each_console(con) {
>               if (!(con->flags & CON_BOOT))
> @@ -2996,13 +3000,15 @@ static int __init printk_late_init(void)
>                       unregister_console(con);
>               }
>       }
> +#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?

thanks,

greg k-h

Reply via email to