netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, Stephen Hemminger <shemminger@vyatta.com>
Subject: Re: [PATCH 6/13] bridge: Add core IGMP snooping support
Date: Mon, 8 Mar 2010 19:15:46 -0800	[thread overview]
Message-ID: <20100309031546.GE6739@linux.vnet.ibm.com> (raw)
In-Reply-To: <201003081950.48384.arnd@arndb.de>

On Mon, Mar 08, 2010 at 07:50:48PM +0100, Arnd Bergmann wrote:
> On Sunday 07 March 2010, Paul E. McKenney wrote:
> > On Sun, Mar 07, 2010 at 10:45:00AM +0800, Herbert Xu wrote:
> > > On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
> > 
> > OK, just re-checked your patch, and it looks OK.
> > 
> > Also adding Arnd to CC.
> > 
> > Arnd, would it be reasonable to extend your RCU-sparse changes to have
> > four different pointer namespaces, one for each flavor of RCU?  (RCU,
> > RCU-bh, RCU-sched, and SRCU)?  Always a fan of making the computer do
> > the auditing where reasonable.  ;-)
> 
> Yes, I guess that would be possible. I'd still leave out the rculist
> from any annotations for now, as this would get even more complex then.

Understood!

> One consequence will be the need for new rcu_assign_pointer{,_bh,_sched}
> macros that check the address space of the first argument, otherwise
> you'd be able to stick anything in there, including non-__rcu pointers.
> 
> I've also found a few places (less than a handful) that use RCU to
> protect per-CPU data. Not sure how to deal with that, because now
> this also has its own named address space (__percpu), and it's probably
> a bit too much to introduce all combinations of
> {s,}rcu_{assign_pointer,dereference}{,_bh,_sched}{,_const}{,_percpu},
> so I'm ignoring them for now.

Ouch!!!

> > This could potentially catch the mismatched call_rcu()s, at least if the
> > rcu_head could be labeled.
> 
> I haven't labeled the rcu_head at all so far, and I'm not sure if that's
> necessary. What I've been thinking about is replacing typical code like
> 
> /* this is called with the writer-side lock held */
> void foo_assign(struct foo *foo, struct bar *newbar)
> {
> 	struct bar *bar = rcu_dereference_const(foo->bar); /* I just had to add 
> 							    this dereference */
> 	rcu_assign_pointer(foo->bar, newbar);
> 	if (bar)
> 		call_rcu(&bar->rcu, bar_destructor);
> }
> 
> with the shorter
> 
> void foo_assign(struct foo *foo, struct bar *newbar)
> {
> 	struct bar *bar = rcu_exchange(foo->bar, newbar);
> 	if (bar)
> 		call_rcu(&bar->rcu, bar_destructor);
> }
> 
> Now we could combine this to
> 
> void foo_assign(struct foo *foo, struct bar *newbar)
> {
> 	rcu_exchange_call(foo->bar, newbar, rcu, bar_destructor);
> }
> 
> #define rcu_exchange_call(ptr, new, member, func) \
> ({ \
> 	typeof(new) old = rcu_exchange((ptr),(new)); \
> 	if (old) \
> 		call_rcu(&(old)->member, (func));	\
> 	old; \
> })
> 
> and make appropriate versions of all the above rcu methods for this.
> With some extra macro magic, this could even become type safe and
> accept a function that takes a typeof(ptr) argument instead of the
> rcu_head.

This approach does look promising!  And probably a lot simpler than
attempting to label the rcu_head structure.  I am not yet convinced
about the typesafe function taking typeof(ptr), but possibly I am
suffering a failure of C-preprocessor imagination?

							Thanx, Paul

  reply	other threads:[~2010-03-09  3:15 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-26 15:34 [RFC] [1/13] bridge: Add IGMP snooping support Herbert Xu
2010-02-26 15:35 ` [PATCH 1/13] bridge: Do br_pass_frame_up after other ports Herbert Xu
2010-02-26 15:35 ` [PATCH 2/13] bridge: Allow tail-call on br_pass_frame_up Herbert Xu
2010-02-27 11:14   ` David Miller
2010-02-27 15:36     ` Herbert Xu
2010-02-26 15:35 ` [PATCH 3/13] bridge: Avoid unnecessary clone on forward path Herbert Xu
2010-02-26 15:35 ` [PATCH 4/13] bridge: Use BR_INPUT_SKB_CB on xmit path Herbert Xu
2010-02-26 15:35 ` [PATCH 5/13] bridge: Split may_deliver/deliver_clone out of br_flood Herbert Xu
2010-02-26 15:35 ` [PATCH 6/13] bridge: Add core IGMP snooping support Herbert Xu
2010-02-26 15:35 ` [PATCH 7/13] bridge: Add multicast forwarding functions Herbert Xu
2010-02-26 15:35 ` [PATCH 8/13] bridge: Add multicast start/stop hooks Herbert Xu
2010-02-26 15:35 ` [PATCH 9/13] bridge: Add multicast data-path hooks Herbert Xu
2010-02-26 15:35 ` [PATCH 10/13] bridge: Add multicast_router sysfs entries Herbert Xu
2010-02-27  0:42   ` Stephen Hemminger
2010-02-27 11:29     ` David Miller
2010-02-27 15:53       ` Herbert Xu
2010-03-09 12:25     ` Herbert Xu
2010-03-09 12:26       ` Herbert Xu
2010-02-26 15:35 ` [PATCH 11/13] bridge: Add multicast_snooping sysfs toggle Herbert Xu
2010-02-26 15:35 ` [PATCH 12/13] bridge: Add hash elasticity/max sysfs entries Herbert Xu
2010-02-26 15:35 ` [PATCH 13/13] bridge: Add multicast count/interval " Herbert Xu
2010-02-28  5:40 ` [1/13] bridge: Add IGMP snooping support Herbert Xu
2010-02-28  5:41   ` [PATCH 1/13] bridge: Do br_pass_frame_up after other ports Herbert Xu
2010-02-28  5:41   ` [PATCH 2/13] bridge: Allow tail-call on br_pass_frame_up Herbert Xu
2010-02-28  5:41   ` [PATCH 3/13] bridge: Avoid unnecessary clone on forward path Herbert Xu
2010-02-28  5:41   ` [PATCH 4/13] bridge: Use BR_INPUT_SKB_CB on xmit path Herbert Xu
2010-02-28  5:41   ` [PATCH 5/13] bridge: Split may_deliver/deliver_clone out of br_flood Herbert Xu
2010-02-28  5:41   ` [PATCH 6/13] bridge: Add core IGMP snooping support Herbert Xu
2010-03-05 23:43     ` Paul E. McKenney
2010-03-06  1:17       ` Herbert Xu
2010-03-06  5:06         ` Paul E. McKenney
2010-03-06  6:56           ` Herbert Xu
2010-03-06  7:03             ` Herbert Xu
2010-03-07 23:31               ` David Miller
2010-03-06  7:07             ` Herbert Xu
2010-03-07 23:31               ` David Miller
2010-03-06 15:00             ` Paul E. McKenney
2010-03-06 15:19             ` Paul E. McKenney
2010-03-06 19:00               ` Paul E. McKenney
2010-03-07  2:45                 ` Herbert Xu
2010-03-07  3:11                   ` Paul E. McKenney
2010-03-08 18:50                     ` Arnd Bergmann
2010-03-09  3:15                       ` Paul E. McKenney [this message]
2010-03-11 18:49                       ` Arnd Bergmann
2010-03-14 23:01                         ` Paul E. McKenney
2010-03-09 21:12                     ` Arnd Bergmann
2010-03-10  2:14                       ` Paul E. McKenney
2010-03-10  9:41                         ` Arnd Bergmann
2010-03-10 10:39                           ` Eric Dumazet
2010-03-10 10:49                             ` Herbert Xu
2010-03-10 13:13                               ` Paul E. McKenney
2010-03-10 14:07                                 ` Herbert Xu
2010-03-10 16:26                                   ` Paul E. McKenney
2010-03-10 16:35                                     ` David Miller
2010-03-10 17:56                                       ` Arnd Bergmann
2010-03-10 21:25                                         ` Paul E. McKenney
2010-03-10 13:27                               ` Arnd Bergmann
2010-03-10 13:39                               ` Arnd Bergmann
2010-03-10 13:19                           ` Paul E. McKenney
2010-03-10 13:30                             ` Arnd Bergmann
2010-03-10 13:57                               ` Paul E. McKenney
2010-02-28  5:41   ` [PATCH 7/13] bridge: Add multicast forwarding functions Herbert Xu
2010-02-28  5:41   ` [PATCH 8/13] bridge: Add multicast start/stop hooks Herbert Xu
2010-02-28  5:41   ` [PATCH 9/13] bridge: Add multicast data-path hooks Herbert Xu
2010-04-27 17:13     ` [PATCH net-next] bridge: use is_multicast_ether_addr Stephen Hemminger
2010-04-27 19:53       ` David Miller
2010-02-28  5:41   ` [PATCH 10/13] bridge: Add multicast_router sysfs entries Herbert Xu
2010-04-27 17:13     ` [PATCH net-next] bridge: multicast router list manipulation Stephen Hemminger
2010-04-27 19:54       ` David Miller
2010-04-27 23:11       ` Michał Mirosław
2010-04-27 23:25         ` Stephen Hemminger
2010-04-27 23:28           ` David Miller
2010-04-27 23:44             ` Stephen Hemminger
2010-04-27 23:51               ` David Miller
2010-04-27 23:27         ` David Miller
2010-04-28  1:51       ` Herbert Xu
2010-02-28  5:41   ` [PATCH 11/13] bridge: Add multicast_snooping sysfs toggle Herbert Xu
2010-02-28  5:41   ` [PATCH 12/13] bridge: Add hash elasticity/max sysfs entries Herbert Xu
2010-02-28  5:41   ` [PATCH 13/13] bridge: Add multicast count/interval " Herbert Xu
2010-02-28  8:52   ` [1/13] bridge: Add IGMP snooping support David Miller
2010-03-01  2:08     ` Herbert Xu

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=20100309031546.GE6739@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=netdev@vger.kernel.org \
    --cc=shemminger@vyatta.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).