Subject: Re: [PATCH v3] net: #ifdef inet_bind_bucket::ib_net

From: Eric Dumazet <[email-masked]>
Date: Fri, 14 Nov 2008 07:21:13 +0100

Alexey Dobriyan a écrit :
> On Fri, Nov 14, 2008 at 05:54:26AM +0100, Eric Dumazet wrote:
>> David Miller a écrit :
>>> From: Eric Dumazet <[email-masked]>
>>> Date: Fri, 14 Nov 2008 05:36:15 +0100
>>>
>>>> This is better because :
>>>>
>>>> 1) No #ifdef CONFIG_NET_NS
>>>>
>>>> 2) The magic about &init_net is not duplicated in ten different include files, but
>>>>    centralized in the right file : include/net/net_namespace.h
>>> I %100 agree.
>> Speaking of those functions, what do you think of this one ?
>>
>> static inline
>> void dev_net_set(struct net_device *dev, struct net *net)
>> {
>> #ifdef CONFIG_NET_NS
>>        release_net(dev->nd_net);
>>        dev->nd_net = hold_net(net);
>> #endif
>> }
>>
>> I believe that its safer to hold a reference on "new" *before*
>> releasing reference on "old" object.
>>
>> Also, release_net() and hold_net() can be defined to do
>> the use_count refcounting regardless of CONFIG_NET_NS
>> (Its a different NETNS_REFCNT_DEBUG #ifdef)
> 
> NETNS_REFCNT_DEBUG makes sense only with NET_NS=y because init_net
> is never freed.

It makes sense when you want to debug things, without full NET_NS

Check CONFIG_PROVE_LOCKING or other debugging stuff

CONFIG_SMP can be unset, still we are able to check spinlocks are
lock()/unlock() are correctly paired

> 
>> Yet another example where read_pnet() and write_pnet()
>> are the right answer : Its cleaner and fixes *bugs*.
> 
> pnet stuff by definition can't fix bugs :-)

It will, I bet it.

Cant you see difference between

#ifdef CONFIG_NET_NS
	ptr->net = expression;
#endif

and

write_pnet(&ptr->net, expression);

Answer : In the 2nd case, (expression) is evaluated.

In case its evaluation has side effects (like... refcounting), it makes a huge difference.