On 5/18/26 14:36, Gabriele Monaco wrote:
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?


Hi Gabriele,

Thanks for the patch and the discussion.

I wonder if generating a dummy hook that calls da_destroy_storage by default is the best way to go. Given that da_destroy_storage internally uses kfree_rcu(), it might introduce unnecessary memory allocation/free overhead and could even affect RCU grace periods -- especially for monitors that never actually need to release objects.

Perhaps a gentler approach for rvgen would be to generate a commented example in the template, showing how to use da_skip_deallocation to silence the warning.

--
Best wishes,
Wen




--
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