All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Jiri Olsa <jolsa@kernel.org>
Subject: Re: [PATCH 4/5] irq_work: Handle some irq_work in SOFTIRQ on PREEMPT_RT
Date: Fri, 1 Oct 2021 14:08:55 +0200	[thread overview]
Message-ID: <20211001120855.hjjaqt5bpowit2r7@linutronix.de> (raw)
In-Reply-To: <YVbjxjzVM5Dx4Mv4@hirez.programming.kicks-ass.net>

On 2021-10-01 12:32:38 [+0200], Peter Zijlstra wrote:
> On Thu, Sep 30, 2021 at 06:38:58PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2021-09-30 16:39:51 [+0200], Peter Zijlstra wrote:
> 
> > > Runing them all at the same prio still sucks (much like the single
> > > net-RX thing), but at least a kthread is somewhat controllable.
> > 
> > I could replace the softirq processing with a per-CPU thread. This
> > should work. But I would have to (still) delay the wake-up of the thread
> > to the timer tick - or - we try the wake from the irqwork-self-IPI.
> 
> That, just wake the thread from the hardirq.

"just". Let me do that and see how bad it gets ;)

> > I
> > just don't know how many will arrive back-to-back. The RCU callback
> > (rcu_preempt_deferred_qs_handler()) pops up a lot. By my naive guesswork
> > I would say that the irqwork is not needed since preempt-enable
> > somewhere should do needed scheduling. But then commit
> >   0864f057b050b ("rcu: Use irq_work to get scheduler's attention in clean context")
> > 
> > claims it is not enough.
> 
> Oh gawd, that was something really nasty. I'm not sure that Changelog
> captures all (at least I'm not sure I fully understand the problem again
> reading it).
> 
> But basically that thing wants to reschedule, but suffers the same
> problem as:
> 
> 	preempt_disable();
> 
> 	<TIF_NEED_RESCHED gets set>
> 
> 	local_irq_disable();
> 	preempt_enable();
> 	  // cannea schedule because IRQs are disabled
> 	local_irq_enable();
> 	// lost a reschedule
> 
> 
> Yes, that will _eventually_ reschedule, but violates the PREEMPT rules
> because there is an unspecified amount of time until it does actually do
> reschedule.

Yeah but buh. We could let local_irq_enable/restore() check that
need-resched bit if the above is considered pretty and supported _or_
start to yell if it is not. A middle way would be to trigger that
self-IPI in such a case. I mean everyone suffers from that lost
reschedule and, if I'm not mistaken, you don't receive a remote wakeup
because the remote CPU notices need-resched bit and assumes that it is
about to be handled. So RCU isn't special here.

> So what RCU does there is basically trigger a self-IPI, which guarantees
> that we reschedule after IRQs are finally enabled, which then triggers a
> resched.
> 
> I see no problem marking that particular irq_work as HARD tho, it really
> doesn't do anything (other than tell RCU the GP is no longer blocked)
> and triggering the return-from-interrupt path.

Hmm. Your Highness. I'm going back to my peasant village to build the
thread you asked for. I will look into this. I see two of those irq-work
things that is the scheduler thingy and this.

Thanks.

> There's also a fun comment in perf_lock_task_context() that possibly
> predates the above RCU fix.

Sebastian

  reply	other threads:[~2021-10-01 12:09 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-27 21:19 [PATCH 0/5] irq_work: PREEMPT_RT bits Sebastian Andrzej Siewior
2021-09-27 21:19 ` [PATCH 1/5] sched/rt: Annotate the RT balancing logic irqwork as IRQ_WORK_HARD_IRQ Sebastian Andrzej Siewior
2021-09-27 21:19 ` [PATCH 2/5] irq_work: Ensure that irq_work runs in in-IRQ context Sebastian Andrzej Siewior
2021-10-05 15:48   ` Sebastian Andrzej Siewior
2021-10-05 20:11     ` Peter Zijlstra
2021-09-27 21:19 ` [PATCH 3/5] irq_work: Allow irq_work_sync() to sleep if irq_work() no IRQ support Sebastian Andrzej Siewior
2021-09-27 21:19 ` [PATCH 4/5] irq_work: Handle some irq_work in SOFTIRQ on PREEMPT_RT Sebastian Andrzej Siewior
2021-09-30  9:07   ` Peter Zijlstra
2021-09-30  9:53     ` Sebastian Andrzej Siewior
2021-09-30 14:39       ` Peter Zijlstra
2021-09-30 16:38         ` Sebastian Andrzej Siewior
2021-10-01 10:32           ` Peter Zijlstra
2021-10-01 12:08             ` Sebastian Andrzej Siewior [this message]
2021-10-01 13:40               ` Peter Zijlstra
2021-09-27 21:19 ` [PATCH 5/5] irq_work: Also rcuwait for !IRQ_WORK_HARD_IRQ " Sebastian Andrzej Siewior

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=20211001120855.hjjaqt5bpowit2r7@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.