All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Bitao Hu <yaoma@linux.alibaba.com>,
	dianders@chromium.org, liusong@linux.alibaba.com,
	akpm@linux-foundation.org, pmladek@suse.com,
	kernelfans@gmail.com, deller@gmx.de, npiggin@gmail.com,
	tsbogend@alpha.franken.de, James.Bottomley@HansenPartnership.com,
	jan.kiszka@siemens.com
Cc: linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
	linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCHv10 3/4] genirq: Avoid summation loops for /proc/interrupts
Date: Tue, 27 Feb 2024 16:39:41 +0100	[thread overview]
Message-ID: <87a5nlapc2.ffs@tglx> (raw)
In-Reply-To: <e78357ae-7b00-446c-b010-3bd770892c9e@linux.alibaba.com>

On Tue, Feb 27 2024 at 19:20, Bitao Hu wrote:
> On 2024/2/27 17:26, Thomas Gleixner wrote:
>> 
>> and then let kstat_irqs() and show_interrupts() use it. See?
>
> I have a concern. kstat_irqs() uses for_each_possible_cpu() for
> summation. However, show_interrupts() uses for_each_online_cpu(),
> which means it only outputs interrupt statistics for online cpus.
> If we use for_each_possible_cpu() in show_interrupts() to calculate
> 'any_count', there could be a problem with the following scenario:
> If an interrupt has a count of zero on online cpus but a non-zero
> count on possible cpus, then 'any_count' would not be zero, and the
> statistics for that interrupt would be output, which is not the
> desired behavior for show_interrupts(). Therefore, I think it's not
> good to have kstat_irqs() and show_interrupts() both use the same
> logic. What do you think?

Good point. But you simply can have

unsigned int kstat_irq_desc(struct irq_desc *desc, const struct cpumask *mask)

and hand in the appropriate cpumask, which still shares the code, no?

Thanks,

        tglx

WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: Bitao Hu <yaoma@linux.alibaba.com>,
	dianders@chromium.org, liusong@linux.alibaba.com,
	akpm@linux-foundation.org, pmladek@suse.com,
	kernelfans@gmail.com, deller@gmx.de, npiggin@gmail.com,
	tsbogend@alpha.franken.de, James.Bottomley@HansenPartnership.com,
	jan.kiszka@siemens.com
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-parisc@vger.kernel.org, linux-mips@vger.kernel.org
Subject: Re: [PATCHv10 3/4] genirq: Avoid summation loops for /proc/interrupts
Date: Tue, 27 Feb 2024 16:39:41 +0100	[thread overview]
Message-ID: <87a5nlapc2.ffs@tglx> (raw)
In-Reply-To: <e78357ae-7b00-446c-b010-3bd770892c9e@linux.alibaba.com>

On Tue, Feb 27 2024 at 19:20, Bitao Hu wrote:
> On 2024/2/27 17:26, Thomas Gleixner wrote:
>> 
>> and then let kstat_irqs() and show_interrupts() use it. See?
>
> I have a concern. kstat_irqs() uses for_each_possible_cpu() for
> summation. However, show_interrupts() uses for_each_online_cpu(),
> which means it only outputs interrupt statistics for online cpus.
> If we use for_each_possible_cpu() in show_interrupts() to calculate
> 'any_count', there could be a problem with the following scenario:
> If an interrupt has a count of zero on online cpus but a non-zero
> count on possible cpus, then 'any_count' would not be zero, and the
> statistics for that interrupt would be output, which is not the
> desired behavior for show_interrupts(). Therefore, I think it's not
> good to have kstat_irqs() and show_interrupts() both use the same
> logic. What do you think?

Good point. But you simply can have

unsigned int kstat_irq_desc(struct irq_desc *desc, const struct cpumask *mask)

and hand in the appropriate cpumask, which still shares the code, no?

Thanks,

        tglx

  reply	other threads:[~2024-02-27 15:39 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-26  2:09 [PATCHv10 0/4] *** Detect interrupt storm in softlockup *** Bitao Hu
2024-02-26  2:09 ` Bitao Hu
2024-02-26  2:09 ` [PATCHv10 1/4] watchdog/softlockup: low-overhead detection of interrupt storm Bitao Hu
2024-02-26  2:09   ` Bitao Hu
2024-02-26  2:09 ` [PATCHv10 2/4] genirq: Provide a snapshot mechanism for interrupt statistics Bitao Hu
2024-02-26  2:09   ` Bitao Hu
2024-02-27  4:10   ` Liu Song
2024-02-27  4:10     ` Liu Song
2024-02-26  2:09 ` [PATCHv10 3/4] genirq: Avoid summation loops for /proc/interrupts Bitao Hu
2024-02-26  2:09   ` Bitao Hu
2024-02-27  7:48   ` Liu Song
2024-02-27  7:48     ` Liu Song
2024-02-27  9:26   ` Thomas Gleixner
2024-02-27  9:26     ` Thomas Gleixner
2024-02-27 11:20     ` Bitao Hu
2024-02-27 11:20       ` Bitao Hu
2024-02-27 15:39       ` Thomas Gleixner [this message]
2024-02-27 15:39         ` Thomas Gleixner
2024-02-28  6:07         ` Bitao Hu
2024-02-28  6:07           ` Bitao Hu
2024-02-26  2:09 ` [PATCHv10 4/4] watchdog/softlockup: report the most frequent interrupts Bitao Hu
2024-02-26  2:09   ` Bitao Hu
2024-02-27  9:02   ` Liu Song
2024-02-27  9:02     ` Liu Song

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=87a5nlapc2.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=deller@gmx.de \
    --cc=dianders@chromium.org \
    --cc=jan.kiszka@siemens.com \
    --cc=kernelfans@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-parisc@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=liusong@linux.alibaba.com \
    --cc=npiggin@gmail.com \
    --cc=pmladek@suse.com \
    --cc=tsbogend@alpha.franken.de \
    --cc=yaoma@linux.alibaba.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.