linux-kernel - Re: [PATCH V7 01/11] sched: Scheduler time slice extension

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: <20250807154929.4Wpr6V4N@linutronix.de>
Date: Thu, 7 Aug 2025 17:49:29 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Prakash Sangappa <prakash.sangappa@...cle.com>,
	linux-kernel@...r.kernel.org, peterz@...radead.org,
	rostedt@...dmis.org, mathieu.desnoyers@...icios.com,
	kprateek.nayak@....com, vineethr@...ux.ibm.com
Subject: Re: [PATCH V7 01/11] sched: Scheduler time slice extension

On 2025-08-06 22:34:00 [+0200], Thomas Gleixner wrote:
> On Thu, Jul 24 2025 at 16:16, Prakash Sangappa wrote:
> 
> The obvious way to solve both issues is to clear NEED_RESCHED when
> the delay is granted and then do in syscall_enter_from_user_mode_work()
> 
>         rseq_delay_sys_enter()
>         {
>              if (unlikely(current->rseq_delay_resched == GRANTED)) {
> 		    set_tsk_need_resched(current);
>                     schedule();
>              }       
>         }     	
> 
> No?
> 
> It's debatable whether the schedule() there is necessary. Removing it
> would allow the task to either complete the syscall and reschedule on
> exit to user space or go to sleep in the syscall. But that's a trivial
> detail.

Either schedule() or setting NEED_RESCHED is enough.

> The important point is that the NEED_RESCHED semantics stay sane and the
> problem is solved right on the next syscall entry.
> 
…
> > +static inline bool rseq_delay_resched(unsigned long ti_work)
> > +{
> > +	if (!IS_ENABLED(CONFIG_RSEQ_RESCHED_DELAY))
> > +		return false;
> > +
> > +	if (unlikely(current->rseq_delay_resched != RSEQ_RESCHED_DELAY_PROBE))

The functions and the task_struct member field share the same.

> > +		return false;
> 
> Why unlikely? The majority of applications do not use this.
> 
> > +
> > +	if (!(ti_work & (_TIF_NEED_RESCHED|_TIF_NEED_RESCHED_LAZY)))
> > +		return false;
> 
> The caller already established that one of these flags is set, no?

correct, and if they are set, this never gets to false.

> > +	if (__rseq_delay_resched()) {
> > +		clear_tsk_need_resched(current);
> 
> Why has this to be inline and is not done in __rseq_delay_resched()?

A SCHED_OTHER wake up sets _TIF_NEED_RESCHED_LAZY so
clear_tsk_need_resched() will revoke this granting an extension.

The RT/DL wake up will set _TIF_NEED_RESCHED and
clear_tsk_need_resched() will also clear it. However this one
additionally sets set_preempt_need_resched() so the next preempt
disable/ enable combo will lead to a scheduling event. A remote wakeup
will trigger an IPI (scheduler_ipi()) which also does
set_preempt_need_resched().

If I understand this correct then a RT/DL wake up while the task is in
kernel-mode should lead to a scheduling event assuming we pass a
spinlock_t (ignoring the irq argument).
Should the task be in user-mode then we return to user mode with the TIF
flag cleared and the NEED-RESCHED flag folded into the preemption
counter.

I am once again asking to limit this to _TIF_NEED_RESCHED_LAZY.

> > +		return true;
> > +	}
> > +	return false;
> 

…

> Thanks,
> 
>         tglx

Sebastian