Laurent Dufour <lduf...@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.

Reply via email to