On 5/25/2018 3:37 PM, Stephen Hemminger wrote: > On Thu, 24 May 2018 09:55:13 -0700 > Sridhar Samudrala wrote: > > >> + spin_lock(&failover_lock); > Since register is not in fast path, this should be a mutex? This is Jiri's comment which made me to switch to spinlock from mutex >> Why mutex? Apparently you don't need to sleep while holding a lock. >> Simple spinlock would do. > > >> +int failover_slave_unregister(struct net_device *slave_dev) >> +{ >> + struct net_device *failover_dev; >> + struct failover_ops *fops; >> + >> + if (!netif_is_failover_slave(slave_dev)) >> + goto done; >> + >> + ASSERT_RTNL(); >> + >> + failover_dev = failover_get_bymac(slave_dev->perm_addr, &fops); >> + if (!failover_dev) >> + goto done; > Since the slave device must have a master device set already, why not use > that instead of searching by MAC address on unregister or link change. > We also need to get the fops(failover_ops)