[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bf3dd03e-a1f2-4586-8f00-7003848016aa@gmx.net>
Date: Fri, 24 Nov 2023 15:01:21 +0100
From: Stefan Wahren <wahrenst@....net>
To: Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>
Cc: Lino Sanfilippo <LinoSanfilippo@....de>,
Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4 net] qca_spi: Fix SPI IRQ handling
Hi Paolo,
Am 23.11.23 um 12:37 schrieb Paolo Abeni:
> On Tue, 2023-11-21 at 17:30 +0100, Stefan Wahren wrote:
>> The functions qcaspi_netdev_open/close are responsible of request &
>> free of the SPI interrupt, which wasn't the best choice. Currently
>> it's possible to trigger a double free of the interrupt by calling
>> qcaspi_netdev_close() after qcaspi_netdev_open() has failed.
>> So let us split IRQ allocation & enabling, so we can take advantage
>> of a device managed IRQ and also fix the issue.
>>
>> Fixes: 291ab06ecf67 ("net: qualcomm: new Ethernet over SPI driver for QCA7000")
>> Signed-off-by: Stefan Wahren <wahrenst@....net>
> The change makes sense, but the changelog is confusing.
>
> qcaspi_netdev_close() and qcaspi_netdev_open() are invoked only via
> ndo_open and ndo_close(), right? So qcaspi_netdev_close() will never be
> invoked qcaspi_netdev_open(), failure - that is when IFF_UP is not set.
sorry, i missed to mention an important part. This issue is partly
connected to patch 3.
Please look at qcaspi_set_ringparam() which also call ndo_close() and
ndo_open(). If you only apply patch 3 you could trigger this issue by
running the following script, interrupt via Strg+C and start again:
#!/bin/bash
while [ true ]; do
ethtool -G eth1 tx 8
ethtool -g eth1
ethtool -G eth1 tx 10
done
[ 75.713471] qcaspi spi1.0 eth1: SPI thread exit
[ 75.721814] qcaspi spi1.0 eth1: SPI thread created
[ 76.795239] qcaspi spi1.0 eth1: SPI thread exit
[ 76.815801] qcaspi spi1.0 eth1: SPI thread created
[ 77.915872] qcaspi spi1.0 eth1: SPI thread exit
[ 77.933982] qcaspi spi1.0 eth1: SPI thread created
[ 79.036024] qcaspi spi1.0 eth1: SPI thread exit
[ 79.055595] qcaspi spi1.0 eth1: SPI thread created
[ 80.076223] qcaspi spi1.0 eth1: SPI thread exit
[ 80.097305] qcaspi spi1.0 eth1: SPI thread created
[ 81.196471] qcaspi spi1.0 eth1: SPI thread exit
[ 81.217351] qcaspi spi1.0 eth1: SPI thread created
[ 82.316592] qcaspi spi1.0 eth1: SPI thread exit
[ 82.336963] qcaspi spi1.0 eth1: SPI thread created
[ 83.436864] qcaspi spi1.0 eth1: SPI thread exit
[ 83.461252] qcaspi spi1.0 eth1: SPI thread created
[ 84.556950] qcaspi spi1.0 eth1: SPI thread exit
[ 84.575897] qcaspi spi1.0 eth1: SPI thread created
[ 85.677105] qcaspi spi1.0 eth1: SPI thread exit
[ 85.695061] qcaspi spi1.0 eth1: SPI thread created
[ 86.717215] qcaspi spi1.0 eth1: SPI thread exit
[ 86.739535] qcaspi spi1.0 eth1: SPI thread created
[ 87.837355] qcaspi spi1.0 eth1: SPI thread exit
<-- Strg + C
[ 87.841072] qcaspi spi1.0 eth1: qcaspi: unable to start kernel thread.
root@...ragon:/srv# ./test_ring_fast.sh
------------[ cut here ]------------
WARNING: CPU: 0 PID: 724 at kernel/irq/manage.c:1887 free_irq+0x23c/0x288
Trying to free already-free IRQ 73
CPU: 0 PID: 724 Comm: ethtool Not tainted
6.1.49-chargebyte-00029-g8c38d497af8a-dirty #108
Hardware name: Freescale i.MX6 Ultralite (Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x24/0x2c
dump_stack_lvl from __warn+0x74/0xbc
__warn from warn_slowpath_fmt+0xc8/0x120
warn_slowpath_fmt from free_irq+0x23c/0x288
free_irq from qcaspi_netdev_close+0x38/0x5c
qcaspi_netdev_close from qcaspi_set_ringparam+0x48/0x90
qcaspi_set_ringparam from ethnl_set_rings+0x2dc/0x320
ethnl_set_rings from genl_rcv_msg+0x2c4/0x344
genl_rcv_msg from netlink_rcv_skb+0x98/0xfc
netlink_rcv_skb from genl_rcv+0x20/0x34
genl_rcv from netlink_unicast+0x114/0x1a4
netlink_unicast from netlink_sendmsg+0x314/0x340
netlink_sendmsg from sock_sendmsg_nosec+0x14/0x24
sock_sendmsg_nosec from __sys_sendto+0xc4/0xf8
__sys_sendto from ret_fast_syscall+0x0/0x54
Exception stack(0xe115dfa8 to 0xe115dff0)
dfa0: b6ed24dc 0000000c 00000003 005c4238 0000002c
00000000
dfc0: b6ed24dc 0000000c b6f6a5a0 00000122 00472e04 005c41f0 00436b60
005c4190
dfe0: 00000122 bec50b68 b6e5f841 b6dd1ae6
---[ end trace 0000000000000000 ]---
>
> Cheers,
>
> Paolo
>
Powered by blists - more mailing lists