Re: [PATCH v2 20/20] rv: Add deadline monitors

[Gabriele Monaco <gmonaco@redhat.com>]

On Fri, 2025-10-10 at 17:04 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > +/*
> > + * Dummy values if not available
> > + */
> > +#ifndef __NR_sched_setscheduler
> > +#define __NR_sched_setscheduler -1
> > +#endif
> > +#ifndef __NR_sched_setattr
> > +#define __NR_sched_setattr -2
> > +#endif
> 
> You can use __COUNTER__ intead, would make it easier to add to or remove
> from this list later on.

Right, good point.

> > +static void handle_syscall(void *data, struct pt_regs *regs, long id)
> > +{
> > +	struct task_struct *p;
> > +	int new_policy = -1;
> > +
> > +	new_policy = extract_params(regs, id, &p);
> > +	if (new_policy < 0 || new_policy == p->policy)
> > +		return;
> > +	if (p->policy == SCHED_DEADLINE)
> > +		da_reset(EXPAND_ID(&p->dl, DL_TASK));
> > +	else if (new_policy == SCHED_DEADLINE)
> > +		da_create_conditional(EXPAND_ID(&p->dl, DL_TASK),
> > GFP_NOWAIT);
> > +}
> 
> What if the syscall fails for any reason? Wouldn't the monitor stores
> incorrect information?

Yes, that could happen, this seems an issue only in the throttle monitor when
failing a DEADLINE->NORMAL, other cases are at most resetting the monitor.

In this case, the monitor is really saying: a deadline task is no longer
deadline, so its runtime should be counted as fair server. If the syscall does
fail, we are assuming the fair server as armed (not running) for a bit more.

Here I couldn't find a nice way to handle this: if I attach only to the syscall
return, I see if it succeeded but I lost the previous policy, attaching to both
(and keep a per-cpu storage just for this process) seems a bit overcomplicated.
I see this can potentially lead to errors in the monitor, but I'm not sure the
extra complexity is worth it here, considering this failure might only happen if
a user really makes wrong calls.
I tried to avoid creating a new tracepoint on a successful policy change.

What do you think?

Thanks,
Gabriele