From: Francesco Dolcini <francesco.dolcini@toradex.com>
To: stable@vger.kernel.org, Sasha Levin <sashal@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Francesco Dolcini <francesco.dolcini@toradex.com>,
Andrew Lunn <andrew@lunn.ch>,
Heiner Kallweit <hkallweit1@gmail.com>,
Russell King <linux@armlinux.org.uk>,
Joakim Zhang <qiangqing.zhang@nxp.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org
Subject: Re: [PATCH net v2] net: phy: Fix race condition on link status change
Date: Fri, 6 May 2022 12:10:31 +0200 [thread overview]
Message-ID: <20220506101031.GA417678@francesco-nb.int.toradex.com> (raw)
In-Reply-To: <20220506060815.327382-1-francesco.dolcini@toradex.com>
Hello,
I have one question about the process of patch/fixes backporting
to old stable kernel.
Assuming the following patch will be deemed correct and applied it will
have to be eventually backported.
This patch will apply cleanly on v5.15+, but some work is needed to
backport to v5.10 or older.
Should I send a patch for the older kernels once the fix is merged?
Reading Documentation/process/stable-kernel-rules.rst it was not clear
to me what to do in this "mixed" situations.
Thanks a lot,
Francesco
On Fri, May 06, 2022 at 08:08:15AM +0200, Francesco Dolcini wrote:
> This fixes the following error caused by a race condition between
> phydev->adjust_link() and a MDIO transaction in the phy interrupt
> handler. The issue was reproduced with the ethernet FEC driver and a
> micrel KSZ9031 phy.
>
> [ 146.195696] fec 2188000.ethernet eth0: MDIO read timeout
> [ 146.201779] ------------[ cut here ]------------
> [ 146.206671] WARNING: CPU: 0 PID: 571 at drivers/net/phy/phy.c:942 phy_error+0x24/0x6c
> [ 146.214744] Modules linked in: bnep imx_vdoa imx_sdma evbug
> [ 146.220640] CPU: 0 PID: 571 Comm: irq/128-2188000 Not tainted 5.18.0-rc3-00080-gd569e86915b7 #9
> [ 146.229563] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [ 146.236257] unwind_backtrace from show_stack+0x10/0x14
> [ 146.241640] show_stack from dump_stack_lvl+0x58/0x70
> [ 146.246841] dump_stack_lvl from __warn+0xb4/0x24c
> [ 146.251772] __warn from warn_slowpath_fmt+0x5c/0xd4
> [ 146.256873] warn_slowpath_fmt from phy_error+0x24/0x6c
> [ 146.262249] phy_error from kszphy_handle_interrupt+0x40/0x48
> [ 146.268159] kszphy_handle_interrupt from irq_thread_fn+0x1c/0x78
> [ 146.274417] irq_thread_fn from irq_thread+0xf0/0x1dc
> [ 146.279605] irq_thread from kthread+0xe4/0x104
> [ 146.284267] kthread from ret_from_fork+0x14/0x28
> [ 146.289164] Exception stack(0xe6fa1fb0 to 0xe6fa1ff8)
> [ 146.294448] 1fa0: 00000000 00000000 00000000 00000000
> [ 146.302842] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 146.311281] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 146.318262] irq event stamp: 12325
> [ 146.321780] hardirqs last enabled at (12333): [<c01984c4>] __up_console_sem+0x50/0x60
> [ 146.330013] hardirqs last disabled at (12342): [<c01984b0>] __up_console_sem+0x3c/0x60
> [ 146.338259] softirqs last enabled at (12324): [<c01017f0>] __do_softirq+0x2c0/0x624
> [ 146.346311] softirqs last disabled at (12319): [<c01300ac>] __irq_exit_rcu+0x138/0x178
> [ 146.354447] ---[ end trace 0000000000000000 ]---
>
> With the FEC driver phydev->adjust_link() calls fec_enet_adjust_link()
> calls fec_stop()/fec_restart() and both these function reset and
> temporary disable the FEC disrupting any MII transaction that
> could be happening at the same time.
>
> fec_enet_adjust_link() and phy_read() can be running at the same time
> when we have one additional interrupt before the phy_state_machine() is
> able to terminate.
>
> Thread 1 (phylib WQ) | Thread 2 (phy interrupt)
> |
> | phy_interrupt() <-- PHY IRQ
> | handle_interrupt()
> | phy_read()
> | phy_trigger_machine()
> | --> schedule phylib WQ
> |
> |
> phy_state_machine() |
> phy_check_link_status() |
> phy_link_change() |
> phydev->adjust_link() |
> fec_enet_adjust_link() |
> --> FEC reset | phy_interrupt() <-- PHY IRQ
> | phy_read()
> |
>
> Fix this by acquiring the phydev lock in phy_interrupt().
>
> Link: https://lore.kernel.org/all/20220422152612.GA510015@francesco-nb.int.toradex.com/
> Fixes: c974bdbc3e77 ("net: phy: Use threaded IRQ, to allow IRQ from sleeping devices")
> cc: <stable@vger.kernel.org>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> drivers/net/phy/phy.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index beb2b66da132..f122026c4682 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -970,8 +970,13 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
> {
> struct phy_device *phydev = phy_dat;
> struct phy_driver *drv = phydev->drv;
> + irqreturn_t ret;
>
> - return drv->handle_interrupt(phydev);
> + mutex_lock(&phydev->lock);
> + ret = drv->handle_interrupt(phydev);
> + mutex_unlock(&phydev->lock);
> +
> + return ret;
> }
>
> /**
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-05-06 10:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-06 6:08 [PATCH net v2] net: phy: Fix race condition on link status change Francesco Dolcini
2022-05-06 6:13 ` Francesco Dolcini
2022-05-06 10:10 ` Francesco Dolcini [this message]
2022-05-06 14:51 ` Greg Kroah-Hartman
2022-05-07 13:57 ` Andrew Lunn
2022-05-10 1:00 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220506101031.GA417678@francesco-nb.int.toradex.com \
--to=francesco.dolcini@toradex.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=qiangqing.zhang@nxp.com \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).