Hi Gabriele and Nam,

Sep 23, 2025 09:43:05 Gabriele Monaco [off-list ref]:

On Mon, 2025-09-22 at 18:29 +0200, Nam Cao wrote:

quoted

On Fri, Sep 19, 2025 at 02:26:12PM +0200, Gabriele Monaco wrote:

quoted

On Fri, 2025-09-19 at 12:49 +0200, Thomas Weißschuh wrote:

quoted

+static void rv_reaction_signal(int signal, const char *fmt, va_list args)
+{
+   struct rv_signal_work *work;
+   char message[256];
+
+   work = mempool_alloc_preallocated(rv_signal_task_work_pool);
+   if (!work) {
+       pr_warn_ratelimited("Unable to signal through task_work,
sending directly\n");
+       vsnprintf(message, sizeof(message), fmt, args);
+       rv_signal_force_sig(signal, message);
+       return;
+   }

Why do you use the task_work at all instead of signalling directly?
If that's something not safe from a (any) tracepoint because it can sleep

If I remember correctly, sending signals requires a spinlock and therefore
may sleep on PREEMPT_RT.

Yeah that's what I quickly glanced at. Which seems to be the case also for
mempool_alloc_preallocated by the way, so I'm not sure that's safer than
signalling directly on PREEMPT_RT.

Thomas, did you test your reactor on PREEMPT_RT? I'd expect a few fat warnings
when this is called from sched tracepoints. Unless you're lucky and never get
contention. Lockdep (CONFIG_PROVE_LOCKING) may help here.

I trusted the documentation, which promised not to sleep.
I'll rework it for v2.

Thanks,
Gabriele

quoted

quoted

you should definitely not call it if allocation fails.

Yep.

Ack.

quoted

We probably can get away with not reacting at all if allocation fails, by
crafting our tests such that only one reaction happens at a time, and
allocation won't fail.

Ack.


I am wondering if it would make sense to add a new tracepoint that fires in addition of the reactors.
That would allow multiple simultaneous consumers and also bespoke handlers in userspace.


Thomas