[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zc-BBQGauwIEJJXy@smile.fi.intel.com>
Date: Fri, 16 Feb 2024 17:36:37 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Leonardo Bras <leobras@...hat.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Tony Lindgren <tony@...mide.com>,
John Ogness <john.ogness@...utronix.de>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Shanker Donthineni <sdonthineni@...dia.com>,
linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [RFC PATCH v2 2/4] irq/spurious: Account for multiple handles in
note_interrupt
On Fri, Feb 16, 2024 at 04:59:44AM -0300, Leonardo Bras wrote:
> Currently note_interrupt() will check threads_handled for changes and use
> it to mark an IRQ as handled, in order to avoid having a threaded
> interrupt to falsely trigger unhandled IRQ detection.
>
> This detection can still be falsely triggered if we have many IRQ handled
> accounted between each check of threads_handled, as those will be counted
> as a single one in the unhandled IRQ detection.
>
> In order to fix this, subtract from irqs_unhandled the number of IRQs
> handled since the last check (threads_handled_last - threads_handled).
..
> +static inline int get_handled_diff(struct irq_desc *desc)
> +{
> + unsigned int handled;
> + int diff;
> +
> + handled = (unsigned int)atomic_read(&desc->threads_handled);
> + handled |= SPURIOUS_DEFERRED;
> +
> + diff = handled - desc->threads_handled_last;
> + diff >>= SPURIOUS_DEFERRED_SHIFT;
> +
> + /*
> + * Note: We keep the SPURIOUS_DEFERRED bit set. We are handling the
> + * previous invocation right now. Keep it for the current one, so the
> + * next hardware interrupt will account for it.
> + */
> + if (diff != 0)
if (diff)
> + desc->threads_handled_last = handled;
> +
> + return diff;
> +}
..
> + diff = get_handled_diff(desc);
> + if (diff > 0) {
diff may not be negative as you always right shift by 1 (or more) bit. Hence
if (diff)
will suffice (also be aligned with the similar check inside the helper) and
making the helper to return unsigned value will be clearer. Am I correct?
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists