Re: [PATCH v5 08/14] sched/ext: Add a DL server for sched_ext tasks
From: Tejun Heo
Date: Mon Jun 30 2025 - 11:38:11 EST
Hello, Joel.
On Mon, Jun 30, 2025 at 11:12:52AM -0400, Joel Fernandes wrote:
...
> > One thing that confuses me is that all that the patchset needs to do seems
> > to be adding @rf to dl_server_pick_f and that seems fine to me. Why is it
> > necessary to add @rf to sched_class->pick_task()?
>
> Because ext_server_pick_task is called via DL's pick_task?
>
> In deadline.c, pick_task_dl -> _pick_task_dl -> ext_server_pick_task
Ah, right, sorry about being so dense.
> This changes the signature of the pick_task_dl function, which in turn
> changes the signature of class->pick_task.
>
> How about I pass NULL to pick_task_scx() from ext_server_pick_task(), and
> also annotate all functions where rf is unused, by naming the argument as
> rf_unused (except for DL), would that make it more clear that the rq lock
> should not be arbitrarily dropped just because rf was passed? And perhaps
> sprinkling some more code comments.
I think what bothers me is that this erases the distinction between
->balance() and ->pick_task(). However, I'm not sure the distinction means
anything anymore especially given the removal of !CONFIG_SMP paths. Looking
at the balance callsites, I think we can just fold each ->balance() into the
head of the corresponding ->pick_task(). Peter, what do you think?
Thanks.
--
tejun