Subject: Re: [PATCH 12/29] powerpc/pseries/mobility: extract VASI session polling logic

From: Michael Ellerman <[email-masked]>
Date: Fri, 04 Dec 2020 23:51:07 +1100

Nathan Lynch <[email-masked]> writes:
> The behavior of rtas_ibm_suspend_me_unsafe() is to return -EAGAIN to
> the caller until the specified VASI suspend session state makes the
> transition from H_VASI_ENABLED to H_VASI_SUSPENDING. In the interest
> of separating concerns to prepare for a new implementation of the
> join/suspend sequence, extract VASI session polling logic into a
> couple of local functions. Waiting for the session state to reach
> H_VASI_SUSPENDING before calling rtas_ibm_suspend_me_unsafe() ensures
> that we will never get an EAGAIN result necessitating a retry. No
> user-visible change in behavior is intended.
>
> Signed-off-by: Nathan Lynch <[email-masked]>
> ---
>  arch/powerpc/platforms/pseries/mobility.c | 76 +++++++++++++++++++++--
>  1 file changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index dc6abf164db7..1b8ae221b98a 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -345,6 +345,73 @@ void post_mobility_fixup(void)
...

> +
> +static int wait_for_vasi_session_suspending(u64 handle)
> +{
> +	unsigned long state;
> +	bool keep_polling;
> +	int ret;
> +
> +	/*
> +	 * Wait for transition from H_VASI_ENABLED to
> +	 * H_VASI_SUSPENDING. Treat anything else as an error.
> +	 */
> +	do {
> +		keep_polling = false;
> +		ret = poll_vasi_state(handle, &state);
> +		if (ret != 0)
> +			break;
> +
> +		switch (state) {
> +		case H_VASI_SUSPENDING:
> +			break;
> +		case H_VASI_ENABLED:
> +			keep_polling = true;
> +			ssleep(1);
> +			break;
> +		default:
> +			pr_err("unexpected H_VASI_STATE result %lu\n", state);
> +			ret = -EIO;
> +			break;
> +		}
> +	} while (keep_polling);

This seems like it could be simpler?

eg:

	while (true) {
		ret = poll_vasi_state(handle, &state);

		if (ret != 0 || state == H_VASI_SUSPENDING)
			break;
		else if (state == H_VASI_ENABLED)
			ssleep(1);
		else {
			pr_err("unexpected H_VASI_STATE result %lu\n", state);
			ret = -EIO;
			break;
		}
	}

Or did I miss something?

cheers