Re: [PATCH 9/9] rv: Mandate deallocation for per-obj monitors

[Wen Yang <wen.yang@linux.dev>]

One gap: tools/verification/rvgen/rvgen/templates/dot2k/main.c uses 
RV_MON_%%MONITOR_TYPE%% but generates no deallocation code, may fail
to build with a -Wunused-function warning.


--
Best wishes,
Wen

On 5/12/26 22:02, Gabriele Monaco wrote:
> The per-object monitors use a hash tables and dynamic allocation of the
> monitor storage, functions to clean a monitor that is no longer needed
> are provided but nothing ensures the monitor actually uses them.
> 
> Remove the inline specifier on the deallocation function to let the
> compiler warn in case it isn't referenced. If the monitor really doesn't
> need one (for instance because instances will never cease to exist
> before disabling the monitor), the da_skip_deallocation() helper macro
> can be used to silence the warning.
> 
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>   include/rv/da_monitor.h                      | 14 +++++++++++++-
>   kernel/trace/rv/monitors/deadline/deadline.h |  5 ++++-
>   2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 402d3b935c08..378d23ab7dfb 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -489,8 +489,11 @@ static inline monitor_target da_get_target_by_id(da_id_type id)
>    * locks.
>    * This function includes an RCU read-side critical section to synchronise
>    * against da_monitor_destroy().
> + * NOTE: inline is omitted on purpose to let the compiler warn if this function
> + * is never referenced. For monitors that don't require a deallocation hook,
> + * da_skip_deallocation() can be used.
>    */
> -static inline void da_destroy_storage(da_id_type id)
> +static void da_destroy_storage(da_id_type id)
>   {
>   	struct da_monitor_storage *mon_storage;
>   
> @@ -504,6 +507,15 @@ static inline void da_destroy_storage(da_id_type id)
>   	kfree_rcu(mon_storage, rcu);
>   }
>   
> +/*
> + * da_skip_deallocation - explicitly mark a deallocation function as not required
> + *
> + * Only use when you are absolutely sure the monitor doesn't require a
> + * deallocation hook (i.e. it's not possible for an object to finish existing
> + * when the monitor is still running).
> + */
> +#define da_skip_deallocation(hook) ((void)hook)
> +
>   static void da_monitor_reset_all(void)
>   {
>   	struct da_monitor_storage *mon_storage;
> diff --git a/kernel/trace/rv/monitors/deadline/deadline.h b/kernel/trace/rv/monitors/deadline/deadline.h
> index 78fca873d61e..c39fd79148c2 100644
> --- a/kernel/trace/rv/monitors/deadline/deadline.h
> +++ b/kernel/trace/rv/monitors/deadline/deadline.h
> @@ -194,7 +194,10 @@ static void __maybe_unused handle_newtask(void *data, struct task_struct *task,
>   		da_create_storage(EXPAND_ID_TASK(task), NULL);
>   }
>   
> -static void __maybe_unused handle_exit(void *data, struct task_struct *p, bool group_dead)
> +/*
> + * Deallocation hook, use da_skip_deallocation() when not necessary
> + */
> +static void handle_exit(void *data, struct task_struct *p, bool group_dead)
>   {
>   	if (p->policy == SCHED_DEADLINE)
>   		da_destroy_storage(get_entity_id(&p->dl, DL_TASK, DL_TASK));