Re: [PATCH sched_ext/for-6.11 2/2] sched_ext: Implement scx_bpf_consume_task()

[Tejun Heo <tj@kernel.org>]

Hello, Andrii.

On Fri, Jun 28, 2024 at 03:34:01PM -0700, Andrii Nakryiko wrote:
> > This should work but it's not as neat in that it now involves three dsq_id
> > -> DSQ lookups. Also, there's extra subtlety arising from @barrier_seq being
> > different from the barrier seq that the scx_dsq iterator would be using.
> 
> maybe a stupid question, but why scx_dsq iterator cannot accept
> scx_dispatch_q pointer directly instead of dsq_id and then doing
> lookup? I.e., what if you had a kfunc to do dsq_id -> scx_dispatch_q
> lookup (returning PTR_TRUSTED instance), and then you can pass that to
> iterator, you can pass that to scx_bpf_consume_task() kfunc.

Not a stupid question at all. It's just that all the existing interface is
based on IDs. This is partly because there's not much the BPF code can do
with the DSQ data structure and partly because DSQs are usually not accessed
multiple times in sequence (ie. if the BPF code isn't going to look it up
and hold it persistently, it's going to have to look it up each time
anyway).

The multiple lookups aren't the end of the world. They're all on a resizing
hashtable, so lookups should be pretty low cost. It's just a little bit sad
to look at.

> Technically, you can also have another kfunc accepting scx_dispatch_q
> and returning current "timestamp" as some special type (TRUSTED and
> all), which will be passed into consume_task() as well.
> 
> Is this too explicit in terms of types?

That would work fine too and maybe we can make the iter init function to
accept NULL pointer to start its own scope so that users who don't want to
use consume_task() can skip the extra step.

Thanks.

-- 
tejun