Re: [PATCH 2/2] sched_ext: Sync common and compat headers from the scx repo
Changwoo Min <[email protected]>
| Newsgroups | dev.linux.lists.sched-ext,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hello,
On 8/19/26 6:33 AM, Tejun Heo wrote:
>>> +/*
>>> + * True if the non-sleepable BPF trampoline prolog (__bpf_prog_enter) calls
>>> + * migrate_disable() for the current task. Recorded once by
>>> + * scx_lib_init_probe, an fentry program on bpf_scx_reg() that fires during
>>> + * the natural scheduler-attach call chain (auto-attached by scx_ops_attach!).
>>> + *
>>> + * Defaults to true (conservative). Over-reporting in is_migration_disabled()
>>> + * causes local-only dispatch, which is safe. Under-reporting can crash the
>>> + * scheduler, so we err high if the probe somehow fails to run.
>>> + */
>>> +bool __scx_prolog_disables_migration __weak = true;
>> [Severity: High]
>> Does this default value actually under-report if the probe fails to run?
>>
>> The documentation indicates that the fallback should err high (over-report)
>> to avoid crashing the scheduler. However, looking at the slow path in
>> is_migration_disabled() below, when __scx_prolog_disables_migration is true,
>> the macro evaluates bpf_get_current_task_btf() != p. This causes it to
>> return false for the current task, which means it incorrectly assumes
>> migration is not disabled (under-reporting).
>>
>> Should the default value of __scx_prolog_disables_migration be false
>> instead, so that it falls back to returning true?
> The comment and the default do disagree. true matches the stock pre-v6.18
> prolog behavior, so a failed probe still yields correct results on stock
> kernels. However, if the probe fails on a kernel that cherry-picked
> 8e4f0b1ebcf2 ("bpf: use rcu_read_lock_dont_migrate() for trampoline.c"),
> is_migration_disabled(current) under-reports, which is the direction the
> comment claims to avoid. Erring high would be defaulting to false.
>
> Changwoo, this code is synced from the scx repo. Can you update the scx
> side, either flipping the default or fixing the comment? I'll re-sync
> afterwards.
I submitted a PR to fix this: https://github.com/sched-ext/scx/pull/3766
Regards,
Changwoo Min