linux-kernel - Re: [PATCH 0/5] sched: Lazy preemption muck

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20241009171923.78813d22@gandalf.local.home>
Date: Wed, 9 Oct 2024 17:19:23 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Peter Zijlstra <peterz@...radead.org>, Sebastian Andrzej Siewior
 <bigeasy@...utronix.de>, Ankur Arora <ankur.a.arora@...cle.com>,
 mingo@...nel.org, linux-kernel@...r.kernel.org, juri.lelli@...hat.com,
 vincent.guittot@...aro.org, dietmar.eggemann@....com, bsegall@...gle.com,
 mgorman@...e.de, vschneid@...hat.com, efault@....de
Subject: Re: [PATCH 0/5] sched: Lazy preemption muck

On Wed, 09 Oct 2024 23:06:00 +0200
Thomas Gleixner <tglx@...utronix.de> wrote:

> > Perhaps these cond_resched() is proper? That is, the need_resched() /
> > cond_resched() is not something that is being done for PREEMPT_NONE, but
> > for preempt/voluntary kernels too. Maybe these cond_resched() should stay?
> > If we spin in the loop for one more tick, that is actually changing the
> > behavior of PREEMPT_NONE and PREEMPT_VOLUNTARY, as the need_resched()/cond_resched()
> > helps with latency. If we just wait for the next tick, these loops (and
> > there's a lot of them) will all now run for one tick longer than if
> > PREEMPT_NONE or PREEMPT_VOLUNTARY were set today.  
> 
> You are looking at it from the wrong perspective. You are trying to
> preserve the status quo. I know that's the path of least effort but it's
> the fundamentally wrong approach.
> 
>     spin_lock(L);
>     while ($cond) {
>           do_stuff();
>           if (need_resched()) {
>           	spin_unlock(L);
>                 resched();
>                 spin_lock(L);
>           }
>     }
>     spin_unlock(L);
> 
> is the bogus pattern which was introduced to deal with the NONE
> shortcomings. That's what we want to get rid of and not proliferate.

So this is actually a different issue that you are trying to address. But I
don't see how it had to deal with NONE, as even PREEMPT would suffer from
that loop, right? As the you can't preempt while holding the spinlock.

> 
> For the transition phase we obviously need to do:
> 
>     while ($cond) {
>           spin_lock(L);
>           do_stuff();
>           spin_unlock(L);
>           cond_resched();
>     }

But if $cond needs to be protected by spin_lock(), what then?

	spin_lock();
	while ($cond) {
		do_stuff();
		spin_unlock();
		spin_lock();
	}
	spin_unlock();

?

> 
> And once all the problems with LAZY are sorted then this cond_resched()
> line just goes away and the loop looks like this:
> 
>     while ($cond) {
>           spin_lock(L);
>           do_stuff();
>           spin_unlock(L);
>     }
> 
> There is absolutely no argument that the spinlock held section needs to
> spawn the loop. We fixed up several of these constructs over the years
> and none of them caused a performance regression. Quite the contrary
> quite some of them improved performance because dropping the lock lets
> other CPUs interleave.
> 
> Seriously, this crap preserving mindset has to stop. If we go new ways
> then we go them all the way.

It's not about "crap preserving" but more of taking smaller steps. Then we
can see where a regression happened if one does come up. Kind of like how
you did the x86 64bit/32bit merge. Do steps that keep things as close to
what they were at the start and slowly move toward your goals.

-- Steve