Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <c46ddb954cfe45d9849c911271d7ec23@hisilicon.com>
 <CAK8P3a2adJsz5hRT_eMzSoHnUBC+aK9HZ18=oAYCZ-gisEkd1w@mail.gmail.com>
 <24e0652b3afa48cdbf7c83287e43c087@hisilicon.com> <CAK8P3a0fwMe9LaXMfKjH46yvt6o-euZJZ4HXtVRPhYbKvAUPKg@mail.gmail.com>
 <0b766dba0b004ced94131e158cd8e67d@hisilicon.com>
In-Reply-To: <0b766dba0b004ced94131e158cd8e67d@hisilicon.com>
From:   Arnd Bergmann <arnd@kernel.org>
Date:   Sat, 13 Feb 2021 17:32:01 +0100
Message-ID: <CAK8P3a2ZnKeeZ-zEWO+vHogs0DdLuDrZet61cSmJe_UMYhtaWQ@mail.gmail.com>
Subject: Re: [RFC] IRQ handlers run with some high-priority interrupts(not
 NMI) enabled on some platform
To:     "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>
Cc:     "tglx@linutronix.de" <tglx@linutronix.de>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "arnd@arndb.de" <arnd@arndb.de>,
        "geert@linux-m68k.org" <geert@linux-m68k.org>,
        "funaho@jurai.org" <funaho@jurai.org>,
        "philb@gnu.org" <philb@gnu.org>, "corbet@lwn.net" <corbet@lwn.net>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "linux-m68k@lists.linux-m68k.org" <linux-m68k@lists.linux-m68k.org>,
        "fthain@telegraphics.com.au" <fthain@telegraphics.com.au>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Feb 13, 2021 at 12:50 AM Song Bao Hua (Barry Song)
<song.bao.hua@hisilicon.com> wrote:

> So I was actually trying to warn this unusual case - interrupts
> get nested while both in_hardirq() and irqs_disabled() are true.
>
> diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
> index 7c9d6a2d7e90..b8ca27555c76 100644
> --- a/include/linux/hardirq.h
> +++ b/include/linux/hardirq.h
> @@ -32,6 +32,7 @@ static __always_inline void rcu_irq_enter_check_tick(void)
>   */
>  #define __irq_enter()                                  \
>         do {                                            \
> +               WARN_ONCE(in_hardirq() && irqs_disabled(), "nested
> interrupts\n"); \
>                 preempt_count_add(HARDIRQ_OFFSET);      \

That seems to be a rather heavyweight change in a critical path.

A more useful change might be to implement lockdep support for m68k
and see if that warns about any actual problems. I'm not sure
what is actually missing for that, but these are the commits that
added it for other architectures in the past:

3c4697982982 ("riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT")
000591f1ca33 ("csky: Enable LOCKDEP_SUPPORT")
78cdfb5cf15e ("openrisc: enable LOCKDEP_SUPPORT and irqflags tracing")
8f371c752154 ("xtensa: enable lockdep support")
bf2d80966890 ("microblaze: Lockdep support")

> And I also think it is better for m68k's arch_irqs_disabled() to
> return true only when both low and high priority interrupts are
> disabled rather than try to mute this warn in genirq by a weaker
> condition:
>              if (WARN_ONCE(!irqs_disabled(),"irq %u handler %pS enabled interrupts\n",
>                              irq, action->handler))
>                        local_irq_disable();
> }
>
> This warn is not activated on m68k because its arch_irqs_disabled() return
> true though its high-priority interrupts are still enabled.

Then it would just end up always warning when a nested hardirq happens,
right? That seems no different to dropping support for nested hardirqs
on m68k altogether, which of course is what you suggested already.

       Arnd