linux-kernel - Re: [PATCH 01/14] sched: Employ sched

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c4955d95-8d23-49da-9214-da481f28f797@amd.com>
Date: Thu, 11 Sep 2025 14:36:21 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Peter Zijlstra <peterz@...radead.org>, <tj@...nel.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>
Subject: Re: [PATCH 01/14] sched: Employ sched_change guards

Hello Peter,

On 9/10/2025 9:14 PM, Peter Zijlstra wrote:
> @@ -9240,8 +9213,9 @@ static void sched_change_group(struct ta
>   */
>  void sched_move_task(struct task_struct *tsk, bool for_autogroup)
>  {
> -	int queued, running, queue_flags =
> +	unsigned int queue_flags =
>  		DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK;

nit.

Since we don't do a complete dequeue for delayed task in
sched_move_task(), can we get rid of that DEQUEUE_NOCLOCK and ...

> +	bool resched = false;
>  	struct rq *rq;
>  
>  	CLASS(task_rq_lock, rq_guard)(tsk);
> @@ -9249,28 +9223,12 @@ void sched_move_task(struct task_struct
>  
>  	update_rq_clock(rq);

... this clock update and instead rely on sched_change_begin() to
handle it within the guard?

>  
> -	running = task_current_donor(rq, tsk);
> -	queued = task_on_rq_queued(tsk);
> -
> -	if (queued)
> -		dequeue_task(rq, tsk, queue_flags);
> -	if (running)
> -		put_prev_task(rq, tsk);
> -
> -	sched_change_group(tsk);
> -	if (!for_autogroup)
> -		scx_cgroup_move_task(tsk);
> -
> -	if (queued)
> -		enqueue_task(rq, tsk, queue_flags);
> -	if (running) {
> -		set_next_task(rq, tsk);
> -		/*
> -		 * After changing group, the running task may have joined a
> -		 * throttled one but it's still the running task. Trigger a
> -		 * resched to make sure that task can still run.
> -		 */
> -		resched_curr(rq);
> +	scoped_guard (sched_change, tsk, queue_flags) {
> +		sched_change_group(tsk);
> +		if (!for_autogroup)
> +			scx_cgroup_move_task(tsk);
> +		if (scope->running)
> +			resched = true;
>  	}

Also, are we missing a:

	if (resched)
		resched_curr(rq);

here after the guard? I don't see anything in sched_change_end() at this
point that would trigger a resched.

>  }
-- 
Thanks and Regards,
Prateek