Re: [PATCH v2 4/4] pseries/mobility: Set NMI watchdog factor during LPM
From: Nathan Lynch <hidden>
Date: 2022-06-23 18:28:59
Also in:
lkml
Laurent Dufour [off-list ref] writes:
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
+ .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.