public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Ankur Arora <ankur.a.arora@oracle.com>,
	mingo@kernel.org, linux-kernel@vger.kernel.org,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	dietmar.eggemann@arm.com, bsegall@google.com, mgorman@suse.de,
	vschneid@redhat.com, efault@gmx.de
Subject: Re: [PATCH 0/5] sched: Lazy preemption muck
Date: Wed, 09 Oct 2024 23:06:00 +0200	[thread overview]
Message-ID: <87ed4pq953.ffs@tglx> (raw)
In-Reply-To: <20241009164355.1ca1d3d3@gandalf.local.home>

On Wed, Oct 09 2024 at 16:43, Steven Rostedt wrote:
> On Wed, 09 Oct 2024 22:13:51 +0200
> Thomas Gleixner <tglx@linutronix.de> wrote:
>> The reason why this works is that preempt_enable() actually has a
>> meaning while it does not with NONE.
>
> Looking around, I see the pattern of checking need_resched() from within a
> loop where a spinlock is held. Then after the break of the loop and release
> of the spinlock, cond_resched() is checked, and the loop is entered again.
>
> Thus, I guess this is the reason you are saying that it should just check
> NEED_RESCHED and not the LAZY variant. Because if we remove that
> cond_resched() then it would just re-enter the loop again with the LAZY
> being set.
>
> Hmm, but then again...
>
> 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.

For the transition phase we obviously need to do:

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

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.

There is a cost involved for cleaning the existing crap up, but that
cost is well worth it, because anything else is just adding to the ever
increasing accumulation of technical debt. We have enough of that
already.

Thanks,

        tglx

  reply	other threads:[~2024-10-09 21:06 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-07  7:46 [PATCH 0/5] sched: Lazy preemption muck Peter Zijlstra
2024-10-07  7:46 ` [PATCH 1/5] sched: Add TIF_NEED_RESCHED_LAZY infrastructure Peter Zijlstra
2024-10-09 12:18   ` Sebastian Andrzej Siewior
2024-10-09 13:01     ` Peter Zijlstra
2024-11-06 10:48   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-10-07  7:46 ` [PATCH 2/5] sched: Add Lazy preemption model Peter Zijlstra
2024-10-08  5:43   ` Ankur Arora
2024-10-08 14:48     ` Peter Zijlstra
2024-10-09  8:50   ` Sebastian Andrzej Siewior
2024-10-09  9:14     ` Peter Zijlstra
2024-10-09  9:19       ` Sebastian Andrzej Siewior
2024-10-15 14:37   ` Shrikanth Hegde
2024-10-25 10:42     ` Sebastian Andrzej Siewior
2024-10-22 16:44   ` Shrikanth Hegde
2024-10-25 13:19     ` Sebastian Andrzej Siewior
2024-10-29 18:57       ` Shrikanth Hegde
2024-11-06 10:48   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-10-07  7:46 ` [PATCH 3/5] sched: Enable PREEMPT_DYNAMIC for PREEMPT_RT Peter Zijlstra
2024-10-08 13:24   ` Sebastian Andrzej Siewior
2024-10-08 14:40     ` Peter Zijlstra
2024-10-10  6:52   ` Christoph Hellwig
2024-10-10  7:50     ` Peter Zijlstra
2024-11-06 10:48   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-10-07  7:46 ` [PATCH 4/5] sched, x86: Enable Lazy preemption Peter Zijlstra
2024-11-06 10:48   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2024-10-07  7:46 ` [PATCH 5/5] sched: Add laziest preempt model Peter Zijlstra
2024-10-08  5:59   ` Ankur Arora
2024-10-08 14:23   ` Thomas Gleixner
2024-10-08 14:40     ` Peter Zijlstra
2024-10-08 15:07   ` Sebastian Andrzej Siewior
2024-10-07  8:33 ` [PATCH 0/5] sched: Lazy preemption muck Sebastian Andrzej Siewior
2024-10-08  4:58 ` Mike Galbraith
2024-10-08 15:32 ` Sebastian Andrzej Siewior
2024-10-09  4:40   ` Ankur Arora
2024-10-09  6:20     ` Sebastian Andrzej Siewior
2024-10-09  7:23       ` Ankur Arora
2024-10-09  8:02       ` Peter Zijlstra
2024-10-09  8:45         ` Sebastian Andrzej Siewior
2024-10-09 14:01         ` Steven Rostedt
2024-10-09 20:13           ` Thomas Gleixner
2024-10-09 20:43             ` Steven Rostedt
2024-10-09 21:06               ` Thomas Gleixner [this message]
2024-10-09 21:19                 ` Steven Rostedt
2024-10-09 23:16                   ` Thomas Gleixner
2024-10-09 23:29                     ` Steven Rostedt
2024-10-10  1:20                       ` Thomas Gleixner
2024-10-10 10:23                 ` David Laight
2024-10-13 19:02                   ` Thomas Gleixner
2024-10-14  8:21                     ` David Laight
2024-10-10  3:12               ` Tianchen Ding
2024-10-10  7:47                 ` Thomas Gleixner
2024-10-09  7:30   ` Ankur Arora
2024-10-09  7:46   ` Peter Zijlstra
2024-10-09 11:07 ` Sebastian Andrzej Siewior
2024-10-17 12:36 ` Mike Galbraith
2024-11-07 17:21   ` Thomas Meyer
2024-11-08  0:59     ` Mike Galbraith

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ed4pq953.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=ankur.a.arora@oracle.com \
    --cc=bigeasy@linutronix.de \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=efault@gmx.de \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox