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

[Michael Ellerman]

Nathan Lynch <nath...@linux.ibm.com> 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 <nath...@linux.ibm.com>
> ---
>  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