Re: [PATCH net-next v3 09/12] net: ethernet: aquantia: Call netdev_register after all initialized.
From: David Miller <davem@davemloft.net>
Date: 2017-02-20 16:28:22
From: Pavel Belous <redacted> Date: Mon, 20 Feb 2017 14:41:51 +0300
quoted hunk ↗ jump to hunk
From: Pavel Belous <redacted> netdev_register should be called when everything is initialized. Signed-off-by: Pavel Belous <redacted> Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de> --- drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c index a8a27c5..fdd7d72 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c@@ -261,16 +261,18 @@ int aq_nic_ndev_register(struct aq_nic_s *self) ether_addr_copy(self->ndev->dev_addr, mac_addr_permanent); } #endif - err = register_netdev(self->ndev); - if (err < 0) - goto err_exit; - self->is_ndev_registered = true; netif_carrier_off(self->ndev); for (i = AQ_CFG_VECS_MAX; i--;) aq_nic_ndev_queue_stop(self, i); + err = register_netdev(self->ndev); + if (err < 0) + goto err_exit; + + self->is_ndev_registered = true; +
Please get rid of this self->is_ndev_registered state, it is not necessary and it is racey. The very moment you call register_netdev(), your ->open() method along with the rest of your driver can be invoked. Therefore any dependency of state changes after register_netdev() cannot be depended upon by the rest of the driver, code is going to run immediately elsewhere in your driver as soon as that register_netdev() is invoked, and it is going to test that ->is_ndev_registered() value, and it's going to be wrong. Instead, test netdev->reg_state if you absolutely must have this piece of information.