netdev - Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared

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: <53EE0D79.3020809@cogentembedded.com>
Date:	Fri, 15 Aug 2014 17:39:05 +0400
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	christophe leroy <christophe.leroy@....fr>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	netdev <netdev@...r.kernel.org>
Subject: Re: Issue with commit 33c133cc7598e60976a phy: IRQ cannot be shared

Hello.

On 08/15/2014 01:10 PM, christophe leroy wrote:

>>> I have an hardware with two ethernet interfaces, and with the two PHYs inside
>>> the same component INTEL LXT973 which has only one interrupt.
>>> I also have another hardware with two ethernet interfaces and two independant
>>> PHYs. But the two PHYs are wired to the same interrupt.
>>> This is working perfectly up to Linux 3.12.

>>    Hm, I'm surprised it works. Are you sure you're getting interrupts from
>> both PHYs? Because if both Ethernet controllers are active simultaneously,
>> only the first registered PHY IRQ handler should get all the interrupts.

> Yes it works. Why should only the first one get the interrupts ?
> handle_irq_event_percpu() (from kernel/irq/handle.c, extract below) calls all
> handlers regardless of whether they answer IRQ_NONE or IRQ_HANDLED. The break
> applies to the switch(), not to the while(). So all handlers are called.

    Indeed, my reasoning seems obsolete now, if ever valid at all. :-/
    I couldn't yet remember other reasons that caused me to do that patch last 
December. Perhaps it was also connected to the "rude" behaviour of the 
phylib's IRQ handler, which calls disable_irq_nosync()...

[...]

>>> Reading the commit log, I can't really understand the reason for the change.

>>> Is it really worth it, and therefore how shall my case be handled ?

>>    PHY IRQs are not necessary for the phylib state machine.

> However, polling is less efficient than IRQs. It wastes CPU and link loss
> detection is slower.

    Yes, but you can't avoid it even with valid IRQ, the way phylib is 
written: the state workqueue is activated once a second even in the absence of 
interrupts.

    What can also be done is getting rid of the IRQ workqueue and using 
threaded IRQs,

> BR
> Christophe

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html