Thread (21 messages) 21 messages, 3 authors, 2009-11-30

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

From: Eric W. Biederman <hidden>
Date: 2009-11-30 23:42:18

David Miller [off-list ref] writes:
From: "Eric W. Biederman" <redacted>
Date: Sun, 29 Nov 2009 17:45:58 -0800
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help