linux-kernel - 答复: [RFC] irq: Skip printing irq when desc->action is null even if any

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Jan 2020 02:06:09 +0000
From:   "liuchao (CR)" <liuchao173@...wei.com>
To:     Neil Horman <nhorman@...driver.com>,
        Thomas Gleixner <tglx@...utronix.de>
CC:     linfeilong <linfeilong@...wei.com>,
        Hushiyuan <hushiyuan@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        PJ Waskiewicz <peter.waskiewicz.jr@...el.com>
Subject: 答复: [RFC] irq: Skip printing irq when desc->action is null even if any_count is not zero

On Thu, Jan 23, 2020 at 03:29AM +0800, Neil Horman wrote:
> On Wed, Jan 22, 2020 at 01:42:48PM +0100, Thomas Gleixner wrote:
> > Chao,
> >
> > l00520965 <liuchao173@...wei.com> writes:
> >
> > > When desc->action is empty, there is no need to print out the irq and its'
> > > count in each cpu. The desc is not alloced in request_irq or freed
> > > in free_irq.
> >
> > request/free_irq() never allocate/free irq descriptors.
> >
> > > So some PCI devices, such as rtl8139, uses request_irq and free_irq,
> >
> > All PCI devices use some variant of request_irq()/free_irq(). The
> > interrupt descriptors are allocated by the underlying PCI machinery.
> > They are only allocated/freed when the device driver is
> > loaded/removed.
> >
> > And this property exists for _ALL_ interrupts independent of PCI.
> >
> > > which only modify the action of desc. So /proc/interrupts could be
> > > like this:
> >
> > I think you want to explain:
> >
> >   If an interrupt is released via free_irq() without removing the
> >   underlying irq descriptor, the interrupt count of the irq descriptor
> >   is not reset. /proc/interrupt shows such interrupts with an empty
> >   action handler name:
> >
> > >            CPU0       CPU1
> > >  38:         46          0     GICv3  36 Level     ehci_hcd:usb1
> > >  39:         66          0     GICv3  37 Level
> >
> >   irqbalance fails to detect that this interrupt is not longer in use
> >   and parses the last word in the line 'Level' as the action handler
> >   name.
> >
> > > Irqbalance gets the list of interrupts according to
> > > /proc/interrupts. In this case, irqbalance does not remove the
> > > interrupt from the balance list, and the last string in this line,which is Level,
> is used as irq_name.
> >
> > Right, this is historic behaviour and I don't know how irqbalance
> > dealt with that in the past 20+ years. At least I haven't seen any complaints.
> >
> > I'm not opposed to suppress the output, but I really want the opinion
> > of the irqbalance maintainers on that.

Irqbalance is an example. I mean, when this happens, users who cat /proc/interrupts 
may be confused about where the interrupt came from and what it was used for. 
People who use Linux may not understand the principle of this. They are not sure 
whether this is a problem of the system or not.

> >
> Actually, irqbalance ignores the trailing irq name (or it should at least), so you
> should be able to drop that portion of /proc/irqbalance, though I cant speak for
> any other users of it.

If irq isn't removed from /proc/interrups, it will still be parsed in collect_full_irq_list 
and parse_proc_interrupts. irq_name is used in guess_arm_irq_hints.

> 
> > > Or we can clear desc->kstat_irqs in each cpu in free_irq when
> > > desc->action is null?
> >
> > No, we can't. The historic behaviour is that the total interrupt count
> > for a device is maintained independent of the number of
> > request/free_irq() pairs.
> >
> > > Signed-off-by: LiuChao <liuchao173@...wei.com>
> > > Reviewed-by: Thomas Gleixner <tglx@...utronix.de>
> >
> > I really can't remember that I have reviewed this patch already.
> > Please don't add tags which claim that some one has reviewed or acked
> > your patch unless you really got that Reviewed-by or Acked-by from
> > that person.
> >
> > Thanks,
> >
> >         tglx
> >

Thanks,

		LiuChao