Re: [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq()

[Christian Loehle]

On 8/12/25 09:00, Peter Zijlstra wrote:
> On Mon, Aug 11, 2025 at 10:21:47PM +0100, Christian Loehle wrote:
>> scx_bpf_cpu_rq() currently allows accessing struct rq fields without
>> holding the associated rq.
>> It is being used by scx_cosmos, scx_flash, scx_lavd, scx_layered, and
>> scx_tickless. Fortunately it is only ever used to fetch rq->curr.
>> So provide an alternative scx_bpf_task_acquire_remote_curr() that
>> doesn't expose struct rq and provide a hardened scx_bpf_cpu_rq_locked()
>> by ensuring we hold the rq lock.
>> Add a deprecation warning to scx_bpf_cpu_rq_locked() that mentions the
>> two alternatives.
>>
>> This also simplifies scx code from:
>>
>> rq = scx_bpf_cpu_rq(cpu);
>> if (!rq)
>> 	return;
>> p = rq->curr
>> if (!p)
>> 	return;
>> /* ... Do something with p */
>>
>> into:
>>
>> p = scx_bpf_task_acquire_remote_curr(cpu);
>> if (!p)
>> 	return;
>> /* ... Do something with p */
>> bpf_task_release(p);
> 
> Why do that mandatory refcount dance, rather than directly exposing the
> RCU-ness of that pointer? IIRC BPF was good with RCU.

Just because that's how
bpf_task_from_pid()
bpf_task_from_vpid()
already work. I have no strong preference either way.
Apart from the above just returning
rcu_dereference(cpu_rq(cpu)->curr);
is obviously a bit less cumbersome (and yes, RCUs are exposed to BPF,
for scx most callbacks have that implicit anyway.)

I'll change it to scx_bpf_remote_curr() that does that in the next
version, thanks!