netdev - RE: [RFC net-next 2/4] net: stmmac: move to threaded IRQ

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: <DB8PR04MB67950EE6E54896BD84EF8CE2E6749@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date:   Mon, 13 Dec 2021 07:09:06 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     "Yannick Vignon (OSS)" <yannick.vignon@....nxp.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jose Abreu <joabreu@...opsys.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Wei Wang <weiwan@...gle.com>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Xiaoliang Yang <xiaoliang.yang_1@....com>,
        Mingkai Hu <mingkai.hu@....com>,
        Sebastien Laveze <sebastien.laveze@....com>
CC:     Yannick Vignon <yannick.vignon@....com>
Subject: RE: [RFC net-next 2/4] net: stmmac: move to threaded IRQ


Hi Yannick,

> -----Original Message-----
> From: Yannick Vignon (OSS) <yannick.vignon@....nxp.com>
> Sent: 2021年12月11日 3:36
> To: Giuseppe Cavallaro <peppe.cavallaro@...com>; Alexandre Torgue
> <alexandre.torgue@...com>; netdev@...r.kernel.org; Ong Boon Leong
> <boon.leong.ong@...el.com>; David S. Miller <davem@...emloft.net>;
> Jakub Kicinski <kuba@...nel.org>; Jose Abreu <joabreu@...opsys.com>;
> Eric Dumazet <edumazet@...gle.com>; Wei Wang <weiwan@...gle.com>;
> Alexander Lobakin <alexandr.lobakin@...el.com>; Vladimir Oltean
> <olteanv@...il.com>; Xiaoliang Yang <xiaoliang.yang_1@....com>;
> Mingkai Hu <mingkai.hu@....com>; Joakim Zhang
> <qiangqing.zhang@....com>; Sebastien Laveze
> <sebastien.laveze@....com>
> Cc: Yannick Vignon <yannick.vignon@....com>
> Subject: [RFC net-next 2/4] net: stmmac: move to threaded IRQ
> 
> From: Yannick Vignon <yannick.vignon@....com>
> 
> WIP (seems to generate warnings/error on startup)
> 
> When an IRQ is forced threaded, execution of the handler remains protected
> by local_bh_disable()/local_bh_enable() calls to keep the semantics of the
> IRQ context and avoid deadlocks. However, this also creates a contention
> point where a higher prio interrupt handler gets blocked by a lower prio task
> already holding the lock. Even though priority inheritance kicks in in such a
> case, the lower prio task can still execute for an indefinite time.
> 
> Move the stmmac interrupts to be explicitely threaded, so that high priority
> traffic can be processed without delay even if another piece of code was
> already running with BH disabled.
> 
> Signed-off-by: Yannick Vignon <yannick.vignon@....com>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 +++++++++----------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 748195697e5a..8bf24902be3c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3460,8 +3460,8 @@ static int stmmac_request_irq_multi_msi(struct
> net_device *dev)
>  	/* For common interrupt */
>  	int_name = priv->int_name_mac;
>  	sprintf(int_name, "%s:%s", dev->name, "mac");
> -	ret = request_irq(dev->irq, stmmac_mac_interrupt,
> -			  0, int_name, dev);
> +	ret = request_threaded_irq(dev->irq, NULL, stmmac_interrupt,
> +			  IRQF_ONESHOT, int_name, dev);

Why change from stmmac_mac_interrupt() to stmmac_interrupt()? A copy-paste issue?

Best Regards,
Joakim Zhang
>  	if (unlikely(ret < 0)) {
>  		netdev_err(priv->dev,
>  			   "%s: alloc mac MSI %d (error: %d)\n", @@ -3476,9 +3476,9
> @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
>  	if (priv->wol_irq > 0 && priv->wol_irq != dev->irq) {
>  		int_name = priv->int_name_wol;
>  		sprintf(int_name, "%s:%s", dev->name, "wol");
> -		ret = request_irq(priv->wol_irq,
> -				  stmmac_mac_interrupt,
> -				  0, int_name, dev);
> +		ret = request_threaded_irq(priv->wol_irq,
> +				  NULL, stmmac_mac_interrupt,
> +				  IRQF_ONESHOT, int_name, dev);
>  		if (unlikely(ret < 0)) {
>  			netdev_err(priv->dev,
>  				   "%s: alloc wol MSI %d (error: %d)\n", @@ -3494,9
> +3494,9 @@ static int stmmac_request_irq_multi_msi(struct net_device
> *dev)
>  	if (priv->lpi_irq > 0 && priv->lpi_irq != dev->irq) {
>  		int_name = priv->int_name_lpi;
>  		sprintf(int_name, "%s:%s", dev->name, "lpi");
> -		ret = request_irq(priv->lpi_irq,
> -				  stmmac_mac_interrupt,
> -				  0, int_name, dev);
> +		ret = request_threaded_irq(priv->lpi_irq,
> +				  NULL, stmmac_mac_interrupt,
> +				  IRQF_ONESHOT, int_name, dev);
>  		if (unlikely(ret < 0)) {
>  			netdev_err(priv->dev,
>  				   "%s: alloc lpi MSI %d (error: %d)\n", @@ -3605,8
> +3605,8 @@ static int stmmac_request_irq_single(struct net_device *dev)
>  	enum request_irq_err irq_err;
>  	int ret;
> 
> -	ret = request_irq(dev->irq, stmmac_interrupt,
> -			  IRQF_SHARED, dev->name, dev);
> +	ret = request_threaded_irq(dev->irq, NULL, stmmac_interrupt,
> +			  IRQF_SHARED | IRQF_ONESHOT, dev->name, dev);
>  	if (unlikely(ret < 0)) {
>  		netdev_err(priv->dev,
>  			   "%s: ERROR: allocating the IRQ %d (error: %d)\n", @@
> -3619,8 +3619,8 @@ static int stmmac_request_irq_single(struct net_device
> *dev)
>  	 * is used for WoL
>  	 */
>  	if (priv->wol_irq > 0 && priv->wol_irq != dev->irq) {
> -		ret = request_irq(priv->wol_irq, stmmac_interrupt,
> -				  IRQF_SHARED, dev->name, dev);
> +		ret = request_threaded_irq(priv->wol_irq, NULL,
> stmmac_interrupt,
> +				  IRQF_SHARED | IRQF_ONESHOT, dev->name, dev);
>  		if (unlikely(ret < 0)) {
>  			netdev_err(priv->dev,
>  				   "%s: ERROR: allocating the WoL IRQ %d (%d)\n", @@
> -3632,8 +3632,8 @@ static int stmmac_request_irq_single(struct net_device
> *dev)
> 
>  	/* Request the IRQ lines */
>  	if (priv->lpi_irq > 0 && priv->lpi_irq != dev->irq) {
> -		ret = request_irq(priv->lpi_irq, stmmac_interrupt,
> -				  IRQF_SHARED, dev->name, dev);
> +		ret = request_threaded_irq(priv->lpi_irq, NULL, stmmac_interrupt,
> +				  IRQF_SHARED | IRQF_ONESHOT, dev->name, dev);
>  		if (unlikely(ret < 0)) {
>  			netdev_err(priv->dev,
>  				   "%s: ERROR: allocating the LPI IRQ %d (%d)\n",
> --
> 2.25.1