Re: [PATCH 2/2] liveupdate: Remember FLB retrieve() status

[Pasha Tatashin]

On 05-28 17:41, David Matlack wrote:
> LUO keeps track of successful retrieve attempts on an FLB. It does so
> to avoid multiple retrievals of the same FLB. Multiple retrievals cause
> problems because once the FLB is retrieved, the serialized data
> structures are likely freed and the FLB is likely in a very different
> state from what the code expects.
> 
> All this works well when retrieve succeeds. When it fails,
> luo_flb_retrieve_one() returns the error immediately, without ever
> storing anywhere that a retrieve was attempted or what its error code
> was. If the user attempts to retrieve another file registered with the
> same FLB, LUO will attempt to call the FLB's retrieve() callback again.
> 
> The retry is problematic for much of the same reasons listed above. The
> FLB is likely in a very different state than what the retrieve logic
> normally expects (e.g. some KHO pages may have already been restored and
> freed).
> 
> There is no sane way of attempting the retrieve again. Remember the
> error retrieve returned and directly return it on a retry.
> 
> This is done by changing the retrieved bool to a retrieve_status
> integer. A value of 0 means retrieve was never attempted, a positive
> value means it succeeded, and a negative value means it failed and the
> error code is the value.
> 
> This is similar to commit f85b1c6af5bc ("liveupdate: luo_file: remember
> retrieve() status") which did the same for LUO files.
> 
> Fixes: cab056f2aae7 ("liveupdate: luo_flb: introduce File-Lifecycle-Bound global state")
> Assisted-by: Gemini:gemini-3-pro-preview
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>

Reviewed-by: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx> 

> ---
>  include/linux/liveupdate.h  |  6 ++++--
>  kernel/liveupdate/luo_flb.c | 10 +++++++---
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h
> index c344bf987b63..63ea5417de84 100644
> --- a/include/linux/liveupdate.h
> +++ b/include/linux/liveupdate.h
> @@ -173,7 +173,9 @@ struct liveupdate_flb_ops {
>   * @lock:      A mutex that protects all fields within this structure, providing
>   *             the synchronization service for the FLB's ops.
>   * @finished:  True once the FLB's finish() callback has run.
> - * @retrieved: True once the FLB's retrieve() callback has run.
> + * @retrieve_status: Status code indicating whether retrieve() has been
> + *                   attempted. 0 means not attempted, 1 means successful,
> + *                   and negative value means it failed with that error code.
>   */
>  struct luo_flb_private_state {
>  	refcount_t count;
> @@ -181,7 +183,7 @@ struct luo_flb_private_state {
>  	void *obj;
>  	struct mutex lock;
>  	bool finished;
> -	bool retrieved;
> +	int retrieve_status;
>  };
>  
>  /*
> diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
> index 7ddef552ff6b..f8852f7e62e5 100644
> --- a/kernel/liveupdate/luo_flb.c
> +++ b/kernel/liveupdate/luo_flb.c
> @@ -170,7 +170,10 @@ static int luo_flb_retrieve_one(struct liveupdate_flb *flb)
>  	if (private->incoming.finished)
>  		return -ENODATA;
>  
> -	if (private->incoming.retrieved)
> +	if (private->incoming.retrieve_status < 0)
> +		return private->incoming.retrieve_status;
> +
> +	if (private->incoming.retrieve_status > 0)
>  		return 0;
>  
>  	if (!fh->active)
> @@ -196,12 +199,13 @@ static int luo_flb_retrieve_one(struct liveupdate_flb *flb)
>  
>  	err = flb->ops->retrieve(&args);
>  	if (err) {
> +		private->incoming.retrieve_status = err;
>  		module_put(flb->ops->owner);
>  		return err;
>  	}
>  
>  	private->incoming.obj = args.obj;
> -	private->incoming.retrieved = true;
> +	private->incoming.retrieve_status = 1;
>  
>  	return 0;
>  }
> @@ -215,7 +219,7 @@ void liveupdate_flb_put_incoming(struct liveupdate_flb *flb)
>  		if (!refcount_dec_and_test(&private->incoming.count))
>  			return;
>  
> -		if (!private->incoming.retrieved) {
> +		if (private->incoming.retrieve_status <= 0) {
>  			int err = luo_flb_retrieve_one(flb);
>  
>  			if (WARN_ON(err))
> -- 
> 2.54.0.823.g6e5bcc1fc9-goog
>