Re: [PATCH v1 2/2] PM: hibernate: Combine return paths in power_down()

[Mario Limonciello]

On 9/26/25 11:41 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> To avoid code duplication and improve clarity, combine the code
> paths in power_down() leading to a return from that function.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

The actual change looks fine to me.

Since it's used in "both" a success and failure path would you consider 
using "wakeup" for the label instead of "rollback"?  Or anything else 
you can think of that doesn't have a connotation of failure.

Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>

> ---
>   kernel/power/hibernate.c |   32 ++++++++++++++------------------
>   1 file changed, 14 insertions(+), 18 deletions(-)
>
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -708,21 +708,11 @@ static void power_down(void)
>   	if (hibernation_mode == HIBERNATION_SUSPEND) {
>   		pm_restore_gfp_mask();
>   		error = suspend_devices_and_enter(mem_sleep_current);
> -		if (error) {
> -			hibernation_mode = hibernation_ops ?
> -						HIBERNATION_PLATFORM :
> -						HIBERNATION_SHUTDOWN;
> -		} else {
> -			/* Match pm_restore_gfp_mask() call in hibernate() */
> -			pm_restrict_gfp_mask();
> +		if (!error)
> +			goto rollback;
>   
> -			/* Restore swap signature. */
> -			error = swsusp_unmark();
> -			if (error)
> -				pr_err("Swap will be unusable! Try swapon -a.\n");
> -
> -			return;
> -		}
> +		hibernation_mode = hibernation_ops ? HIBERNATION_PLATFORM :
> +						     HIBERNATION_SHUTDOWN;
>   	}
>   #endif
>   
> @@ -733,12 +723,9 @@ static void power_down(void)
>   	case HIBERNATION_PLATFORM:
>   		error = hibernation_platform_enter();
>   		if (error == -EAGAIN || error == -EBUSY) {
> -			/* Match pm_restore_gfp_mask() in hibernate(). */
> -			pm_restrict_gfp_mask();
> -			swsusp_unmark();
>   			events_check_enabled = false;
>   			pr_info("Wakeup event detected during hibernation, rolling back.\n");
> -			return;
> +			goto rollback;
>   		}
>   		fallthrough;
>   	case HIBERNATION_SHUTDOWN:
> @@ -757,6 +744,15 @@ static void power_down(void)
>   	pr_crit("Power down manually\n");
>   	while (1)
>   		cpu_relax();
> +
> +rollback:
> +	/* Match the pm_restore_gfp_mask() call in hibernate(). */
> +	pm_restrict_gfp_mask();
> +
> +	/* Restore swap signature. */
> +	error = swsusp_unmark();
> +	if (error)
> +		pr_err("Swap will be unusable! Try swapon -a.\n");
>   }
>   
>   static int load_image_and_restore(void)