| Age | Commit message (Collapse) | Author |
|---|
|
When switching a task's sched_class away from sched_ext, we get the
following sequence of events in __sched_setscheduler():
sched_change_begin()
switched_from_scx()
scx_disable_task(p)
ops.disable(p)
__setscheduler_params()
set_load_weight()
reweight_task_scx(p)
ops.set_weight(p)
p->sched_class = next_class;
sched_change_end()
...
Notably, ops.set_weight() is called _after_ ops.disable().
This violates the expected semantics of the callbacks, the expectation
being that ops.disable() can only be followed by ops.exit_task() or
ops.enable().
Skipping the weight adjustment for disabled tasks should be harmless
since the weight will be recalculated in scx_enable_task() if the task
ever rejoins SCX.
Fixes: 637b0682821b ("sched: Fold sched_class::switch{ing,ed}_{to,from}() into the change pattern")
Cc: stable@vger.kernel.org # v6.19+
Signed-off-by: Kuba Piecuch <jpiecuch@google.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
scx_alloc_and_add_sched() publishes @sch through ops->priv before allocating
the cgroup path. If that allocation fails, the unwind path clears ops->priv
and frees @sch immediately. scx_prog_sched() callers can dereference
ops->priv from RCU context the moment it is set, so freeing without a grace
period can use-after-free a concurrent kfunc caller.
Move the publication below the cgroup path allocation so that every failure
path after publication frees @sch through kobject_put(), whose release path
defers the freeing by a grace period.
Fixes: 105dcd005be2 ("sched_ext: Introduce scx_prog_sched()")
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
scx_sub_enable_workfn() has several failure paths that only return an errno
(e.g. -ENOMEM from an allocation) and jump to err_disable without calling
scx_error(). scx_flush_disable_work() runs the disable, and thus ops.exit(),
only when an error has been recorded, so an errno-only failure leaves the
half-initialized sub-scheduler linked.
Record an error at the err_disable sink so every errno-only failure runs the
disable path.
Fixes: ebeca1f930ea ("sched_ext: Introduce cgroup sub-sched support")
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
set_next_task_scx() updates the tick dependency before __schedule()
updates rq->curr. When switching from a non-EXT task, such as idle, to
an EXT task with a finite slice, sched_update_tick_dependency() checks
the outgoing task and can allow the tick to remain stopped.
The dependency can also be lost without a slice-type transition. After a
finite-slice task leaves the CPU idle, the enqueue path can clear the
dependency against the idle rq->curr. SCX_RQ_CAN_STOP_TICK still records
a finite slice, so another finite task skips the transition block and
can run without the ticks needed to expire its slice.
The reverse mismatch can also happen when the last finite-slice EXT task
is dequeued: sub_nr_running() updates the dependency before rq->curr
changes, so the outgoing task state can keep the dependency set after
the CPU goes idle.
Fix this by unconditionally enabling the scheduler tick whenever a
finite-slice EXT task is selected on a nohz_full CPU. Moreover, when the
last runnable EXT task leaves, ignore the outgoing EXT slice state so
the generic scheduler can correctly re-evaluate and clear the tick
dependency.
Fixes: 22a920209ab6 ("sched_ext: Implement tickless support")
Signed-off-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
dispatch_to_local_dsq() can run from scx_bpf_dsq_move_to_local() while
ops.dispatch() has recorded the current rq. Moving a task to a local DSQ
may switch to the source or destination rq before synchronously invoking
ops.dequeue() through the following path:
SCX_CALL_OP(dispatch, rq)
ops.dispatch()
scx_bpf_dsq_move_to_local()
scx_flush_dispatch_buf()
finish_dispatch()
dispatch_to_local_dsq()
scx_dispatch_enqueue()
local_dsq_post_enq()
call_task_dequeue()
SCX_CALL_OP_TASK(dequeue, locked_rq, ...)
The nested callback saves the recorded rq and restores it on return. If
the rq tracking does not follow the lock switch, update_locked_rq() can
trigger the following lockdep assertion while restoring an rq which is
no longer held:
WARNING: kernel/sched/sched.h:1641 at call_task_dequeue+0x160/0x170
Call Trace:
scx_dispatch_enqueue+0x2b0/0x460
dispatch_to_local_dsq+0x138/0x230
scx_flush_dispatch_buf+0x1af/0x220
scx_bpf_dsq_move_to_local___v2+0xe2/0x1c0
bpf__sched_ext_ops_dispatch+0x4b/0xa7
do_pick_task_scx+0x3b6/0x910
__pick_next_task+0x105/0x1f0
__schedule+0x3e7/0x1980
Introduce switch_rq_lock() to update the tracking state together with
each rq lock handoff. Use it in dispatch_to_local_dsq(),
move_remote_task_to_local_dsq() and the in-balance paths of
scx_dsq_move(), ensuring that scx_locked_rq() consistently refers to the
rq whose lock is actually held throughout the lock dance.
Fixes: 7fb39e4eb4c3 ("sched_ext: Save and restore scx_locked_rq across SCX_CALL_OP")
Cc: stable@vger.kernel.org # 7.1+
Signed-off-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
put_prev_task_scx() warns when a runnable task drops to a lower sched_class
without SCX_OPS_ENQ_LAST, on the assumption that balance_one() would have
kept it running. Core scheduling breaks that: a forced-idle SMT sibling
reschedules through the core_pick fast path in pick_next_task(), which skips
pick_task_scx() and thus balance_one(), so a runnable task can drop to idle
with ENQ_LAST unset.
Gate the warning on sched_cpu_cookie_match(): a cookie mismatch means core
scheduling forced the idle, while a match (or core scheduling off) still
catches a genuine missing-ENQ_LAST drop.
Fixes: 7c65ae81ea86 ("sched_ext: Don't call put_prev_task_scx() before picking the next task")
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
A child sub-scheduler dereferences its parent scx_sched throughout its life,
e.g., in scx_sub_disable() which reparents the child's tasks and calls
parent->ops.sub_detach() after unlinking from the parent. However, the
parent is pinned only through parent->sub_kset, which is dropped during
disable. The parent scx_sched can be RCU-freed while a child is still
disabling.
Take a direct reference on the parent in scx_alloc_and_add_sched(), dropped
in scx_sched_free_rcu_work(), so a parent always outlives its descendants.
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
scx_kick_syncs is a per-CPU __rcu pointer, so per_cpu_ptr() returns struct
scx_kick_syncs __rcu **. alloc_kick_syncs() and free_kick_syncs() stored it
in a plain struct scx_kick_syncs **ksyncs, which sparse flags as an __rcu
address-space mismatch. Annotate ksyncs to match. Its accesses already go
through rcu_*_pointer().
Fixes: 987e00035c0e ("sched_ext: Rename pnt_seq to kick_sync")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202606122315.pbnDHP0n-lkp@intel.com/
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
task_can_run_on_remote_rq() operates under the assumption that
p->migration_disabled is stable, i.e. if the kernel observed
is_migration_disabled(p) == true, then the BPF scheduler must have also
been able to see this when dispatching the task, and it's the BPF
scheduler's fault that it tried to dispatch a task with migration
disabled to a CPU other than the task's current CPU.
This assumption does not always hold. It's possible that the BPF
scheduler saw is_migration_disabled(p) == false, while the kernel
observes is_migration_disabled(p) == true in dispatch_to_local_dsq()
-> task_can_run_on_remote_rq().
The crucial thing here is that with CONFIG_PREEMPT_RCU, migration is
disabled while a task is executing a BPF program. So, if there's a
situation where the BPF scheduler checks a task while it's not executing
a BPF program, while the kernel checks it while it is executing one,
the BPF scheduler will be killed through no fault of its own.
Consider the following scenario:
1. SCX task @p is executing on CPU A and CPU A gets preempted by a
higher-priority scheduling class. On entry to __schedule(),
p->migration_disabled == 0.
2. In put_prev_task_scx() @p is enqueued on the BPF scheduler's internal
data structures, making it available for other CPUs to dispatch.
3. CPU B enters ops.dispatch(), pops @p from the BPF scheduler's data
structures, checks is_migration_disabled(p) which returns false,
and dispatches @p to CPU B's local DSQ.
4. On CPU A, @p hasn't been switched out yet. Execution reaches
trace_sched_switch() which enters a BPF program, as the BPF scheduler
hooks into the sched_switch tracepoint to detect idle->fair
transitions. On entry into the BPF program, @p disables migration.
5. CPU B enters finish_dispatch() -> dispatch_to_local_dsq() ->
task_can_run_on_remote_rq() which observes
is_migration_disabled(p) == true, triggering scx_error().
This all happens while holding CPU B's rq lock, so it's not
synchronized with @p switching out.
This patch fixes this by moving the call to task_can_run_on_remote_rq()
after @p's rq lock is acquired in dispatch_to_local_dsq(). This way, we
synchronize with @p switching out, since @p holds its rq lock all
the way until it's switched out. Thus, any BPF programs that are called
between put_prev_task_scx() and the end of the context switch are
guaranteed to have finished and cannot influence p->migration_disabled.
Also add a lockdep assertion in task_can_run_on_remote_rq() which
ensures the task rq lock is held if enforce == true.
Signed-off-by: Kuba Piecuch <jpiecuch@google.com>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
When a task leaves the BPF scheduler's control, p->scx.dsq_vtime and
p->scx.slice keep whatever values they last held. The slice value is
core-managed and is refilled on the next enqueue, but dsq_vtime is owned
by the BPF scheduler and is never cleared by the core, so a task that
leaves SCX and later returns carries a stale dsq_vtime across the
round-trip.
The stale values are also visible to other SCX schedulers that inspect
the scx fields of non-SCX tasks.
Fix this by resetting both dsq_vtime and slice in scx_disable_task(),
after ops.disable(), so the BPF scheduler can still observe the task's
final values and non-SCX tasks do not retain stale SCX state.
Signed-off-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
The deprecation notices for direct p->scx.slice/dsq_vtime writes and for
ops->cpu_acquire/release() use plain pr_warn(), so they repeat on every
scheduler (re)load and can flood the kernel log.
The slice/dsq_vtime notice is the worst offender: it is emitted from the
BPF verifier's btf_struct_access callback, which is re-evaluated as the
verifier explores program paths, so a single scheduler load can print it
many times -- hundreds of lines on some hosts, dozens within the same
second.
Switch both notices to pr_warn_ratelimited() so each deprecation is still
reported but bursts no longer spam the log, and add the missing newline
to the slice/dsq_vtime message.
Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
|
idle.c and cid.c are included into build_policy.c together with ext.c and
use helpers that ext.c defines. Because the helpers live in ext.c, the two
files can not parse as standalone units and clangd reports errors in them.
Move the helpers to the headers they belong to. The op-dispatch macros and
helpers plus scx_parent() to internal.h, and scx_cpu_arg()/scx_cpu_ret() to
cid.h. No functional change. idle.c and cid.c now parse clean standalone.
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
The sources under kernel/sched/ext/ build as a single translation unit:
build_policy.c includes the source files and headers. An LSP/clangd editor
parses each as a standalone unit, sees no types, and reports a flood of
errors.
Give each header its dependencies and include guard, and have each source
include the headers it uses.
ext.c, arena.c and the ext headers now parse clean standalone. idle.c and
cid.c still reference a few macros and helpers defined in ext.c. The next
patch moves those to shared headers.
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
|
|
The sched_ext sources had grown to ten ext* files directly under
kernel/sched/. Move them into a new kernel/sched/ext/ subdirectory and drop
the now-redundant ext_ prefix. ext.c/h keep their names.
kernel/sched/ext.{c,h} -> kernel/sched/ext/ext.{c,h}
kernel/sched/ext_internal.h -> kernel/sched/ext/internal.h
kernel/sched/ext_types.h -> kernel/sched/ext/types.h
kernel/sched/ext_idle.{c,h} -> kernel/sched/ext/idle.{c,h}
kernel/sched/ext_cid.{c,h} -> kernel/sched/ext/cid.{c,h}
kernel/sched/ext_arena.{c,h} -> kernel/sched/ext/arena.{c,h}
The include paths in build_policy.c and sched.h, the MAINTAINERS glob, and a
few documentation and comment references are updated to match. No code or
symbol changes.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
|
