From: Pavel Belous <pavel.bel...@aquantia.com> Date: Mon, 20 Feb 2017 14:41:51 +0300
> From: Pavel Belous <pavel.bel...@aquantia.com> > > netdev_register should be called when everything is initialized. > > Signed-off-by: Pavel Belous <pavel.bel...@aquantia.com> > Reviewed-by: Lino Sanfilippo <linosanfili...@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.
