linux-kernel - Re: [RESEND v1 net-next 3/5] net: stmmac: introduce MSI Interrupt routines for mac, safety, RX & TX

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:   Tue, 16 Mar 2021 14:29:41 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Voon Weifeng <weifeng.voon@...el.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jose Abreu <joabreu@...opsys.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Andrew Lunn <andrew@...n.ch>,
        Alexandre Torgue <alexandre.torgue@...com>,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Wong Vee Khee <vee.khee.wong@...el.com>
Subject: Re: [RESEND v1 net-next 3/5] net: stmmac: introduce MSI Interrupt
 routines for mac, safety, RX & TX

On Tue, 16 Mar 2021 20:18:21 +0800 Voon Weifeng wrote:
> From: Ong Boon Leong <boon.leong.ong@...el.com>
> 
> Now we introduce MSI interrupt service routines and hook these routines
> up if stmmac_open() sees valid irq line being requested:-
> 
> stmmac_mac_interrupt()    :- MAC (dev->irq), WOL (wol_irq), LPI (lpi_irq)
> stmmac_safety_interrupt() :- Safety Feat Correctible Error (sfty_ce_irq)
>                              & Uncorrectible Error (sfty_ue_irq)
> stmmac_msi_intr_rx()      :- For all RX MSI irq (rx_irq)
> stmmac_msi_intr_tx()      :- For all TX MSI irq (tx_irq)

Do you split RX and TX irqs out on purpose? Most commonly one queue
pair maps to one CPU, so using single IRQ for Rx and Tx results in
fewer IRQs being triggered and better system performance.

> Each of IRQs will have its unique name so that we can differentiate
> them easily under /proc/interrupts.
> 
> Signed-off-by: Ong Boon Leong <boon.leong.ong@...el.com>
> Signed-off-by: Voon Weifeng <weifeng.voon@...el.com>

> +static int stmmac_request_irq(struct net_device *dev)

This function is a one huge if statement, please factor out both sides
into separate subfunctions.

> +	netdev_info(priv->dev, "PASS: requesting IRQs\n");

Does the user really need to know interrupts were requested on every
probe?

> +	return ret;

return 0; ?

> +irq_error:
> +	stmmac_free_irq(dev, irq_err, irq_idx);
> +	return ret;
> +}