Re: [PATCH v2 1/4] bootchooser: implement locking of attempts counter

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 6/16/25 17:06, Lars Schmidt wrote:
> This this new global slot lock inhibits the remaining attempts counter
> from decreasing.
> There are ways around this, but both come with a disadvantage:
>   - If we mark the old slot as bad after a good update, we lose the
>     distinction between old slots that are unbootable and ones that
>     are bootable. Maintaining this distinction allows a health monitor
>     to explicitly boot an old slot while preventing the bootloader
>     from doing it automatically
>   - If we set the maximum attempts to a very high number, we keep
>     writing the storage every boot, even if we don't do
> In both cases, by not decreasing and increasing the remaining attempts
> counter constantly the number of write cycles is also lowered.
> 
> Signed-off-by: Lars Schmidt <l.schmidt@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  common/bootchooser.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/common/bootchooser.c b/common/bootchooser.c
> index 58032a2b576284fe65ee53ee0e49aea68e09697a..50ce73682fb6bac8e4319119169dfb8334eb8762 100644
> --- a/common/bootchooser.c
> +++ b/common/bootchooser.c
> @@ -49,6 +49,7 @@ struct bootchooser {
>  	struct state *state;
>  	char *state_prefix;
>  	int refs;
> +	bool attempts_locked;
>  
>  	int verbose;
>  	int dryrun;
> @@ -353,6 +354,7 @@ struct bootchooser *bootchooser_get(void)
>  	int ret = -EINVAL, id = 1;
>  	uint32_t last_chosen;
>  	static int attempts_resetted;
> +	uint32_t locked;
>  
>  	if (bootchooser) {
>  		bootchooser->refs++;
> @@ -397,6 +399,14 @@ struct bootchooser *bootchooser_get(void)
>  		pr_warn("using non-redundant NV instead of barebox-state\n");
>  	}
>  
> +	/* this is an optional value */
> +	bc->attempts_locked = false;
> +	ret = getenv_u32(bc->state_prefix, "attempts_locked", &locked);
> +	if (!ret && locked) {
> +		bc->attempts_locked = true;
> +		pr_debug("remaining attempt counter is locked\n");
> +	}
> +
>  	INIT_LIST_HEAD(&bc->targets);
>  
>  	freep = targets = xstrdup(available_targets);
> @@ -650,11 +660,14 @@ static struct bootchooser_target *bootchooser_get_target(struct bootchooser *bc)
>  	return ERR_PTR(-ENOENT);
>  
>  found:
> -	target->remaining_attempts--;
> -
> -	if (bc->verbose)
> -		pr_info("name=%s decrementing remaining_attempts to %d\n",
> -			target->name, target->remaining_attempts);
> +	if (!bc->attempts_locked) {
> +		target->remaining_attempts--;
> +		if (bc->verbose)
> +			pr_info("name=%s remaining_attempts %d\n", target->name,
> +				target->remaining_attempts);
> +	} else {
> +		pr_info("Attempts are locked, not decreasing remaining_attempts\n");
> +	}
>  
>  	if (bc->verbose)
>  		pr_info("selected target '%s', boot '%s'\n", target->name, target->boot);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |