Re: [PATCH 1/2] sched_ext: Track currently locked rq

Tejun Heo
2025-04-21T19:03:09Z

Hello,

On Sun, Apr 20, 2025 at 09:30:21PM +0200, Andrea Righi wrote:
...
> +static inline struct rq *scx_locked_rq(void)
> +{
> + return __this_cpu_read(locked_rq);
> +}
> +
> +#define SCX_CALL_OP(mask, rq, op, args...) \
> do { \
> + update_locked_rq(rq); \

Minor but why not

if (rq)
update_locked_rq(rq);

here too to be symmetric?

> if (mask) { \
> scx_kf_allow(mask); \
> scx_ops.op(args); \
> @@ -1125,11 +1155,15 @@ do { \
> } else { \
> scx_ops.op(args); \
> } \
> + if (rq) \
> + update_locked_rq(NULL); \

Or alternatively, drop `if (rq)` from both places. That's simpler and given
that all the hot paths are called with rq locked, that may be *minutely*
faster.

> @@ -2174,7 +2210,7 @@ static void do_enqueue_task(struct rq *rq, struct task_struct *p, u64 enq_flags,
> WARN_ON_ONCE(*ddsp_taskp);
> *ddsp_taskp = p;
>
> - SCX_CALL_OP_TASK(SCX_KF_ENQUEUE, enqueue, p, enq_flags);
> + SCX_CALL_OP_TASK(SCX_KF_ENQUEUE, rq, enqueue, p, enq_flags);

Let's do SCX_CALL_OP_TASK(SCX_FK_ENQUEUE, enqueue, rq, p, enq_flags) so that
the static parts of the invocation are grouped together and we usually have
@rq and @p next to each other when they're used as parameters.

Thanks.

--
tejun