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: <20250911095523.GX3289052@noisy.programming.kicks-ass.net>
Date: Thu, 11 Sep 2025 11:55:23 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: tj@...nel.org, 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

On Thu, Sep 11, 2025 at 02:36:21PM +0530, K Prateek Nayak wrote:
> 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?

Yeah, I suppose we could. But let me try and do that in a later patch,
on-top of all this.

> > -	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.

Bah, yes. That hunk must've gone missing in one of the many rebases I
did while folding back fixes :/