Messages in this thread | | | From | Nathan Lynch <> | Subject | Re: [PATCH v2 4/4] pseries/mobility: Set NMI watchdog factor during LPM | Date | Thu, 23 Jun 2022 12:28:34 -0500 |
| |
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.
| |