All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Jiri Slaby <jirislaby@kernel.org>,
	Wladislav Wiebe <wladislav.wiebe@nokia.com>,
	corbet@lwn.net, jirislaby@kernel.org
Cc: akpm@linux-foundation.org, paulmck@kernel.org,
	rostedt@goodmis.org, Neeraj.Upadhyay@amd.com, david@redhat.com,
	bp@alien8.de, arnd@arndb.de, fvdl@google.com,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	peterz@infradead.org
Subject: Re: [PATCH v3] genirq: add support for warning on long-running IRQ handlers
Date: Thu, 24 Jul 2025 11:47:38 +0200	[thread overview]
Message-ID: <87ldodrkcl.ffs@tglx> (raw)
In-Reply-To: <aeeb783d-d921-450c-885d-c8e8b328f81b@kernel.org>

On Thu, Jul 24 2025 at 07:18, Jiri Slaby wrote:

> On 23. 07. 25, 20:28, Wladislav Wiebe wrote:
>> Introduce a mechanism to detect and warn about prolonged IRQ handlers.
>> With a new command-line parameter (irqhandler.duration_warn_us=),
>> users can configure the duration threshold in microseconds when a warning
>> in such format should be emitted:
>> 
>> "[CPU14] long duration of IRQ[159:bad_irq_handler [long_irq]], took: 1330 us"
>> 
>> The implementation uses local_clock() to measure the execution duration of the
>> generic IRQ per-CPU event handler.
> ...> +static inline void irqhandler_duration_check(u64 ts_start, 
> unsigned int irq,
>> +					     const struct irqaction *action)
>> +{
>> +	/* Approx. conversion to microseconds */
>> +	u64 delta_us = (local_clock() - ts_start) >> 10;
>
> Is this a microoptimization -- have you measured what speedup does it 
> bring? IOW is it worth it instead of cleaner "/ NSEC_PER_USEC"?

A 64-bit division is definitely more expensive than a shift operation
and on 32-bit w/o a 64-bit divide instruction it's more than horribly
slow.

> Or instead, you could store the diff in irqhandler_duration_threshold_ns 
> (mind that "_ns") and avoid the shift and div completely.

That's the right thing to do. The setup code can do a *1000 and be done.

> And what about the wrap? Don't you need abs_diff()?

~500 years after boot :)

Thanks,

        tglx

  parent reply	other threads:[~2025-07-24  9:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-23 18:28 [PATCH v3] genirq: add support for warning on long-running IRQ handlers Wladislav Wiebe
2025-07-24  5:18 ` Jiri Slaby
2025-07-24  5:30   ` Jiri Slaby
2025-07-24  9:47   ` Thomas Gleixner [this message]
2025-07-24 16:07     ` Wladislav Wiebe

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=87ldodrkcl.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=Neeraj.Upadhyay@amd.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=corbet@lwn.net \
    --cc=david@redhat.com \
    --cc=fvdl@google.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=wladislav.wiebe@nokia.com \
    /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.