Re: [PATCH] powerpc/rtas: retry when cpu offline races with suspend/migration

[mmc <mmc@linux.vnet.ibm.com>]

On 2019-06-21 00:05, Nathan Lynch wrote:
> The protocol for suspending or migrating an LPAR requires all present
> processor threads to enter H_JOIN. So if we have threads offline, we
> have to temporarily bring them up. This can race with administrator
> actions such as SMT state changes. As of dfd718a2ed1f ("powerpc/rtas:
> Fix a potential race between CPU-Offline & Migration"),
> rtas_ibm_suspend_me() accounts for this, but errors out with -EBUSY
> for what almost certainly is a transient condition in any reasonable
> scenario.
> 
> Callers of rtas_ibm_suspend_me() already retry when -EAGAIN is
> returned, and it is typical during a migration for that to happen
> repeatedly for several minutes polling the H_VASI_STATE hcall result
> before proceeding to the next stage.
> 
> So return -EAGAIN instead of -EBUSY when this race is
> encountered. Additionally: logging this event is still appropriate but
> use pr_info instead of pr_err; and remove use of unlikely() while here
> as this is not a hot path at all.
> 

Looks good, since it's not a hot path anyway, so unlikely() should 
benefit from optimize compiler path, and should stay. No?

Mingming
> Fixes: dfd718a2ed1f ("powerpc/rtas: Fix a potential race between
> CPU-Offline & Migration")
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> ---
>  arch/powerpc/kernel/rtas.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index fbc676160adf..9b4d2a2ffb4f 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -984,10 +984,9 @@ int rtas_ibm_suspend_me(u64 handle)
>  	cpu_hotplug_disable();
> 
>  	/* Check if we raced with a CPU-Offline Operation */
> -	if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
> -		pr_err("%s: Raced against a concurrent CPU-Offline\n",
> -		       __func__);
> -		atomic_set(&data.error, -EBUSY);
> +	if (!cpumask_equal(cpu_present_mask, cpu_online_mask)) {
> +		pr_info("%s: Raced against a concurrent CPU-Offline\n", __func__);
> +		atomic_set(&data.error, -EAGAIN);
>  		goto out_hotplug_enable;
>  	}