netdev - Re: [PATCH net v2 1/2] net: txgbe: remove separate irq request for MSI and INTx

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]

Message-ID: <20240627164138.725aa957@kernel.org>
Date: Thu, 27 Jun 2024 16:41:38 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
 horms@...nel.org, andrew@...n.ch, netdev@...r.kernel.org,
 przemyslaw.kitszel@...el.com, mengyuanlou@...-swift.com,
 duanqiangwen@...-swift.com
Subject: Re: [PATCH net v2 1/2] net: txgbe: remove separate irq request for
 MSI and INTx

On Wed, 26 Jun 2024 14:07:02 +0800 Jiawen Wu wrote:
> When using MSI or INTx interrupts, request_irq() for pdev->irq will
> conflict with request_threaded_irq() for txgbe->misc.irq, to cause
> system crash. So remove txgbe_request_irq() for MSI/INTx case, and
> rename txgbe_request_msix_irqs() since it only request for queue irqs.

Do you have any users who need INTx support? Maybe you could drop
the support and simplify the code?

> Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller")
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_lib.c   |  3 +-
>  .../net/ethernet/wangxun/txgbe/txgbe_irq.c    | 78 ++-----------------
>  .../net/ethernet/wangxun/txgbe/txgbe_irq.h    |  2 +-
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  2 +-
>  4 files changed, 10 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> index 68bde91b67a0..99f55a3573c8 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> @@ -1996,7 +1996,8 @@ void wx_free_irq(struct wx *wx)
>  	int vector;
>  
>  	if (!(pdev->msix_enabled)) {
> -		free_irq(pdev->irq, wx);
> +		if (wx->mac.type == wx_mac_em)
> +			free_irq(pdev->irq, wx);

It seems strange to match on type to decide whether to free an IRQ.
Isn't there or shouldn't there be some IRQ related flag informing
the library how to manage the IRQs?