[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2b37d74f-7a5d-486b-98df-679bd7e2b0c2@linux.ibm.com>
Date: Tue, 7 Oct 2025 10:42:29 +0530
From: Shrikanth Hegde <sshegde@...ux.ibm.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, mingo@...hat.com, juri.lelli@...hat.com,
vincent.guittot@...aro.org, dietmar.eggemann@....com,
rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
vschneid@...hat.com, longman@...hat.com, hannes@...xchg.org,
mkoutny@...e.com, void@...ifault.com, arighi@...dia.com,
changwoo@...lia.com, cgroups@...r.kernel.org,
sched-ext@...ts.linux.dev, liuwenfang@...or.com, tglx@...utronix.de,
tj@...nel.org
Subject: Re: [PATCH 01/14] sched: Employ sched_change guards
On 10/6/25 11:44 PM, Peter Zijlstra wrote:
> On Mon, Oct 06, 2025 at 08:51:27PM +0530, Shrikanth Hegde wrote:
>>
>>
>> On 9/10/25 9:14 PM, Peter Zijlstra wrote:
>>> As proposed a long while ago -- and half done by scx -- wrap the
>>> scheduler's 'change' pattern in a guard helper.
>>>
>> [...]> put_task_struct(p);
>>> --- a/kernel/sched/sched.h
>>> +++ b/kernel/sched/sched.h
>>> @@ -3860,23 +3860,22 @@ extern void check_class_changed(struct r
>>> extern struct balance_callback *splice_balance_callbacks(struct rq *rq);
>>> extern void balance_callbacks(struct rq *rq, struct balance_callback *head);
>>> -#ifdef CONFIG_SCHED_CLASS_EXT
>>> -/*
>>> - * Used by SCX in the enable/disable paths to move tasks between sched_classes
>>> - * and establish invariants.
>>> - */
>>> -struct sched_enq_and_set_ctx {
>>> +struct sched_change_ctx {
>>> struct task_struct *p;
>>> - int queue_flags;
>>> + int flags;
>>> bool queued;
>>> bool running;
>>> };
>>> -void sched_deq_and_put_task(struct task_struct *p, int queue_flags,
>>> - struct sched_enq_and_set_ctx *ctx);
>>> -void sched_enq_and_set_task(struct sched_enq_and_set_ctx *ctx);
>>> +struct sched_change_ctx *sched_change_begin(struct task_struct *p, unsigned int flags);
>>> +void sched_change_end(struct sched_change_ctx *ctx);
>>> -#endif /* CONFIG_SCHED_CLASS_EXT */
>>> +DEFINE_CLASS(sched_change, struct sched_change_ctx *,
>>> + sched_change_end(_T),
>>> + sched_change_begin(p, flags),
>>> + struct task_struct *p, unsigned int flags)
>>> +
>>> +DEFINE_CLASS_IS_UNCONDITIONAL(sched_change)
>>> #include "ext.h"
>> could you please add a comment on matching flags on dequeue/enqueue
>> here?
>
> Would something like so be okay? This assumes at least the second patch
> is applied as well.
>
> ---
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10783,6 +10783,12 @@ struct sched_change_ctx *sched_change_be
> struct sched_change_ctx *ctx = this_cpu_ptr(&sched_change_ctx);
> struct rq *rq = task_rq(p);
>
> + /*
> + * Must exclusively use matched flags since this is both dequeue and
> + * enqueue.
> + */
yes. Something like that. Unless callsites explicitly change the flags using
the scope, enqueue will happen with matching flags.
> + WARN_ON_ONCE(flags & 0xFFFF0000);
> +
A mythical example:
scope_guard(sched_change, p, DEQUEUE_THROTTLE)
scope->flags &= ~DEQUEUE_THROTTLE;
scope->flags |= ENQUEUE_HEAD;
But, One could still do this right? for such users the warning may be wrong.
> lockdep_assert_rq_held(rq);
>
> if (!(flags & DEQUEUE_NOCLOCK)) {
Powered by blists - more mailing lists