Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] DA_ALLOC_POOL strategy causes lockdep 'Invalid wait context' warnings and crashes on PREEMPT_RT when used in tracepoints that hold raw spinlocks. --
--- Patch [1]: [PATCH v5 1/9] rv: Introduce DA_MON_ALLOCATION_STRATEGY --- commit e8961cb4205551f74e01ec6b7818734bafc846f3 Author: Wen Yang <[email protected]> rv: Introduce DA_MON_ALLOCATION_STRATEGY This patch introduces a compile-time selection for per-object deterministic automata (DA) monitor allocation strategies. It adds DA_ALLOC_AUTO, DA_ALLOC_POOL, and DA_ALLOC_MANUAL to handle contexts where dynamic memory allocation is problematic. > diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h > index e3cf85c9ce556..48c534324cbb2 100644 > --- a/include/rv/da_monitor.h > +++ b/include/rv/da_monitor.h [ ... ] > @@ -543,6 +596,59 @@ static inline monitor_target > da_get_target_by_id(da_id_type id) > return mon_storage->target; > } > > +/* > + * Pre-allocated mempool for DA_ALLOC_POOL monitors: DA_MON_POOL_SIZE > + * slots, eager-allocated at init. mempool_alloc_preallocated() pops a > + * slot without touching the allocator (bounded start latency; NULL when > + * exhausted). mempool_free() is safe from RCU-callback context. > + * Non-pool monitors get a zero-initialised mempool_t; pool paths compile > + * away. > + */ > +static mempool_t da_monitor_pool; [ ... ] > +/* > + * da_create_pool_storage - pop a free pool slot and insert it into the hash. > + * > + * Returns the new da_monitor, or NULL if the pool is exhausted. Finding > + * an existing entry for the same id fires WARN_ON_ONCE (double-start bug). > + * > + * Caller must hold an RCU read-side CS and the monitor's serialisation lock. > + */ > +static inline struct da_monitor * > +da_create_pool_storage(da_id_type id, monitor_target target, > + struct da_monitor *da_mon) > +{ > + struct da_monitor_storage *mon_storage, *existing; > + > + if (da_mon) > + return da_mon; > + > + mon_storage = mempool_alloc_preallocated(&da_monitor_pool); [Severity: High] Will this cause an "Invalid wait context" lockdep warning or a kernel panic on PREEMPT_RT systems when executed from tracepoints holding raw spinlocks? The patch describes DA_ALLOC_POOL as being useful for contexts problematic for allocation, such as scheduling. If a monitor uses this pool strategy in a tracepoint like sched_switch, it will be called while holding a raw_spinlock_t (like rq->lock). Looking at mempool_alloc_preallocated(), it unconditionally acquires the pool lock, which is a spinlock_t. On PREEMPT_RT, spinlock_t maps to a sleepable rt_mutex. Acquiring a sleeping lock while holding a raw_spinlock_t is a strict wait-context locking violation. Is there a way to manage the pre-allocated pool in da_create_pool_storage() without relying on locks that sleep on PREEMPT_RT? > + if (!mon_storage) > + return NULL; > + memset(mon_storage, 0, sizeof(*mon_storage)); > + > + mon_storage->id = id; > + mon_storage->target = target; [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
