linux-kernel - Re: Increment irq count for second edge irq when the first is in progress

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAH3Oq6RP8Md63m_3df9c7J03re+OUJW=K894=O6rHLQT1MjVCw@mail.gmail.com>
Date:	Mon, 11 Jun 2012 17:40:34 +0800
From:	Ning Jiang <ning.n.jiang@...il.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: Increment irq count for second edge irq when the first is in progress

2012/6/5 Ning Jiang <ning.n.jiang@...il.com>:
> I've made a another patch based on your conception of irq accounting.
> It fixes the corner case in which there comes up a second irq when the
> first is still in progress, we'll increment the irq count before we quit.
>
>
> From 93960c69e588989346589fac78b311ac62fb7552 Mon Sep 17 00:00:00 2001
> From: Ning Jiang <ning.n.jiang@...il.com>
> Date: Mon, 4 Jun 2012 09:54:08 +0800
> Subject: [PATCH] genirq: Increment irq count for second edge irq when
> the first is in progress
>
> kstat_incr_irqs_this_cpu() records the real number of interruptions
> to a certain CPU, regardless of the current irq state. That includes
> interrupts which are delivered in a state where the device handler
> cannot be called.
>
> For level irq, if its disabled or no action available, we'll skip
> irq handling. When it is enabled later, the irq will be retriggered
> and kstat_incr_irqs_this_cpu() will be incremented again. This counts
> the number of interrupts arrived at the CPU and therefor it counts
> TWO in this situation. The CPU is interrupted twice, not once. The
> accounting works correctly as expected.
>
> But for edge irq, we have a subtle problem. Suppose a second irq
> happens when the first is still in progress, it will mark itself
> pending and quit, then it will be handled in the same loop after
> the first one has been handled. Thus kstat_incr_irqs_this_cpu()
> will only be counted once for two irqs.
>
> Add a check in edge handler if there is a irq already in progress,
> we'll first kstat_incr_irqs_this_cpu() before we mask and quit, so
> that we can get correct irq count in this case.
>
> [ Thanks Thomas for his excellent comments ]
>
> Signed-off-by: Ning Jiang <ning.n.jiang@...il.com>
> ---
>  kernel/irq/chip.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index eebd6d5..254b8aa 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -489,6 +489,8 @@ handle_edge_irq(unsigned int irq, struct irq_desc *desc)
>        if (unlikely(irqd_irq_disabled(&desc->irq_data) ||
>                     irqd_irq_inprogress(&desc->irq_data) || !desc->action)) {
>                if (!irq_check_poll(desc)) {
> +                       if (irqd_irq_inprogress(&desc->irq_data))
> +                               kstat_incr_irqs_this_cpu(irq, desc);
>                        desc->istate |= IRQS_PENDING;
>                        mask_ack_irq(desc);
>                        goto out_unlock;
> @@ -550,6 +552,8 @@ void handle_edge_eoi_irq(unsigned int irq, struct
> irq_desc *desc)
>        if (unlikely(irqd_irq_disabled(&desc->irq_data) ||
>                     irqd_irq_inprogress(&desc->irq_data) || !desc->action)) {
>                if (!irq_check_poll(desc)) {
> +                       if (irqd_irq_inprogress(&desc->irq_data))
> +                               kstat_incr_irqs_this_cpu(irq, desc);
>                        desc->istate |= IRQS_PENDING;
>                        goto out_eoi;
>                }
> --
> 1.7.1

Does anyone have comments on it? Please raise your concerns if any.

Thanks,
Ning
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/