Re: [PATCH v10 15/25] x86/virt/seamldr: Abort updates after a failed step

[Dave Hansen]

On 5/20/26 06:38, Chao Gao wrote:
> +static void ack_state(struct update_ctrl *ctrl, int result)
>  {
>  	raw_spin_lock(&ctrl->lock);
>  
> +	ctrl->num_failed += !!result;
>  	ctrl->num_ack++;...> @@ -239,8 +242,8 @@ static int do_seamldr_install_module(void
*seamldr_params)
>  			break;
>  		}
>  
> -		ack_state(&update_ctrl);
> -	} while (curstate != MODULE_UPDATE_DONE);
> +		ack_state(&update_ctrl, ret);
> +	} while (curstate != MODULE_UPDATE_DONE && !READ_ONCE(update_ctrl.num_failed));

The READ_ONCE() is cute. But it's not really effective. It's also
overly-complicated.

update_ctrl.num_failed is just a single. Nothing cares if it is 1 or 2
or 999. So why have a count?

Any reason this won't work?

	if (result)
		set_bit(0, ctrl->failed);

... and on the read side:

	test_bit(0, &update_ctrl.failed)

That's 100% non-ambiguous. It doesn't have a counter where one isn't
needed. It also can't even theoretically be messed up by the compiler.