Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 05:10 PM, David Daney wrote: > On 09/06/2017 04:14 PM, Florian Fainelli wrote: >> On 09/06/2017 03:51 PM, David Daney wrote: > [...] >>> >>> Consider instead the case of a Marvell phy with no interrupts connected >>> on a v4.9.43 kernel, single CPU: >>> >>> >>>0)

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 04:14 PM, Florian Fainelli wrote: On 09/06/2017 03:51 PM, David Daney wrote: [...] Consider instead the case of a Marvell phy with no interrupts connected on a v4.9.43 kernel, single CPU: 0) | phy_disconnect() { 0) |

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 03:51 PM, David Daney wrote: > On 09/06/2017 01:49 PM, David Daney wrote: >> On 09/06/2017 11:59 AM, Florian Fainelli wrote: >>> On 09/06/2017 11:00 AM, David Daney wrote: On 08/31/2017 11:29 AM, Florian Fainelli wrote: > On 08/31/2017 11:12 AM, Mason wrote: >> On

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 01:49 PM, David Daney wrote: On 09/06/2017 11:59 AM, Florian Fainelli wrote: On 09/06/2017 11:00 AM, David Daney wrote: On 08/31/2017 11:29 AM, Florian Fainelli wrote: On 08/31/2017 11:12 AM, Mason wrote: On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM,

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 11:59 AM, Florian Fainelli wrote: On 09/06/2017 11:00 AM, David Daney wrote: On 08/31/2017 11:29 AM, Florian Fainelli wrote: On 08/31/2017 11:12 AM, Mason wrote: On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM, Mason wrote: On 31/08/2017 18:57, Florian

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 08:51 AM, Mason wrote: > On 31/08/2017 21:18, Florian Fainelli wrote: > >> On 08/31/2017 12:09 PM, Mason wrote: >> >>> On 31/08/2017 19:03, Florian Fainelli wrote: >>> On 08/31/2017 05:29 AM, Marc Gonzalez wrote: > On 31/08/2017 02:49, Florian Fainelli wrote: >

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 07:55 AM, Mason wrote: > On 31/08/2017 21:18, Florian Fainelli wrote: > >> On 08/31/2017 12:09 PM, Mason wrote: >> >>> 1) nb8800_link_reconfigure() calls phy_print_status() >>> which prints the "Link down" and "Link up" messages >>> to the console. With the patch reverted, nothing

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 06/09/2017 20:00, David Daney wrote: > On 08/31/2017 11:29 AM, Florian Fainelli wrote: >> On 08/31/2017 11:12 AM, Mason wrote: >>> On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM, Mason wrote: > On 31/08/2017 18:57, Florian Fainelli wrote: >> And the race is

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 11:00 AM, David Daney wrote: > On 08/31/2017 11:29 AM, Florian Fainelli wrote: >> On 08/31/2017 11:12 AM, Mason wrote: >>> On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM, Mason wrote: > On 31/08/2017 18:57, Florian Fainelli wrote: >> And the race is

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 11:29 AM, Florian Fainelli wrote: On 08/31/2017 11:12 AM, Mason wrote: On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM, Mason wrote: On 31/08/2017 18:57, Florian Fainelli wrote: And the race is between phy_detach() setting phydev->attached_dev = NULL and

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 09/06/2017 07:33 AM, Mason wrote: On 31/08/2017 20:29, Florian Fainelli wrote: On 08/31/2017 11:12 AM, Mason wrote: On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM, Mason wrote: On 31/08/2017 18:57, Florian Fainelli wrote: And the race is between phy_detach() setting

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 21:18, Florian Fainelli wrote: On 08/31/2017 12:09 PM, Mason wrote: On 31/08/2017 19:03, Florian Fainelli wrote: On 08/31/2017 05:29 AM, Marc Gonzalez wrote: On 31/08/2017 02:49, Florian Fainelli wrote: The original motivation for this change originated from Marc Gonzalez

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 21:18, Florian Fainelli wrote: On 08/31/2017 12:09 PM, Mason wrote: 1) nb8800_link_reconfigure() calls phy_print_status() which prints the "Link down" and "Link up" messages to the console. With the patch reverted, nothing is printed when the link goes down, and the result is

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 20:29, Florian Fainelli wrote: On 08/31/2017 11:12 AM, Mason wrote: On 31/08/2017 19:53, Florian Fainelli wrote: On 08/31/2017 10:49 AM, Mason wrote: On 31/08/2017 18:57, Florian Fainelli wrote: And the race is between phy_detach() setting phydev->attached_dev = NULL and

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 12:09 PM, Mason wrote: > On 31/08/2017 19:03, Florian Fainelli wrote: > >> On 08/31/2017 05:29 AM, Marc Gonzalez wrote: >> >>> On 31/08/2017 02:49, Florian Fainelli wrote: >>> The original motivation for this change originated from Marc Gonzalez indicating that his network

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 19:03, Florian Fainelli wrote: > On 08/31/2017 05:29 AM, Marc Gonzalez wrote: > >> On 31/08/2017 02:49, Florian Fainelli wrote: >> >>> The original motivation for this change originated from Marc Gonzalez >>> indicating that his network driver did not have its adjust_link callback

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 11:12 AM, Mason wrote: > On 31/08/2017 19:53, Florian Fainelli wrote: >> On 08/31/2017 10:49 AM, Mason wrote: >>> On 31/08/2017 18:57, Florian Fainelli wrote: And the race is between phy_detach() setting phydev->attached_dev = NULL and phy_state_machine() running in

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 19:53, Florian Fainelli wrote: > On 08/31/2017 10:49 AM, Mason wrote: >> On 31/08/2017 18:57, Florian Fainelli wrote: >>> And the race is between phy_detach() setting phydev->attached_dev = NULL >>> and phy_state_machine() running in PHY_HALTED state and calling >>>

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 10:49 AM, Mason wrote: > On 31/08/2017 18:57, Florian Fainelli wrote: >> And the race is between phy_detach() setting phydev->attached_dev = NULL >> and phy_state_machine() running in PHY_HALTED state and calling >> netif_carrier_off(). > > I must be missing something. > (Since a

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 18:57, Florian Fainelli wrote: > And the race is between phy_detach() setting phydev->attached_dev = NULL > and phy_state_machine() running in PHY_HALTED state and calling > netif_carrier_off(). I must be missing something. (Since a thread cannot race against itself.)

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 18:36, David Daney wrote: > On 08/31/2017 05:29 AM, Marc Gonzalez wrote: >> On 31/08/2017 02:49, Florian Fainelli wrote: >> >>> This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: >>> Correctly process PHY_HALTED in phy_stop_machine()") because it is >>> creating

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 05:29 AM, Marc Gonzalez wrote: > On 31/08/2017 02:49, Florian Fainelli wrote: > >> This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: >> Correctly process PHY_HALTED in phy_stop_machine()") because it is >> creating the possibility for a NULL pointer

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 09:36 AM, David Daney wrote: > On 08/31/2017 05:29 AM, Marc Gonzalez wrote: >> On 31/08/2017 02:49, Florian Fainelli wrote: >> >>> This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: >>> Correctly process PHY_HALTED in phy_stop_machine()") because it is >>>

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 08/31/2017 05:29 AM, Marc Gonzalez wrote: On 31/08/2017 02:49, Florian Fainelli wrote: This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: Correctly process PHY_HALTED in phy_stop_machine()") because it is creating the possibility for a NULL pointer dereference. David

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 14:29, Marc Gonzalez wrote: > On 31/08/2017 02:49, Florian Fainelli wrote: > >> The original motivation for this change originated from Marc Gonzalez >> indicating that his network driver did not have its adjust_link callback >> executing with phydev->link = 0 while he was

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

On 31/08/2017 02:49, Florian Fainelli wrote: > This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: > Correctly process PHY_HALTED in phy_stop_machine()") because it is > creating the possibility for a NULL pointer dereference. > > David Daney provide the following call trace

Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"

From: Florian Fainelli Date: Wed, 30 Aug 2017 17:49:29 -0700 > This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy: > Correctly process PHY_HALTED in phy_stop_machine()") because it is > creating the possibility for a NULL pointer dereference. > > David