Re: [PATCH] rv: Simplify task monitor slot management

[Gabriele Monaco <gmonaco@redhat.com>]

On Tue, 2026-07-14 at 16:33 +0800, liqiang wrote:
> The slot array already tracks allocation and task_monitor_count
> duplicates that state. On an invalid second release, the old code
> warns but still decrements the counter, corrupting later allocations.
> 
> Use the slot array as the sole source of truth. Return after warning
> about an unused slot, and return -EBUSY when no slot is free.
> 
> Signed-off-by: liqiang <liqiang01@kylinos.cn>

Thanks for the patch! It looks good to me:

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>

However, your signoff should include a "known identity" [1]. For most people
this is an official name and surname (Li Qiang ?).

You could probably just reply with the new signoff, but it's probably better you
send a V2 with it updated. Feel free to also include my review tag.

I suggest you configure git accordingly e.g.:

  git config set user.name "Name Surname"
  git commit --amend --reset-author --signoff
  # will open an editor from there remove the old signoff and add the review tag

Thanks,
Gabriele

[1] -
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

> ---
>  kernel/trace/rv/rv.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c
> index ee4e68102f17..187d87d5991c 100644
> --- a/kernel/trace/rv/rv.c
> +++ b/kernel/trace/rv/rv.c
> @@ -164,7 +164,6 @@ struct dentry *get_monitors_root(void)
>   */
>  LIST_HEAD(rv_monitors_list);
>  
> -static int task_monitor_count;
>  static bool task_monitor_slots[CONFIG_RV_PER_TASK_MONITORS];
>  
>  int rv_get_task_monitor_slot(void)
> @@ -173,21 +172,14 @@ int rv_get_task_monitor_slot(void)
>  
>  	lockdep_assert_held(&rv_interface_lock);
>  
> -	if (task_monitor_count == CONFIG_RV_PER_TASK_MONITORS)
> -		return -EBUSY;
> -
> -	task_monitor_count++;
> -
>  	for (i = 0; i < CONFIG_RV_PER_TASK_MONITORS; i++) {
> -		if (task_monitor_slots[i] == false) {
> +		if (!task_monitor_slots[i]) {
>  			task_monitor_slots[i] = true;
>  			return i;
>  		}
>  	}
>  
> -	WARN_ONCE(1, "RV task_monitor_count and slots are out of sync\n");
> -
> -	return -EINVAL;
> +	return -EBUSY;
>  }
>  
>  void rv_put_task_monitor_slot(int slot)
> @@ -199,10 +191,10 @@ void rv_put_task_monitor_slot(int slot)
>  		return;
>  	}
>  
> -	WARN_ONCE(!task_monitor_slots[slot], "RV releasing unused
> task_monitor_slots: %d\n",
> -		  slot);
> +	if (WARN_ONCE(!task_monitor_slots[slot],
> +		      "RV releasing unused task monitor slot: %d\n", slot))
> +		return;
>  
> -	task_monitor_count--;
>  	task_monitor_slots[slot] = false;
>  }
>