Re: [PATCH v7 00/14] rv: Add task latency over budget RV monitor

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-10-26 at 12:05 +0100, Gabriele Monaco wrote:
> By the way there has been a running discussion about keeping the C files for
> the monitors as close as possible to the DOT representation, with a long-term
> goal to extend the DOT to represent everything and have logic represented
> there too (at least simple trace filters).
> ...
> These are general guidelines, the C files cannot yet be a 1-to-1 product of
> the model, but shouldn't be a completely different story either.

To push this even further, why don't we get the framework allocate also the
tlob_record ?
I played a bit to adapt the PER_OBJ things to accept non-pointer targets. Now
this is far from clean and is /not/ a recommendation over how the API should look
like, but it seems to work and simplified tlob even further.

Nothing forbids us to embed data with the monitor state also for other types in
the future (besides in-kernel PER_TASK, that must be fixed).

What do you think?

diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index 04caaf529034..4677e19b228d 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -23,6 +23,17 @@
  *                   must run in task context; other handlers only call
  *                   da_handle_event() and never allocate.
  * DA_ALLOC_MANUAL - caller pre-inserts storage; framework links the target.
+ *
+ * The target type: a PER_OBJ monitor usually typedefs monitor_target itself,
+ * as a pointer to the monitored object (borrowed: the object exists
+ * independently of the monitor and outlives the storage).  A monitor whose
+ * target is monitor-owned data can instead define DA_OBJ_EMBEDDED_TYPE with
+ * a complete struct type: the framework then derives monitor_target as a
+ * pointer to it, embeds one instance in each per-object storage, and
+ * allocates and frees it together with the storage.  On storage creation the
+ * caller-supplied target is shallow-copied into the storage before it is
+ * published, so it may be a fully initialised stack object.  Do not define
+ * monitor_target by hand when using DA_OBJ_EMBEDDED_TYPE.
  */
 #define DA_ALLOC_AUTO   0
 #define DA_ALLOC_POOL   1
@@ -61,6 +72,10 @@
 #include <linux/slab.h>
 #include <linux/hashtable.h>
 
+#if defined(DA_OBJ_EMBEDDED_TYPE) && RV_MON_TYPE != RV_MON_PER_OBJ
+#error "DA_OBJ_EMBEDDED_TYPE is only supported by RV_MON_PER_OBJ monitors"
+#endif
+
 /*
  * Per-cpu variables require a unique name although static in some
  * configurations (e.g. alpha modules).
@@ -463,9 +478,24 @@ static inline void da_monitor_destroy(void)
  * Functions to define, init and get a per-object monitor.
  */
 
+#ifdef DA_OBJ_EMBEDDED_TYPE
+/*
+ * Framework-owned target: one instance is embedded in each storage and
+ * allocated/freed with it.  monitor_target keeps pointer semantics, but
+ * da_get_target() points into the storage rather than to a borrowed object.
+ * The header derives the typedef: do not define monitor_target by hand.
+ */
+typedef DA_OBJ_EMBEDDED_TYPE *monitor_target;
+#endif
+
 struct da_monitor_storage {
 	da_id_type id;
+#ifdef DA_OBJ_EMBEDDED_TYPE
+	DA_OBJ_EMBEDDED_TYPE target;
+	bool filled;
+#else
 	monitor_target target;
+#endif
 	union rv_task_monitor rv;
 	struct hlist_node node;
 	struct rcu_head rcu;
@@ -476,6 +506,38 @@ struct da_monitor_storage {
 #endif
 static DEFINE_HASHTABLE(da_monitor_ht, DA_MONITOR_HT_BITS);
 
+#ifdef DA_OBJ_EMBEDDED_TYPE
+
+#define TARGET_PTR(storage) &storage->target
+
+static inline bool da_storage_is_empty(struct da_monitor_storage *mon_storage)
+{
+	return !smp_load_acquire(&mon_storage->filled);
+}
+static inline void da_storage_set_target(struct da_monitor_storage *mon_storage,
+					 monitor_target target)
+{
+	if (WARN_ON_ONCE(!target))
+		return;
+
+	memcpy(&mon_storage->target, target, sizeof(mon_storage->target));
+	smp_store_release(&mon_storage->filled, true);
+}
+#else
+
+#define TARGET_PTR(storage) storage->target
+
+static inline bool da_storage_is_empty(struct da_monitor_storage *mon_storage)
+{
+	return !TARGET_PTR(mon_storage);
+}
+static inline void da_storage_set_target(struct da_monitor_storage *mon_storage,
+					 monitor_target target)
+{
+	mon_storage->target = target;
+}
+#endif
+
 /*
  * da_create_empty_storage - pre-allocate an empty storage
  */
@@ -513,7 +575,7 @@ static inline struct da_monitor *da_create_storage(da_id_type id,
 	if (!mon_storage)
 		return NULL;
 
-	mon_storage->target = target;
+	da_storage_set_target(mon_storage, target);
 	return &mon_storage->rv.da_mon;
 }
 
@@ -549,7 +611,7 @@ static struct da_monitor *da_get_monitor(da_id_type id, monitor_target target)
  */
 static inline monitor_target da_get_target(struct da_monitor *da_mon)
 {
-	return container_of(da_mon, struct da_monitor_storage, rv.da_mon)->target;
+	return TARGET_PTR(container_of(da_mon, struct da_monitor_storage, rv.da_mon));
 }
 
 /*
@@ -582,8 +644,11 @@ static inline struct da_monitor *da_fill_empty_storage(da_id_type id,
 						       monitor_target target,
 						       struct da_monitor *da_mon)
 {
-	if (unlikely(da_mon && !da_get_target(da_mon)))
-		container_of(da_mon, struct da_monitor_storage, rv.da_mon)->target = target;
+	struct da_monitor_storage *mon_storage =
+		container_of(da_mon, struct da_monitor_storage, rv.da_mon);
+
+	if (unlikely(da_mon && da_storage_is_empty(mon_storage)))
+		da_storage_set_target(mon_storage, target);
 	return da_mon;
 }
 
@@ -599,7 +664,7 @@ static inline monitor_target da_get_target_by_id(da_id_type id)
 
 	if (unlikely(!mon_storage))
 		return NULL;
-	return mon_storage->target;
+	return TARGET_PTR(mon_storage);
 }
 
 static void da_kfree_cb(struct rcu_head *head)
@@ -645,7 +710,7 @@ da_create_pool_storage(da_id_type id, monitor_target target,
 	memset(mon_storage, 0, sizeof(*mon_storage));
 
 	mon_storage->id = id;
-	mon_storage->target = target;
+	da_storage_set_target(mon_storage, target);
 
 	hash_add_rcu(da_monitor_ht, &mon_storage->node, id);
 	return &mon_storage->rv.da_mon;