* [PATCH net-next] phylink: add ASSERT_RTNL() on phylink connect functions
@ 2019-10-23 12:32 Ioana Ciornei
2019-10-23 13:10 ` Russell King - ARM Linux admin
0 siblings, 1 reply; 3+ messages in thread
From: Ioana Ciornei @ 2019-10-23 12:32 UTC (permalink / raw)
To: davem, netdev; +Cc: andrew, linux, Ioana Ciornei
The appropriate assert on the rtnl lock is not present in phylink's
connect functions which makes unusual calls to them not to be catched.
Add the appropriate ASSERT_RTNL().
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/net/phy/phylink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index be7a2c0fa59b..d0aa0c861b2d 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -786,6 +786,8 @@ static int __phylink_connect_phy(struct phylink *pl, struct phy_device *phy,
*/
int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
{
+ ASSERT_RTNL();
+
/* Use PHY device/driver interface */
if (pl->link_interface == PHY_INTERFACE_MODE_NA) {
pl->link_interface = phy->interface;
@@ -815,6 +817,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn,
struct phy_device *phy_dev;
int ret;
+ ASSERT_RTNL();
+
/* Fixed links and 802.3z are handled without needing a PHY */
if (pl->link_an_mode == MLO_AN_FIXED ||
(pl->link_an_mode == MLO_AN_INBAND &&
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next] phylink: add ASSERT_RTNL() on phylink connect functions
2019-10-23 12:32 [PATCH net-next] phylink: add ASSERT_RTNL() on phylink connect functions Ioana Ciornei
@ 2019-10-23 13:10 ` Russell King - ARM Linux admin
2019-10-23 15:59 ` Ioana Ciornei
0 siblings, 1 reply; 3+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-23 13:10 UTC (permalink / raw)
To: Ioana Ciornei; +Cc: davem, netdev, andrew
On Wed, Oct 23, 2019 at 03:32:20PM +0300, Ioana Ciornei wrote:
> The appropriate assert on the rtnl lock is not present in phylink's
> connect functions which makes unusual calls to them not to be catched.
> Add the appropriate ASSERT_RTNL().
As I previously replied, this is not necessary. It is safe to attach
PHYs _prior_ to the netdev being registered without taking the rtnl
lock, just like phylib's phy_connect()/phy_attach() are safe.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> drivers/net/phy/phylink.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index be7a2c0fa59b..d0aa0c861b2d 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -786,6 +786,8 @@ static int __phylink_connect_phy(struct phylink *pl, struct phy_device *phy,
> */
> int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
> {
> + ASSERT_RTNL();
> +
> /* Use PHY device/driver interface */
> if (pl->link_interface == PHY_INTERFACE_MODE_NA) {
> pl->link_interface = phy->interface;
> @@ -815,6 +817,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn,
> struct phy_device *phy_dev;
> int ret;
>
> + ASSERT_RTNL();
> +
> /* Fixed links and 802.3z are handled without needing a PHY */
> if (pl->link_an_mode == MLO_AN_FIXED ||
> (pl->link_an_mode == MLO_AN_INBAND &&
> --
> 1.9.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next] phylink: add ASSERT_RTNL() on phylink connect functions
2019-10-23 13:10 ` Russell King - ARM Linux admin
@ 2019-10-23 15:59 ` Ioana Ciornei
0 siblings, 0 replies; 3+ messages in thread
From: Ioana Ciornei @ 2019-10-23 15:59 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: davem@davemloft.net, netdev@vger.kernel.org, andrew@lunn.ch
On 10/23/19 4:10 PM, Russell King - ARM Linux admin wrote:
> On Wed, Oct 23, 2019 at 03:32:20PM +0300, Ioana Ciornei wrote:
>> The appropriate assert on the rtnl lock is not present in phylink's
>> connect functions which makes unusual calls to them not to be catched.
>> Add the appropriate ASSERT_RTNL().
>
> As I previously replied, this is not necessary. It is safe to attach
> PHYs _prior_ to the netdev being registered without taking the rtnl
> lock, just like phylib's phy_connect()/phy_attach() are safe.
>
Yeah.. I was trying to catch other errors like in the case of dpaa2-eth
where we also connect/disconnect when we are notified by the firmware of
a DPMAC disconnect.
I now understand why the assert is not always needed in the _connect().
Ioana
>>
>> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>> ---
>> drivers/net/phy/phylink.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>> index be7a2c0fa59b..d0aa0c861b2d 100644
>> --- a/drivers/net/phy/phylink.c
>> +++ b/drivers/net/phy/phylink.c
>> @@ -786,6 +786,8 @@ static int __phylink_connect_phy(struct phylink *pl, struct phy_device *phy,
>> */
>> int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
>> {
>> + ASSERT_RTNL();
>> +
>> /* Use PHY device/driver interface */
>> if (pl->link_interface == PHY_INTERFACE_MODE_NA) {
>> pl->link_interface = phy->interface;
>> @@ -815,6 +817,8 @@ int phylink_of_phy_connect(struct phylink *pl, struct device_node *dn,
>> struct phy_device *phy_dev;
>> int ret;
>>
>> + ASSERT_RTNL();
>> +
>> /* Fixed links and 802.3z are handled without needing a PHY */
>> if (pl->link_an_mode == MLO_AN_FIXED ||
>> (pl->link_an_mode == MLO_AN_INBAND &&
>> --
>> 1.9.1
>>
>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-23 15:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-23 12:32 [PATCH net-next] phylink: add ASSERT_RTNL() on phylink connect functions Ioana Ciornei
2019-10-23 13:10 ` Russell King - ARM Linux admin
2019-10-23 15:59 ` Ioana Ciornei
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).