Re: [PATCH 01/20] net: NETDEV_UNREGISTER_PERNET -> NETDEV_UNREGISTER_BATCH

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

David Miller <davem@davemloft.net> writes:

> From: "Eric W. Biederman" <ebiederm@xmission.com>
> Date: Sun, 29 Nov 2009 17:45:58 -0800
>
>> Additionally it appears that we moved the route cache flush after
>> the final synchronize_net, which seems wrong and there was no
>> explanation.  So I have restored the original location of the final
>> synchronize_net.
>
> I think the idea was that it's important for all pending stale
> RCU references to the device to be gone before we send out the
> notifier.  And that's why the synchronize_net() comes first.
>
> This needs more thought.

The only case the affects (today) is the invalidation of the route/flow cache.

The routing table until the addition of NETDEV_UNREGISTER_PERNET happened
in the notifier NETDEV_UNREGISTER, which comes before the final synchronize_net.

The routing cache has rcu references to the network device.

So it appears to me that with the current pernet batched routing code cleanup we
can end up calling kfree before all of the call_rcu functions triggered by
rt_do_flush will be run.

    netdev_run_todo()
       kobject_put()
         netdev_release()
           kfree()

NETDEV_UNREGISTER_PERCPU
    rt_cache_flush(xxx, 0)
       rt_do_flush()
         rt_free
           call_rcu

So it appears to me that moving the final syncrhonize_net() back where it was before
the NETDEV_UNREGISTER_PERNET changes (as I did) is necessary for correctness.

Eric