On Sun, 2026-05-17 at 17:52 +0800, Wen Yang wrote:
> 
> 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.
> 

Thanks for the review!

That's technically the purpose of this patch, we don't know exactly how is the
per-obj monitor going to deallocate, so we make sure build fails if they don't
set up a way.

This combined with the fact per-obj monitors aren't really documented (yet),
makes it quite confusing, doesn't it?

Would you prefer we always generate a dummy hook calling da_destroy_storage()
and let the user decide what to do with it without forcing (obscure) compiler
warnings?

Thanks,
Gabriele

> 
> --
> 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 <[email protected]>
> > ---
> >   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));


Reply via email to