All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: David Miller <davem@davemloft.net>, netdev@vger.kernel.org
Subject: Re: [PATCH v3] net: #ifdef inet_bind_bucket::ib_net
Date: Fri, 14 Nov 2008 07:21:13 +0100	[thread overview]
Message-ID: <491D18D9.1050708@cosmosbay.com> (raw)
In-Reply-To: <20081114061452.GA2227@x200.localdomain>

Alexey Dobriyan a écrit :
> On Fri, Nov 14, 2008 at 05:54:26AM +0100, Eric Dumazet wrote:
>> David Miller a écrit :
>>> From: Eric Dumazet <dada1@cosmosbay.com>
>>> 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.





  reply	other threads:[~2008-11-14  6:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-13 23:21 [PATCH v3] net: #ifdef inet_bind_bucket::ib_net Alexey Dobriyan
2008-11-14  4:36 ` Eric Dumazet
2008-11-14  4:40   ` David Miller
2008-11-14  4:54     ` Eric Dumazet
2008-11-14  6:14       ` Alexey Dobriyan
2008-11-14  6:21         ` Eric Dumazet [this message]
2008-11-14  6:41           ` Alexey Dobriyan
  -- strict thread matches above, loose matches on Subject: below --
2008-11-13 22:53 Alexey Dobriyan
2008-11-13 23:03 ` David Miller
2008-11-13 23:09 ` Eric Dumazet
2008-11-11  0:44 [PATCH] net: introduce read_pnet() and write_pnet() functions David Miller
2008-11-11 11:08 ` [PATCH] net: #ifdef inet_bind_bucket::ib_net Alexey Dobriyan
2008-11-11 11:19   ` [PATCH v2] " Alexey Dobriyan
2008-11-12  0:45     ` David Miller
2008-11-12 10:44       ` Alexey Dobriyan
2008-11-12 10:50         ` Eric Dumazet
2008-11-12 12:24           ` [PATCH v3] " Alexey Dobriyan
2008-11-12 12:24             ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=491D18D9.1050708@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=adobriyan@gmail.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.