Thread (19 messages) 19 messages, 4 authors, 2015-03-31

Re: [Patch net-next] fib: move fib_rules_cleanup_ops() under rtnl lock

From: Cong Wang <hidden>
Date: 2015-03-31 17:56:33

On Tue, Mar 31, 2015 at 10:30 AM, Alexander Duyck
[off-list ref] wrote:
On 03/31/2015 09:47 AM, Cong Wang wrote:
quoted
On Mon, Mar 30, 2015 at 8:10 PM, Alexander Duyck
[off-list ref] wrote:
quoted
On 03/30/2015 05:12 PM, Cong Wang wrote:
quoted
2) remove the unnecessary rules_mod_lock

Thanks.

Please define "unnecessary" as we have had a bit of back and forth on how
our views can differ there.  As far as I know it still has to be held for
the fib_rule_ops list manipulation, specifically the call to
list_del_rcu.
However, it doesn't need to be held when we call fib_rules_cleanup_ops.
Look at where rules_mod_lock are held: either when the net is initialized
or when unregistering, neither of them really needs this per netns lock:
new netns is not ready to expose;
concurrent unregistering is prevented by upper layer locking,
readers (lookup_rules_ops) hold RCU but we already should hold rtnl lock
(after patch of course).

I would have almost agreed with you, however I noticed that decnet doesn't
seem to follow the same rules as the rest of the callers to
fib_rules_register.  It will simply call into module_init -> decnet_init ->
dn_fib_init -> dn_fib_rules_injt -> fib_rules_register(&init_net).  It
probably just needs to be rewritten to use
register_pernet_subsys/unregister_pernet_subsys, or correctly use the
net_mutex, and then you could probably go head and tear out the
rules_mod_lock since they would all be covered under the net_mutex.
Good catch, or it can simply hold rtnl lock in module init path.
Anyway, it is for net-next.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help