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: Thu, 30 Sep 2021 11:53:48 +0200	[thread overview]
Message-ID: <20210930095348.tud6jdcenfkfzugz@linutronix.de> (raw)
In-Reply-To: <YVV+RklIlsG6N2ic@hirez.programming.kicks-ass.net>

On 2021-09-30 11:07:18 [+0200], Peter Zijlstra wrote:
> 
> IIRC we have existing problems in -RT due to this irq_work softirq muck.

We have existing problems in -RT due irq_work being used without knowing
the consequences.

> 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.

For signals (at least on x86) we this ARCH_RT_DELAYS_SIGNAL_SEND thingy
where the signal is delayed until exit_to_user_mode_loop().

perf_pending_event() is the only non-HARD on RT (on the perf side). I
think that is due to perf_event_wakeup() where we have wake_up_all() and
read_lock_irqsave().

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

I agree. The anonymous softirqs concept brings problems of its own.
But what should I do with things like that:
- kernel/trace/ring_buffer.c rb_wake_up_waiters()
  kernel/bpf/ringbuf.c bpf_ringbuf_notify()
   wake_up_all()

- drivers/acpi/apei/ghes.c ghes_proc_in_irq()
  spinlock_t
 
- drivers/dma-buf/dma-fence-array.c irq_dma_fence_array_work()
  spinlock_t, callbacks, potential kfree().

- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c signal_irq_work()
  spinlock_t, rpm_put() -> wake_up_var(), callbacks, kref_put() like
  constructs which may free memory.

I didn't look at _all_ of them but just a few. And the only one I looked
at and didn't add to the list was
    drivers/edac/igen6_edac.c ecclog_irq_work_cb()

which simply reads PCI registers (which acquires raw_spinlock_t only
(however only on x86 are those raw_spinlock_t now that looked around))
and does schedule_work(). All harmless.

I *think* the irq_work in printk is going to leave once John is done
with it. But there are way more of these things in the kernel now
compared to when I first pushed them into softirq because they were
causing trouble.

Sebastian

  reply	other threads:[~2021-09-30  9:53 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 [this message]
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=20210930095348.tud6jdcenfkfzugz@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.