From: Andrea Righi <arighi@nvidia.com>
To: Juntong Deng <juntong.deng@outlook.com>
Cc: ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
andrii@kernel.org, martin.lau@linux.dev, eddyz87@gmail.com,
song@kernel.org, yonghong.song@linux.dev, kpsingh@kernel.org,
sdf@fomichev.me, haoluo@google.com, jolsa@kernel.org,
memxor@gmail.com, tj@kernel.org, void@manifault.com,
changwoo@igalia.com, bpf@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH sched_ext/for-6.15 v3 4/5] sched_ext: Removed mask-based runtime restrictions on calling kfuncs in different contexts
Date: Thu, 27 Feb 2025 17:24:21 +0100 [thread overview]
Message-ID: <Z8CRtWS117dVEnFa@gpd3> (raw)
In-Reply-To: <AM6PR03MB5080BDE038C8E8E89996F30E99C22@AM6PR03MB5080.eurprd03.prod.outlook.com>
On Wed, Feb 26, 2025 at 07:28:19PM +0000, Juntong Deng wrote:
> Currently, kfunc filters already support filtering based on struct_ops
> context information.
>
> The BPF verifier can check context-sensitive kfuncs before the SCX
> program is run, avoiding runtime overhead.
>
> Therefore we no longer need mask-based runtime restrictions.
>
> This patch removes the mask-based runtime restrictions.
You may have missed scx_prio_less(), that is still using SCX_KF_REST:
kernel/sched/ext.c: In function ‘scx_prio_less’:
kernel/sched/ext.c:1171:27: error: ‘struct sched_ext_ops’ has no member named ‘SCX_KF_REST’
1171 | __typeof__(scx_ops.op(task0, task1, ##args)) __ret; \
| ^
I think you just need to add:
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 4a4713c3af67b..51c13b8c27743 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -3302,7 +3302,7 @@ bool scx_prio_less(const struct task_struct *a, const struct task_struct *b,
* verifier.
*/
if (SCX_HAS_OP(core_sched_before) && !scx_rq_bypassing(task_rq(a)))
- return SCX_CALL_OP_2TASKS_RET(SCX_KF_REST, core_sched_before,
+ return SCX_CALL_OP_2TASKS_RET(core_sched_before,
(struct task_struct *)a,
(struct task_struct *)b);
else
-Andrea
>
> Signed-off-by: Juntong Deng <juntong.deng@outlook.com>
> ---
> include/linux/sched/ext.h | 24 ----
> kernel/sched/ext.c | 227 ++++++++------------------------------
> kernel/sched/ext_idle.c | 5 +-
> 3 files changed, 50 insertions(+), 206 deletions(-)
>
> diff --git a/include/linux/sched/ext.h b/include/linux/sched/ext.h
> index f7545430a548..9980d6b55c84 100644
> --- a/include/linux/sched/ext.h
> +++ b/include/linux/sched/ext.h
> @@ -96,29 +96,6 @@ enum scx_ent_dsq_flags {
> SCX_TASK_DSQ_ON_PRIQ = 1 << 0, /* task is queued on the priority queue of a dsq */
> };
>
> -/*
> - * Mask bits for scx_entity.kf_mask. Not all kfuncs can be called from
> - * everywhere and the following bits track which kfunc sets are currently
> - * allowed for %current. This simple per-task tracking works because SCX ops
> - * nest in a limited way. BPF will likely implement a way to allow and disallow
> - * kfuncs depending on the calling context which will replace this manual
> - * mechanism. See scx_kf_allow().
> - */
> -enum scx_kf_mask {
> - SCX_KF_UNLOCKED = 0, /* sleepable and not rq locked */
> - /* ENQUEUE and DISPATCH may be nested inside CPU_RELEASE */
> - SCX_KF_CPU_RELEASE = 1 << 0, /* ops.cpu_release() */
> - /* ops.dequeue (in REST) may be nested inside DISPATCH */
> - SCX_KF_DISPATCH = 1 << 1, /* ops.dispatch() */
> - SCX_KF_ENQUEUE = 1 << 2, /* ops.enqueue() and ops.select_cpu() */
> - SCX_KF_SELECT_CPU = 1 << 3, /* ops.select_cpu() */
> - SCX_KF_REST = 1 << 4, /* other rq-locked operations */
> -
> - __SCX_KF_RQ_LOCKED = SCX_KF_CPU_RELEASE | SCX_KF_DISPATCH |
> - SCX_KF_ENQUEUE | SCX_KF_SELECT_CPU | SCX_KF_REST,
> - __SCX_KF_TERMINAL = SCX_KF_ENQUEUE | SCX_KF_SELECT_CPU | SCX_KF_REST,
> -};
> -
> enum scx_dsq_lnode_flags {
> SCX_DSQ_LNODE_ITER_CURSOR = 1 << 0,
>
> @@ -147,7 +124,6 @@ struct sched_ext_entity {
> s32 sticky_cpu;
> s32 holding_cpu;
> s32 selected_cpu;
> - u32 kf_mask; /* see scx_kf_mask above */
> struct task_struct *kf_tasks[2]; /* see SCX_CALL_OP_TASK() */
> atomic_long_t ops_state;
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index c337f6206ae5..7dc5f11be66b 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -1115,19 +1115,6 @@ static long jiffies_delta_msecs(unsigned long at, unsigned long now)
> return -(long)jiffies_to_msecs(now - at);
> }
>
> -/* if the highest set bit is N, return a mask with bits [N+1, 31] set */
> -static u32 higher_bits(u32 flags)
> -{
> - return ~((1 << fls(flags)) - 1);
> -}
> -
> -/* return the mask with only the highest bit set */
> -static u32 highest_bit(u32 flags)
> -{
> - int bit = fls(flags);
> - return ((u64)1 << bit) >> 1;
> -}
> -
> static bool u32_before(u32 a, u32 b)
> {
> return (s32)(a - b) < 0;
> @@ -1143,51 +1130,12 @@ static struct scx_dispatch_q *find_user_dsq(u64 dsq_id)
> return rhashtable_lookup_fast(&dsq_hash, &dsq_id, dsq_hash_params);
> }
>
> -/*
> - * scx_kf_mask enforcement. Some kfuncs can only be called from specific SCX
> - * ops. When invoking SCX ops, SCX_CALL_OP[_RET]() should be used to indicate
> - * the allowed kfuncs and those kfuncs should use scx_kf_allowed() to check
> - * whether it's running from an allowed context.
> - *
> - * @mask is constant, always inline to cull the mask calculations.
> - */
> -static __always_inline void scx_kf_allow(u32 mask)
> -{
> - /* nesting is allowed only in increasing scx_kf_mask order */
> - WARN_ONCE((mask | higher_bits(mask)) & current->scx.kf_mask,
> - "invalid nesting current->scx.kf_mask=0x%x mask=0x%x\n",
> - current->scx.kf_mask, mask);
> - current->scx.kf_mask |= mask;
> - barrier();
> -}
> -
> -static void scx_kf_disallow(u32 mask)
> -{
> - barrier();
> - current->scx.kf_mask &= ~mask;
> -}
> -
> -#define SCX_CALL_OP(mask, op, args...) \
> -do { \
> - if (mask) { \
> - scx_kf_allow(mask); \
> - scx_ops.op(args); \
> - scx_kf_disallow(mask); \
> - } else { \
> - scx_ops.op(args); \
> - } \
> -} while (0)
> +#define SCX_CALL_OP(op, args...) scx_ops.op(args)
>
> -#define SCX_CALL_OP_RET(mask, op, args...) \
> +#define SCX_CALL_OP_RET(op, args...) \
> ({ \
> __typeof__(scx_ops.op(args)) __ret; \
> - if (mask) { \
> - scx_kf_allow(mask); \
> - __ret = scx_ops.op(args); \
> - scx_kf_disallow(mask); \
> - } else { \
> - __ret = scx_ops.op(args); \
> - } \
> + __ret = scx_ops.op(args); \
> __ret; \
> })
>
> @@ -1202,74 +1150,36 @@ do { \
> * scx_kf_allowed_on_arg_tasks() to test whether the invocation is allowed on
> * the specific task.
> */
> -#define SCX_CALL_OP_TASK(mask, op, task, args...) \
> +#define SCX_CALL_OP_TASK(op, task, args...) \
> do { \
> - BUILD_BUG_ON((mask) & ~__SCX_KF_TERMINAL); \
> current->scx.kf_tasks[0] = task; \
> - SCX_CALL_OP(mask, op, task, ##args); \
> + SCX_CALL_OP(op, task, ##args); \
> current->scx.kf_tasks[0] = NULL; \
> } while (0)
>
> -#define SCX_CALL_OP_TASK_RET(mask, op, task, args...) \
> +#define SCX_CALL_OP_TASK_RET(op, task, args...) \
> ({ \
> __typeof__(scx_ops.op(task, ##args)) __ret; \
> - BUILD_BUG_ON((mask) & ~__SCX_KF_TERMINAL); \
> current->scx.kf_tasks[0] = task; \
> - __ret = SCX_CALL_OP_RET(mask, op, task, ##args); \
> + __ret = SCX_CALL_OP_RET(op, task, ##args); \
> current->scx.kf_tasks[0] = NULL; \
> __ret; \
> })
>
> -#define SCX_CALL_OP_2TASKS_RET(mask, op, task0, task1, args...) \
> +#define SCX_CALL_OP_2TASKS_RET(op, task0, task1, args...) \
> ({ \
> __typeof__(scx_ops.op(task0, task1, ##args)) __ret; \
> - BUILD_BUG_ON((mask) & ~__SCX_KF_TERMINAL); \
> current->scx.kf_tasks[0] = task0; \
> current->scx.kf_tasks[1] = task1; \
> - __ret = SCX_CALL_OP_RET(mask, op, task0, task1, ##args); \
> + __ret = SCX_CALL_OP_RET(op, task0, task1, ##args); \
> current->scx.kf_tasks[0] = NULL; \
> current->scx.kf_tasks[1] = NULL; \
> __ret; \
> })
>
> -/* @mask is constant, always inline to cull unnecessary branches */
> -static __always_inline bool scx_kf_allowed(u32 mask)
> -{
> - if (unlikely(!(current->scx.kf_mask & mask))) {
> - scx_ops_error("kfunc with mask 0x%x called from an operation only allowing 0x%x",
> - mask, current->scx.kf_mask);
> - return false;
> - }
> -
> - /*
> - * Enforce nesting boundaries. e.g. A kfunc which can be called from
> - * DISPATCH must not be called if we're running DEQUEUE which is nested
> - * inside ops.dispatch(). We don't need to check boundaries for any
> - * blocking kfuncs as the verifier ensures they're only called from
> - * sleepable progs.
> - */
> - if (unlikely(highest_bit(mask) == SCX_KF_CPU_RELEASE &&
> - (current->scx.kf_mask & higher_bits(SCX_KF_CPU_RELEASE)))) {
> - scx_ops_error("cpu_release kfunc called from a nested operation");
> - return false;
> - }
> -
> - if (unlikely(highest_bit(mask) == SCX_KF_DISPATCH &&
> - (current->scx.kf_mask & higher_bits(SCX_KF_DISPATCH)))) {
> - scx_ops_error("dispatch kfunc called from a nested operation");
> - return false;
> - }
> -
> - return true;
> -}
> -
> /* see SCX_CALL_OP_TASK() */
> -static __always_inline bool scx_kf_allowed_on_arg_tasks(u32 mask,
> - struct task_struct *p)
> +static __always_inline bool scx_kf_allowed_on_arg_tasks(struct task_struct *p)
> {
> - if (!scx_kf_allowed(mask))
> - return false;
> -
> if (unlikely((p != current->scx.kf_tasks[0] &&
> p != current->scx.kf_tasks[1]))) {
> scx_ops_error("called on a task not being operated on");
> @@ -1279,11 +1189,6 @@ static __always_inline bool scx_kf_allowed_on_arg_tasks(u32 mask,
> return true;
> }
>
> -static bool scx_kf_allowed_if_unlocked(void)
> -{
> - return !current->scx.kf_mask;
> -}
> -
> /**
> * nldsq_next_task - Iterate to the next task in a non-local DSQ
> * @dsq: user dsq being iterated
> @@ -2219,7 +2124,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(enqueue, p, enq_flags);
>
> *ddsp_taskp = NULL;
> if (p->scx.ddsp_dsq_id != SCX_DSQ_INVALID)
> @@ -2316,7 +2221,7 @@ static void enqueue_task_scx(struct rq *rq, struct task_struct *p, int enq_flags
> add_nr_running(rq, 1);
>
> if (SCX_HAS_OP(runnable) && !task_on_rq_migrating(p))
> - SCX_CALL_OP_TASK(SCX_KF_REST, runnable, p, enq_flags);
> + SCX_CALL_OP_TASK(runnable, p, enq_flags);
>
> if (enq_flags & SCX_ENQ_WAKEUP)
> touch_core_sched(rq, p);
> @@ -2351,7 +2256,7 @@ static void ops_dequeue(struct task_struct *p, u64 deq_flags)
> BUG();
> case SCX_OPSS_QUEUED:
> if (SCX_HAS_OP(dequeue))
> - SCX_CALL_OP_TASK(SCX_KF_REST, dequeue, p, deq_flags);
> + SCX_CALL_OP_TASK(dequeue, p, deq_flags);
>
> if (atomic_long_try_cmpxchg(&p->scx.ops_state, &opss,
> SCX_OPSS_NONE))
> @@ -2400,11 +2305,11 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flags
> */
> if (SCX_HAS_OP(stopping) && task_current(rq, p)) {
> update_curr_scx(rq);
> - SCX_CALL_OP_TASK(SCX_KF_REST, stopping, p, false);
> + SCX_CALL_OP_TASK(stopping, p, false);
> }
>
> if (SCX_HAS_OP(quiescent) && !task_on_rq_migrating(p))
> - SCX_CALL_OP_TASK(SCX_KF_REST, quiescent, p, deq_flags);
> + SCX_CALL_OP_TASK(quiescent, p, deq_flags);
>
> if (deq_flags & SCX_DEQ_SLEEP)
> p->scx.flags |= SCX_TASK_DEQD_FOR_SLEEP;
> @@ -2424,7 +2329,7 @@ static void yield_task_scx(struct rq *rq)
> struct task_struct *p = rq->curr;
>
> if (SCX_HAS_OP(yield))
> - SCX_CALL_OP_2TASKS_RET(SCX_KF_REST, yield, p, NULL);
> + SCX_CALL_OP_2TASKS_RET(yield, p, NULL);
> else
> p->scx.slice = 0;
> }
> @@ -2434,7 +2339,7 @@ static bool yield_to_task_scx(struct rq *rq, struct task_struct *to)
> struct task_struct *from = rq->curr;
>
> if (SCX_HAS_OP(yield))
> - return SCX_CALL_OP_2TASKS_RET(SCX_KF_REST, yield, from, to);
> + return SCX_CALL_OP_2TASKS_RET(yield, from, to);
> else
> return false;
> }
> @@ -2992,7 +2897,7 @@ static int balance_one(struct rq *rq, struct task_struct *prev)
> * emitted in switch_class().
> */
> if (SCX_HAS_OP(cpu_acquire))
> - SCX_CALL_OP(SCX_KF_REST, cpu_acquire, cpu_of(rq), NULL);
> + SCX_CALL_OP(cpu_acquire, cpu_of(rq), NULL);
> rq->scx.cpu_released = false;
> }
>
> @@ -3037,8 +2942,7 @@ static int balance_one(struct rq *rq, struct task_struct *prev)
> do {
> dspc->nr_tasks = 0;
>
> - SCX_CALL_OP(SCX_KF_DISPATCH, dispatch, cpu_of(rq),
> - prev_on_scx ? prev : NULL);
> + SCX_CALL_OP(dispatch, cpu_of(rq), prev_on_scx ? prev : NULL);
>
> flush_dispatch_buf(rq);
>
> @@ -3159,7 +3063,7 @@ static void set_next_task_scx(struct rq *rq, struct task_struct *p, bool first)
>
> /* see dequeue_task_scx() on why we skip when !QUEUED */
> if (SCX_HAS_OP(running) && (p->scx.flags & SCX_TASK_QUEUED))
> - SCX_CALL_OP_TASK(SCX_KF_REST, running, p);
> + SCX_CALL_OP_TASK(running, p);
>
> clr_task_runnable(p, true);
>
> @@ -3240,8 +3144,7 @@ static void switch_class(struct rq *rq, struct task_struct *next)
> .task = next,
> };
>
> - SCX_CALL_OP(SCX_KF_CPU_RELEASE,
> - cpu_release, cpu_of(rq), &args);
> + SCX_CALL_OP(cpu_release, cpu_of(rq), &args);
> }
> rq->scx.cpu_released = true;
> }
> @@ -3254,7 +3157,7 @@ static void put_prev_task_scx(struct rq *rq, struct task_struct *p,
>
> /* see dequeue_task_scx() on why we skip when !QUEUED */
> if (SCX_HAS_OP(stopping) && (p->scx.flags & SCX_TASK_QUEUED))
> - SCX_CALL_OP_TASK(SCX_KF_REST, stopping, p, true);
> + SCX_CALL_OP_TASK(stopping, p, true);
>
> if (p->scx.flags & SCX_TASK_QUEUED) {
> set_task_runnable(rq, p);
> @@ -3428,8 +3331,7 @@ static int select_task_rq_scx(struct task_struct *p, int prev_cpu, int wake_flag
> WARN_ON_ONCE(*ddsp_taskp);
> *ddsp_taskp = p;
>
> - cpu = SCX_CALL_OP_TASK_RET(SCX_KF_ENQUEUE | SCX_KF_SELECT_CPU,
> - select_cpu, p, prev_cpu, wake_flags);
> + cpu = SCX_CALL_OP_TASK_RET(select_cpu, p, prev_cpu, wake_flags);
> p->scx.selected_cpu = cpu;
> *ddsp_taskp = NULL;
> if (ops_cpu_valid(cpu, "from ops.select_cpu()"))
> @@ -3473,8 +3375,7 @@ static void set_cpus_allowed_scx(struct task_struct *p,
> * designation pointless. Cast it away when calling the operation.
> */
> if (SCX_HAS_OP(set_cpumask))
> - SCX_CALL_OP_TASK(SCX_KF_REST, set_cpumask, p,
> - (struct cpumask *)p->cpus_ptr);
> + SCX_CALL_OP_TASK(set_cpumask, p, (struct cpumask *)p->cpus_ptr);
> }
>
> static void handle_hotplug(struct rq *rq, bool online)
> @@ -3487,9 +3388,9 @@ static void handle_hotplug(struct rq *rq, bool online)
> scx_idle_update_selcpu_topology(&scx_ops);
>
> if (online && SCX_HAS_OP(cpu_online))
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cpu_online, cpu);
> + SCX_CALL_OP(cpu_online, cpu);
> else if (!online && SCX_HAS_OP(cpu_offline))
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cpu_offline, cpu);
> + SCX_CALL_OP(cpu_offline, cpu);
> else
> scx_ops_exit(SCX_ECODE_ACT_RESTART | SCX_ECODE_RSN_HOTPLUG,
> "cpu %d going %s, exiting scheduler", cpu,
> @@ -3593,7 +3494,7 @@ static void task_tick_scx(struct rq *rq, struct task_struct *curr, int queued)
> curr->scx.slice = 0;
> touch_core_sched(rq, curr);
> } else if (SCX_HAS_OP(tick)) {
> - SCX_CALL_OP(SCX_KF_REST, tick, curr);
> + SCX_CALL_OP(tick, curr);
> }
>
> if (!curr->scx.slice)
> @@ -3670,7 +3571,7 @@ static int scx_ops_init_task(struct task_struct *p, struct task_group *tg, bool
> .fork = fork,
> };
>
> - ret = SCX_CALL_OP_RET(SCX_KF_UNLOCKED, init_task, p, &args);
> + ret = SCX_CALL_OP_RET(init_task, p, &args);
> if (unlikely(ret)) {
> ret = ops_sanitize_err("init_task", ret);
> return ret;
> @@ -3727,11 +3628,11 @@ static void scx_ops_enable_task(struct task_struct *p)
> p->scx.weight = sched_weight_to_cgroup(weight);
>
> if (SCX_HAS_OP(enable))
> - SCX_CALL_OP_TASK(SCX_KF_REST, enable, p);
> + SCX_CALL_OP_TASK(enable, p);
> scx_set_task_state(p, SCX_TASK_ENABLED);
>
> if (SCX_HAS_OP(set_weight))
> - SCX_CALL_OP_TASK(SCX_KF_REST, set_weight, p, p->scx.weight);
> + SCX_CALL_OP_TASK(set_weight, p, p->scx.weight);
> }
>
> static void scx_ops_disable_task(struct task_struct *p)
> @@ -3740,7 +3641,7 @@ static void scx_ops_disable_task(struct task_struct *p)
> WARN_ON_ONCE(scx_get_task_state(p) != SCX_TASK_ENABLED);
>
> if (SCX_HAS_OP(disable))
> - SCX_CALL_OP(SCX_KF_REST, disable, p);
> + SCX_CALL_OP(disable, p);
> scx_set_task_state(p, SCX_TASK_READY);
> }
>
> @@ -3769,7 +3670,7 @@ static void scx_ops_exit_task(struct task_struct *p)
> }
>
> if (SCX_HAS_OP(exit_task))
> - SCX_CALL_OP(SCX_KF_REST, exit_task, p, &args);
> + SCX_CALL_OP(exit_task, p, &args);
> scx_set_task_state(p, SCX_TASK_NONE);
> }
>
> @@ -3878,7 +3779,7 @@ static void reweight_task_scx(struct rq *rq, struct task_struct *p,
>
> p->scx.weight = sched_weight_to_cgroup(scale_load_down(lw->weight));
> if (SCX_HAS_OP(set_weight))
> - SCX_CALL_OP_TASK(SCX_KF_REST, set_weight, p, p->scx.weight);
> + SCX_CALL_OP_TASK(set_weight, p, p->scx.weight);
> }
>
> static void prio_changed_scx(struct rq *rq, struct task_struct *p, int oldprio)
> @@ -3894,8 +3795,7 @@ static void switching_to_scx(struct rq *rq, struct task_struct *p)
> * different scheduler class. Keep the BPF scheduler up-to-date.
> */
> if (SCX_HAS_OP(set_cpumask))
> - SCX_CALL_OP_TASK(SCX_KF_REST, set_cpumask, p,
> - (struct cpumask *)p->cpus_ptr);
> + SCX_CALL_OP_TASK(set_cpumask, p, (struct cpumask *)p->cpus_ptr);
> }
>
> static void switched_from_scx(struct rq *rq, struct task_struct *p)
> @@ -3987,8 +3887,7 @@ int scx_tg_online(struct task_group *tg)
> struct scx_cgroup_init_args args =
> { .weight = tg->scx_weight };
>
> - ret = SCX_CALL_OP_RET(SCX_KF_UNLOCKED, cgroup_init,
> - tg->css.cgroup, &args);
> + ret = SCX_CALL_OP_RET(cgroup_init, tg->css.cgroup, &args);
> if (ret)
> ret = ops_sanitize_err("cgroup_init", ret);
> }
> @@ -4009,7 +3908,7 @@ void scx_tg_offline(struct task_group *tg)
> percpu_down_read(&scx_cgroup_rwsem);
>
> if (SCX_HAS_OP(cgroup_exit) && (tg->scx_flags & SCX_TG_INITED))
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cgroup_exit, tg->css.cgroup);
> + SCX_CALL_OP(cgroup_exit, tg->css.cgroup);
> tg->scx_flags &= ~(SCX_TG_ONLINE | SCX_TG_INITED);
>
> percpu_up_read(&scx_cgroup_rwsem);
> @@ -4042,8 +3941,7 @@ int scx_cgroup_can_attach(struct cgroup_taskset *tset)
> continue;
>
> if (SCX_HAS_OP(cgroup_prep_move)) {
> - ret = SCX_CALL_OP_RET(SCX_KF_UNLOCKED, cgroup_prep_move,
> - p, from, css->cgroup);
> + ret = SCX_CALL_OP_RET(cgroup_prep_move, p, from, css->cgroup);
> if (ret)
> goto err;
> }
> @@ -4056,8 +3954,7 @@ int scx_cgroup_can_attach(struct cgroup_taskset *tset)
> err:
> cgroup_taskset_for_each(p, css, tset) {
> if (SCX_HAS_OP(cgroup_cancel_move) && p->scx.cgrp_moving_from)
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cgroup_cancel_move, p,
> - p->scx.cgrp_moving_from, css->cgroup);
> + SCX_CALL_OP(cgroup_cancel_move, p, p->scx.cgrp_moving_from, css->cgroup);
> p->scx.cgrp_moving_from = NULL;
> }
>
> @@ -4075,8 +3972,7 @@ void scx_cgroup_move_task(struct task_struct *p)
> * cgrp_moving_from set.
> */
> if (SCX_HAS_OP(cgroup_move) && !WARN_ON_ONCE(!p->scx.cgrp_moving_from))
> - SCX_CALL_OP_TASK(SCX_KF_UNLOCKED, cgroup_move, p,
> - p->scx.cgrp_moving_from, tg_cgrp(task_group(p)));
> + SCX_CALL_OP_TASK(cgroup_move, p, p->scx.cgrp_moving_from, tg_cgrp(task_group(p)));
> p->scx.cgrp_moving_from = NULL;
> }
>
> @@ -4095,8 +3991,7 @@ void scx_cgroup_cancel_attach(struct cgroup_taskset *tset)
>
> cgroup_taskset_for_each(p, css, tset) {
> if (SCX_HAS_OP(cgroup_cancel_move) && p->scx.cgrp_moving_from)
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cgroup_cancel_move, p,
> - p->scx.cgrp_moving_from, css->cgroup);
> + SCX_CALL_OP(cgroup_cancel_move, p, p->scx.cgrp_moving_from, css->cgroup);
> p->scx.cgrp_moving_from = NULL;
> }
> out_unlock:
> @@ -4109,8 +4004,7 @@ void scx_group_set_weight(struct task_group *tg, unsigned long weight)
>
> if (scx_cgroup_enabled && tg->scx_weight != weight) {
> if (SCX_HAS_OP(cgroup_set_weight))
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cgroup_set_weight,
> - tg_cgrp(tg), weight);
> + SCX_CALL_OP(cgroup_set_weight, tg_cgrp(tg), weight);
> tg->scx_weight = weight;
> }
>
> @@ -4300,7 +4194,7 @@ static void scx_cgroup_exit(void)
> continue;
> rcu_read_unlock();
>
> - SCX_CALL_OP(SCX_KF_UNLOCKED, cgroup_exit, css->cgroup);
> + SCX_CALL_OP(cgroup_exit, css->cgroup);
>
> rcu_read_lock();
> css_put(css);
> @@ -4343,8 +4237,7 @@ static int scx_cgroup_init(void)
> continue;
> rcu_read_unlock();
>
> - ret = SCX_CALL_OP_RET(SCX_KF_UNLOCKED, cgroup_init,
> - css->cgroup, &args);
> + ret = SCX_CALL_OP_RET(cgroup_init, css->cgroup, &args);
> if (ret) {
> css_put(css);
> scx_ops_error("ops.cgroup_init() failed (%d)", ret);
> @@ -4840,7 +4733,7 @@ static void scx_ops_disable_workfn(struct kthread_work *work)
> }
>
> if (scx_ops.exit)
> - SCX_CALL_OP(SCX_KF_UNLOCKED, exit, ei);
> + SCX_CALL_OP(exit, ei);
>
> cancel_delayed_work_sync(&scx_watchdog_work);
>
> @@ -5047,7 +4940,7 @@ static void scx_dump_task(struct seq_buf *s, struct scx_dump_ctx *dctx,
>
> if (SCX_HAS_OP(dump_task)) {
> ops_dump_init(s, " ");
> - SCX_CALL_OP(SCX_KF_REST, dump_task, dctx, p);
> + SCX_CALL_OP(dump_task, dctx, p);
> ops_dump_exit();
> }
>
> @@ -5094,7 +4987,7 @@ static void scx_dump_state(struct scx_exit_info *ei, size_t dump_len)
>
> if (SCX_HAS_OP(dump)) {
> ops_dump_init(&s, "");
> - SCX_CALL_OP(SCX_KF_UNLOCKED, dump, &dctx);
> + SCX_CALL_OP(dump, &dctx);
> ops_dump_exit();
> }
>
> @@ -5151,7 +5044,7 @@ static void scx_dump_state(struct scx_exit_info *ei, size_t dump_len)
> used = seq_buf_used(&ns);
> if (SCX_HAS_OP(dump_cpu)) {
> ops_dump_init(&ns, " ");
> - SCX_CALL_OP(SCX_KF_REST, dump_cpu, &dctx, cpu, idle);
> + SCX_CALL_OP(dump_cpu, &dctx, cpu, idle);
> ops_dump_exit();
> }
>
> @@ -5405,7 +5298,7 @@ static int scx_ops_enable(struct sched_ext_ops *ops, struct bpf_link *link)
> cpus_read_lock();
>
> if (scx_ops.init) {
> - ret = SCX_CALL_OP_RET(SCX_KF_UNLOCKED, init);
> + ret = SCX_CALL_OP_RET(init);
> if (ret) {
> ret = ops_sanitize_err("init", ret);
> cpus_read_unlock();
> @@ -6146,9 +6039,6 @@ void __init init_sched_ext_class(void)
> */
> static bool scx_dsq_insert_preamble(struct task_struct *p, u64 enq_flags)
> {
> - if (!scx_kf_allowed(SCX_KF_ENQUEUE | SCX_KF_DISPATCH))
> - return false;
> -
> lockdep_assert_irqs_disabled();
>
> if (unlikely(!p)) {
> @@ -6310,9 +6200,6 @@ static bool scx_dsq_move(struct bpf_iter_scx_dsq_kern *kit,
> bool in_balance;
> unsigned long flags;
>
> - if (!scx_kf_allowed_if_unlocked() && !scx_kf_allowed(SCX_KF_DISPATCH))
> - return false;
> -
> /*
> * Can be called from either ops.dispatch() locking this_rq() or any
> * context where no rq lock is held. If latter, lock @p's task_rq which
> @@ -6395,9 +6282,6 @@ __bpf_kfunc_start_defs();
> */
> __bpf_kfunc u32 scx_bpf_dispatch_nr_slots(void)
> {
> - if (!scx_kf_allowed(SCX_KF_DISPATCH))
> - return 0;
> -
> return scx_dsp_max_batch - __this_cpu_read(scx_dsp_ctx->cursor);
> }
>
> @@ -6411,9 +6295,6 @@ __bpf_kfunc void scx_bpf_dispatch_cancel(void)
> {
> struct scx_dsp_ctx *dspc = this_cpu_ptr(scx_dsp_ctx);
>
> - if (!scx_kf_allowed(SCX_KF_DISPATCH))
> - return;
> -
> if (dspc->cursor > 0)
> dspc->cursor--;
> else
> @@ -6439,9 +6320,6 @@ __bpf_kfunc bool scx_bpf_dsq_move_to_local(u64 dsq_id)
> struct scx_dsp_ctx *dspc = this_cpu_ptr(scx_dsp_ctx);
> struct scx_dispatch_q *dsq;
>
> - if (!scx_kf_allowed(SCX_KF_DISPATCH))
> - return false;
> -
> flush_dispatch_buf(dspc->rq);
>
> dsq = find_user_dsq(dsq_id);
> @@ -6632,9 +6510,6 @@ __bpf_kfunc u32 scx_bpf_reenqueue_local(void)
> struct rq *rq;
> struct task_struct *p, *n;
>
> - if (!scx_kf_allowed(SCX_KF_CPU_RELEASE))
> - return 0;
> -
> rq = cpu_rq(smp_processor_id());
> lockdep_assert_rq_held(rq);
>
> @@ -7239,7 +7114,7 @@ __bpf_kfunc struct cgroup *scx_bpf_task_cgroup(struct task_struct *p)
> struct task_group *tg = p->sched_task_group;
> struct cgroup *cgrp = &cgrp_dfl_root.cgrp;
>
> - if (!scx_kf_allowed_on_arg_tasks(__SCX_KF_RQ_LOCKED, p))
> + if (!scx_kf_allowed_on_arg_tasks(p))
> goto out;
>
> cgrp = tg_cgrp(tg);
> @@ -7479,10 +7354,6 @@ static int __init scx_init(void)
> *
> * Some kfuncs are context-sensitive and can only be called from
> * specific SCX ops. They are grouped into BTF sets accordingly.
> - * Unfortunately, BPF currently doesn't have a way of enforcing such
> - * restrictions. Eventually, the verifier should be able to enforce
> - * them. For now, register them the same and make each kfunc explicitly
> - * check using scx_kf_allowed().
> */
> if ((ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS,
> &scx_kfunc_set_ops_context_sensitive)) ||
> diff --git a/kernel/sched/ext_idle.c b/kernel/sched/ext_idle.c
> index efb6077810d8..e241935021eb 100644
> --- a/kernel/sched/ext_idle.c
> +++ b/kernel/sched/ext_idle.c
> @@ -658,7 +658,7 @@ void __scx_update_idle(struct rq *rq, bool idle, bool do_notify)
> * managed by put_prev_task_idle()/set_next_task_idle().
> */
> if (SCX_HAS_OP(update_idle) && do_notify && !scx_rq_bypassing(rq))
> - SCX_CALL_OP(SCX_KF_REST, update_idle, cpu_of(rq), idle);
> + SCX_CALL_OP(update_idle, cpu_of(rq), idle);
>
> /*
> * Update the idle masks:
> @@ -803,9 +803,6 @@ __bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct *p, s32 prev_cpu,
> if (!check_builtin_idle_enabled())
> goto prev_cpu;
>
> - if (!scx_kf_allowed(SCX_KF_SELECT_CPU))
> - goto prev_cpu;
> -
> #ifdef CONFIG_SMP
> return scx_select_cpu_dfl(p, prev_cpu, wake_flags, is_idle);
> #endif
> --
> 2.39.5
>
next prev parent reply other threads:[~2025-02-27 16:24 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-26 19:24 [PATCH sched_ext/for-6.15 v3 0/5] bpf, sched_ext: Make kfunc filters support struct_ops context to reduce runtime overhead Juntong Deng
2025-02-26 19:28 ` [PATCH sched_ext/for-6.15 v3 1/5] bpf: Add struct_ops context information to struct bpf_prog_aux Juntong Deng
2025-02-26 19:28 ` [PATCH sched_ext/for-6.15 v3 2/5] sched_ext: Declare context-sensitive kfunc groups that can be used by different SCX operations Juntong Deng
2025-02-27 19:19 ` Tejun Heo
2025-02-28 21:57 ` Tejun Heo
2025-03-03 21:12 ` Juntong Deng
2025-03-04 18:00 ` Tejun Heo
2025-03-07 0:07 ` Juntong Deng
2025-02-26 19:28 ` [PATCH sched_ext/for-6.15 v3 3/5] sched_ext: Add scx_kfunc_ids_ops_context_sensitive for unified filtering of context-sensitive SCX kfuncs Juntong Deng
2025-02-27 20:25 ` Tejun Heo
2025-02-27 21:23 ` Juntong Deng
2025-02-27 21:32 ` Tejun Heo
2025-02-28 2:34 ` Alexei Starovoitov
2025-02-28 21:31 ` Tejun Heo
2025-02-28 23:06 ` Alexei Starovoitov
2025-02-28 18:42 ` Juntong Deng
2025-02-28 21:20 ` Tejun Heo
2025-02-28 2:38 ` Alexei Starovoitov
2025-02-28 21:14 ` Tejun Heo
2025-02-26 19:28 ` [PATCH sched_ext/for-6.15 v3 4/5] sched_ext: Removed mask-based runtime restrictions on calling kfuncs in different contexts Juntong Deng
2025-02-27 16:24 ` Andrea Righi [this message]
2025-02-27 16:49 ` Juntong Deng
2025-02-26 19:28 ` [PATCH sched_ext/for-6.15 v3 5/5] selftests/sched_ext: Update enq_select_cpu_fails to adapt to struct_ops context filter Juntong Deng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z8CRtWS117dVEnFa@gpd3 \
--to=arighi@nvidia.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=changwoo@igalia.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=juntong.deng@outlook.com \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=tj@kernel.org \
--cc=void@manifault.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.