linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
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: Thu, 30 Sep 2021 11:07:18 +0200	[thread overview]
Message-ID: <YVV+RklIlsG6N2ic@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20210927211919.310855-5-bigeasy@linutronix.de>

On Mon, Sep 27, 2021 at 11:19:18PM +0200, Sebastian Andrzej Siewior wrote:
> The irq_work callback is invoked in hard IRQ context. By default all
> callbacks are scheduled for invocation right away (given supported by
> the architecture) except for the ones marked IRQ_WORK_LAZY which are
> delayed until the next timer-tick.
> 
> While looking over the callbacks, some of them may acquire locks
> (spinlock_t, rwlock_t) which are transformed into sleeping locks on
> PREEMPT_RT and must not be acquired in hard IRQ context.
> Changing the locks into locks which could be acquired in this context
> will lead to other problems such as increased latencies if everything
> in the chain has IRQ-off locks. This will not solve all the issues as
> one callback has been noticed which invoked kref_put() and its callback
> invokes kfree() and this can not be invoked in hardirq context.
> 
> Some callbacks are required to be invoked in hardirq context even on
> PREEMPT_RT to work properly. This includes for instance the NO_HZ
> callback which needs to be able to observe the idle context.
> 
> The callbacks which require to be run in hardirq have already been
> marked. Use this information to split the callbacks onto the two lists
> on PREEMPT_RT:
> - lazy_list
>   Work items which are not marked with IRQ_WORK_HARD_IRQ will be added
>   to this list. Callbacks on this list will be invoked from timer
>   softirq handler. The handler here may acquire sleeping locks such as
>   spinlock_t and invoke kfree().
> 
> - raised_list
>   Work items which are marked with IRQ_WORK_HARD_IRQ will be added to
>   this list. They will be invoked in hardirq context and must not
>   acquire any sleeping locks.
> 
> [bigeasy: melt tglx's irq_work_tick_soft() which splits irq_work_tick() into a
>           hard and soft variant. Collected fixes over time from Steven
> 	  Rostedt and Mike Galbraith. ]
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

IIRC we have existing problems in -RT due to this irq_work softirq muck.

I think the problem was something Jolsa found a while ago, where perf
defers to an irq_work (from NMI context) and that irq_work wants to
deliver signals, which it can't on -RT, so the whole thing gets punted
to softirq. With the end-result that if you self-profile RT tasks,
things come apart or something.

There might have been others as well, I don't know. But generally I
think we want *less* softirq, not more.



  reply	other threads:[~2021-09-30  9:08 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 [this message]
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
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=YVV+RklIlsG6N2ic@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bigeasy@linutronix.de \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).