lkml.org 
[lkml]   [2022]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 4/4] pseries/mobility: Set NMI watchdog factor during LPM
Date
Laurent Dufour <ldufour@linux.ibm.com> writes:
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 179bbd4ae881..4284ceaf9060 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -48,6 +48,39 @@ struct update_props_workarea {
> #define MIGRATION_SCOPE (1)
> #define PRRN_SCOPE -2
>
> +#ifdef CONFIG_PPC_WATCHDOG
> +static unsigned int lpm_nmi_wd_factor = 200;
> +
> +#ifdef CONFIG_SYSCTL
> +static struct ctl_table lpm_nmi_wd_factor_ctl_table[] = {
> + {
> + .procname = "lpm_nmi_watchdog_factor",

Assuming the basic idea is acceptable, I suggest making the user-visible
name more generic (e.g. "nmi_watchdog_factor") in case it makes sense to
apply this to other contexts in the future.

> + .data = &lpm_nmi_wd_factor,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_douintvec_minmax,
> + },
> + {}
> +};
> +static struct ctl_table lpm_nmi_wd_factor_sysctl_root[] = {
> + {
> + .procname = "kernel",
> + .mode = 0555,
> + .child = lpm_nmi_wd_factor_ctl_table,
> + },
> + {}
> +};
> +
> +static int __init register_lpm_nmi_wd_factor_sysctl(void)
> +{
> + register_sysctl_table(lpm_nmi_wd_factor_sysctl_root);
> +
> + return 0;
> +}
> +device_initcall(register_lpm_nmi_wd_factor_sysctl);
> +#endif /* CONFIG_SYSCTL */
> +#endif /* CONFIG_PPC_WATCHDOG */
> +
> static int mobility_rtas_call(int token, char *buf, s32 scope)
> {
> int rc;
> @@ -702,6 +735,7 @@ static int pseries_suspend(u64 handle)
> static int pseries_migrate_partition(u64 handle)
> {
> int ret;
> + unsigned int factor = lpm_nmi_wd_factor;
>
> ret = wait_for_vasi_session_suspending(handle);
> if (ret)
> @@ -709,6 +743,13 @@ static int pseries_migrate_partition(u64 handle)
>
> vas_migration_handler(VAS_SUSPEND);
>
> +#ifdef CONFIG_PPC_WATCHDOG
> + if (factor) {
> + pr_info("Set the NMI watchdog factor to %u%%\n", factor);
> + watchdog_nmi_set_lpm_factor(factor);
> + }
> +#endif /* CONFIG_PPC_WATCHDOG */
> +
> ret = pseries_suspend(handle);
> if (ret == 0) {
> post_mobility_fixup();
> @@ -716,6 +757,13 @@ static int pseries_migrate_partition(u64 handle)
> } else
> pseries_cancel_migration(handle, ret);
>
> +#ifdef CONFIG_PPC_WATCHDOG
> + if (factor) {
> + pr_info("Restoring NMI watchdog timer\n");
> + watchdog_nmi_set_lpm_factor(0);
> + }
> +#endif /* CONFIG_PPC_WATCHDOG */
> +

A couple more suggestions:

* Move the prints into a single statement in watchdog_nmi_set_lpm_factor().

* Add no-op versions of watchdog_nmi_set_lpm_factor for
!CONFIG_PPC_WATCHDOG so we can minimize the #ifdef here.

Otherwise this looks fine to me.

\
 
 \ /
  Last update: 2022-06-23 20:29    [W:0.124 / U:0.992 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site