Re: [CFT][PATCH] net: Delay default_device_exit_batch until no devices are unregistering

[ebiederm@xmission.com (Eric W. Biederman)]

David Miller <davem@davemloft.net> writes:

> From: Francesco Ruggeri <fruggeri@aristanetworks.com>
> Date: Tue, 17 Sep 2013 20:50:41 -0700

>> I am not sure that would work.
>> list_empty(&net_todo_list) does not guarantee that there are no
>> unregistering devices still in flight.
>> Another process may have copied net_todo_run into its local list, left
>> net_todo_list empty and still be in the middle of processing
>> unregistering devices (without the rtnl lock) when
>> default_device_exit_batch starts executing.
>
> Ok, now I understand.
>
> Eric B., when you get a chance can you resubmit your patch and perhaps
> elaborate on the situation in the commit message.

The code is on my computer at home so I won't be able to get back to
this until Sunday or Monday.  This should give Francesco time to verify
I really have closed the holes he is seeing.

> If I was confused I'm sure other people will be if they look into
> this in the future.

Agreed.  It is easy to get confused with this issue.  And I was in a
rush so I expect my wording could be improved.



There is still the other part of this issue that dev_close reorders
devices in default_device_exit_batch.

With that reordering we can get the call chain:
  netdev_run_todo
    call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL,...)
      dst_dev_event
        dst_ifdown(..., unregister == 1)
          dst->dev = dev_net(dst->dev)->loopback_dev;
	  dev_hold(dst->dev);

With dev_net(dst->dev)->loopback_dev == NULL;

So my patch to split the close_list out of the unreg_list is also needed
(or something else to address that issue). 

Hopefully my exaspearation with dev_close_many didn't obscure what is
happening there.

Eric