LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Wang Qing <wangqing@vivo.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	Michal Hocko <mhocko@kernel.org>, Wang Qing <wangqing@vivo.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Will Deacon <will@kernel.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Dirk Behme <dirk.behme@de.bosch.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING
Date: Sat, 28 Aug 2021 00:27:34 +0200	[thread overview]
Message-ID: <87k0k61q21.ffs@tglx> (raw)
In-Reply-To: <1629689583-25324-1-git-send-email-wangqing@vivo.com>

Wang,

On Mon, Aug 23 2021 at 11:33, Wang Qing wrote:

> At present, whether the softirq is executed when the interrupt exits 
> is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should 
> be split and allowed to take effect separately.

There is nothing unreasonable about it. When interrupt force threading
is in effect then it obviously requires that soft interrupt processing
goes into threaded mode as well. But the threaded execution still takes
place when the force threaded interrupt handler completes. Only softirqs
which are raised from hard interrupt context (e.g. timer interrupt) are
forced off to ksoftirqd.

What you are proposing here is completly different as you enforce
softirq execution in context of ksoftirqd only.

> At the same time, we should increase the priority of ksoftirqd when
> forbidden to execute in interrupt exits. I refer to the implementation 
> of PREEMPT_RT and think it is reasonable.

What are you referring to? PREEMPT_RT does not modify the priority of
ksoftirqd. If system designers want to do that, then they can do so from
user space. 

And doing so can be problematic depending on the workload as this
effectively breaks the softirq overload mitigation mechanism which
depends on deferring to ksoftirqd so that e.g. the consumers of received
network packets can be scheduled and the system can make progress.

Just because it does not break on your system with your particular
workload and configuration does not make it suitable for general
consumption.

> +#ifdef CONFIG_SOFTIRQ_FORCED_THREADING
> +static inline void invoke_softirq(void)
> +{
> +	wakeup_softirqd();

Depending on the configuration and timing this breaks any early boot
mechanism which depends on softirqs being handled before ksoftirqd is
available. This was clearly never tested with full RCU debugging
enabled. 

Aside of that the changelog lacks any form of technical analysis and
justification for this. Just claiming that things are [un]reasonable and
making uninformed statements about PREEMPT_RT does not qualify. Quite
the contrary it's definitely unreasonable.

Thanks,

        tglx



  parent reply	other threads:[~2021-08-27 22:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-23  3:33 Wang Qing
2021-08-23  4:22 ` Mike Galbraith
2021-08-23  6:33   ` 王擎
2021-08-23  7:43     ` Mike Galbraith
2021-08-27 22:27 ` Thomas Gleixner [this message]
2021-08-28  2:18   ` 王擎
2021-08-28 14:07     ` Thomas Gleixner

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=87k0k61q21.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=dave@stgolabs.net \
    --cc=dirk.behme@de.bosch.com \
    --cc=frederic@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=wangqing@vivo.com \
    --cc=will@kernel.org \
    --subject='Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING' \
    /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

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