* [PATCH net-next 0/2] net: phy: make phy_stop() synchronous
@ 2018-09-18 19:54 Heiner Kallweit
2018-09-18 19:55 ` [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev Heiner Kallweit
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Heiner Kallweit @ 2018-09-18 19:54 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev, Geert Uytterhoeven
There have been few not that successful attempts in the past to make
phy_stop() a synchronous call instead of just changing the state.
Patch 1 of this series addresses an issue which prevented this change.
At least for me it works fine now. Would appreciate if Geert could
re-test as well that suspend doesn't throw an error.
Heiner Kallweit (2):
net: linkwatch: add check for netdevice being present to linkwatch_do_dev
net: phy: call state machine synchronously in phy_stop
drivers/net/phy/phy.c | 2 ++
net/core/link_watch.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
--
2.19.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev
2018-09-18 19:54 [PATCH net-next 0/2] net: phy: make phy_stop() synchronous Heiner Kallweit
@ 2018-09-18 19:55 ` Heiner Kallweit
2018-09-19 8:48 ` Geert Uytterhoeven
2018-09-20 0:03 ` Florian Fainelli
2018-09-18 19:56 ` [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop Heiner Kallweit
2018-09-20 4:07 ` [PATCH net-next 0/2] net: phy: make phy_stop() synchronous David Miller
2 siblings, 2 replies; 8+ messages in thread
From: Heiner Kallweit @ 2018-09-18 19:55 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev, Geert Uytterhoeven
When bringing down the netdevice (incl. detaching it) and calling
netif_carrier_off directly or indirectly the latter triggers an
asynchronous linkwatch event.
This linkwatch event eventually may fail to access chip registers in
the ndo_get_stats/ndo_get_stats64 callback because the device isn't
accessible any longer, see call trace in [0].
To prevent this scenario don't check for IFF_UP only, but also make
sure that the netdevice is present.
[0] https://lists.openwall.net/netdev/2018/03/15/62
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
net/core/link_watch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index e38e641e9..7f51efb2b 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -155,7 +155,7 @@ static void linkwatch_do_dev(struct net_device *dev)
clear_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state);
rfc2863_policy(dev);
- if (dev->flags & IFF_UP) {
+ if (dev->flags & IFF_UP && netif_device_present(dev)) {
if (netif_carrier_ok(dev))
dev_activate(dev);
else
--
2.19.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
2018-09-18 19:54 [PATCH net-next 0/2] net: phy: make phy_stop() synchronous Heiner Kallweit
2018-09-18 19:55 ` [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev Heiner Kallweit
@ 2018-09-18 19:56 ` Heiner Kallweit
2018-09-19 8:49 ` Geert Uytterhoeven
2018-09-20 0:03 ` Florian Fainelli
2018-09-20 4:07 ` [PATCH net-next 0/2] net: phy: make phy_stop() synchronous David Miller
2 siblings, 2 replies; 8+ messages in thread
From: Heiner Kallweit @ 2018-09-18 19:56 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev, Geert Uytterhoeven
phy_stop() may be called e.g. when suspending, therefore all needed
actions should be performed synchronously. Therefore add a synchronous
call to the state machine.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c78203b25..f5bb6a7a8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -861,6 +861,8 @@ void phy_stop(struct phy_device *phydev)
out_unlock:
mutex_unlock(&phydev->lock);
+ phy_state_machine(&phydev->state_queue.work);
+
/* Cannot call flush_scheduled_work() here as desired because
* of rtnl_lock(), but PHY_HALTED shall guarantee phy_change()
* will not reenable interrupts.
--
2.19.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev
2018-09-18 19:55 ` [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev Heiner Kallweit
@ 2018-09-19 8:48 ` Geert Uytterhoeven
2018-09-20 0:03 ` Florian Fainelli
1 sibling, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2018-09-19 8:48 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Florian Fainelli, Andrew Lunn, David S. Miller, netdev,
Geert Uytterhoeven
On Tue, Sep 18, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> When bringing down the netdevice (incl. detaching it) and calling
> netif_carrier_off directly or indirectly the latter triggers an
> asynchronous linkwatch event.
> This linkwatch event eventually may fail to access chip registers in
> the ndo_get_stats/ndo_get_stats64 callback because the device isn't
> accessible any longer, see call trace in [0].
>
> To prevent this scenario don't check for IFF_UP only, but also make
> sure that the netdevice is present.
>
> [0] https://lists.openwall.net/netdev/2018/03/15/62
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Survived 100 suspend/resume cycles on sh73a0/kzm9g and r8a73a4/ape6evm.
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
2018-09-18 19:56 ` [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop Heiner Kallweit
@ 2018-09-19 8:49 ` Geert Uytterhoeven
2018-09-20 0:03 ` Florian Fainelli
1 sibling, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2018-09-19 8:49 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Florian Fainelli, Andrew Lunn, David S. Miller, netdev,
Geert Uytterhoeven
On Tue, Sep 18, 2018 at 9:56 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> phy_stop() may be called e.g. when suspending, therefore all needed
> actions should be performed synchronously. Therefore add a synchronous
> call to the state machine.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Survived 100 suspend/resume cycles on sh73a0/kzm9g and r8a73a4/ape6evm.
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev
2018-09-18 19:55 ` [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev Heiner Kallweit
2018-09-19 8:48 ` Geert Uytterhoeven
@ 2018-09-20 0:03 ` Florian Fainelli
1 sibling, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2018-09-20 0:03 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev, Geert Uytterhoeven
On 09/18/2018 12:55 PM, Heiner Kallweit wrote:
> When bringing down the netdevice (incl. detaching it) and calling
> netif_carrier_off directly or indirectly the latter triggers an
> asynchronous linkwatch event.
> This linkwatch event eventually may fail to access chip registers in
> the ndo_get_stats/ndo_get_stats64 callback because the device isn't
> accessible any longer, see call trace in [0].
>
> To prevent this scenario don't check for IFF_UP only, but also make
> sure that the netdevice is present.
>
> [0] https://lists.openwall.net/netdev/2018/03/15/62
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Heiner!
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
2018-09-18 19:56 ` [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop Heiner Kallweit
2018-09-19 8:49 ` Geert Uytterhoeven
@ 2018-09-20 0:03 ` Florian Fainelli
1 sibling, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2018-09-20 0:03 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev, Geert Uytterhoeven
On 09/18/2018 12:56 PM, Heiner Kallweit wrote:
> phy_stop() may be called e.g. when suspending, therefore all needed
> actions should be performed synchronously. Therefore add a synchronous
> call to the state machine.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Yes!
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: make phy_stop() synchronous
2018-09-18 19:54 [PATCH net-next 0/2] net: phy: make phy_stop() synchronous Heiner Kallweit
2018-09-18 19:55 ` [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev Heiner Kallweit
2018-09-18 19:56 ` [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop Heiner Kallweit
@ 2018-09-20 4:07 ` David Miller
2 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-09-20 4:07 UTC (permalink / raw)
To: hkallweit1; +Cc: f.fainelli, andrew, netdev, geert+renesas
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 18 Sep 2018 21:54:23 +0200
> There have been few not that successful attempts in the past to make
> phy_stop() a synchronous call instead of just changing the state.
> Patch 1 of this series addresses an issue which prevented this change.
> At least for me it works fine now. Would appreciate if Geert could
> re-test as well that suspend doesn't throw an error.
Series applied, thank you.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-09-20 9:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-18 19:54 [PATCH net-next 0/2] net: phy: make phy_stop() synchronous Heiner Kallweit
2018-09-18 19:55 ` [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev Heiner Kallweit
2018-09-19 8:48 ` Geert Uytterhoeven
2018-09-20 0:03 ` Florian Fainelli
2018-09-18 19:56 ` [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop Heiner Kallweit
2018-09-19 8:49 ` Geert Uytterhoeven
2018-09-20 0:03 ` Florian Fainelli
2018-09-20 4:07 ` [PATCH net-next 0/2] net: phy: make phy_stop() synchronous David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.