| Message ID | 20090928113855.52eab44b@nehalam |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Stephen Hemminger a écrit : > Don't need a read/write lock here sinc there already is a spin lock that > is being acquired. > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> > > --- a/net/core/gen_estimator.c 2009-09-24 16:27:01.239755070 -0700 > +++ b/net/core/gen_estimator.c 2009-09-24 16:41:09.290751273 -0700 > @@ -101,9 +101,6 @@ struct gen_estimator_head > > static struct gen_estimator_head elist[EST_MAX_INTERVAL+1]; > > -/* Protects against NULL dereference */ > -static DEFINE_RWLOCK(est_lock); > - > /* Protects against soft lockup during large deletion */ > static struct rb_root est_root = RB_ROOT; > > @@ -118,9 +115,8 @@ static void est_timer(unsigned long arg) > u64 brate; > u32 npackets; > u32 rate; > - > + > spin_lock(e->stats_lock); > - read_lock(&est_lock); > if (e->bstats == NULL) > goto skip; > > @@ -136,7 +132,6 @@ static void est_timer(unsigned long arg) > e->avpps += (rate >> e->ewma_log) - (e->avpps >> e->ewma_log); > e->rate_est->pps = (e->avpps+0x1FF)>>10; > skip: > - read_unlock(&est_lock); > spin_unlock(e->stats_lock); > } > > @@ -270,9 +265,9 @@ void gen_kill_estimator(struct gnet_stat > while ((e = gen_find_node(bstats, rate_est))) { > rb_erase(&e->node, &est_root); > > - write_lock_bh(&est_lock); > + spin_lock(e->stats_lock); Are you sure _bh() variant is not needed here ? > e->bstats = NULL; > - write_unlock_bh(&est_lock); > + spin_unlock(e->stats_lock); > > list_del_rcu(&e->list); > call_rcu(&e->e_rcu, __gen_kill_estimator); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 28 Sep 2009 20:50:28 +0200 > Stephen Hemminger a écrit : >> Don't need a read/write lock here sinc there already is a spin lock that >> is being acquired. >> >> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> ... >> @@ -270,9 +265,9 @@ void gen_kill_estimator(struct gnet_stat >> while ((e = gen_find_node(bstats, rate_est))) { >> rb_erase(&e->node, &est_root); >> >> - write_lock_bh(&est_lock); >> + spin_lock(e->stats_lock); > > Are you sure _bh() variant is not needed here ? Right, that need to be fixed to be spin_lock_bh() and spin_unlock_bh() in the next hunk. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/net/core/gen_estimator.c 2009-09-24 16:27:01.239755070 -0700 +++ b/net/core/gen_estimator.c 2009-09-24 16:41:09.290751273 -0700 @@ -101,9 +101,6 @@ struct gen_estimator_head static struct gen_estimator_head elist[EST_MAX_INTERVAL+1]; -/* Protects against NULL dereference */ -static DEFINE_RWLOCK(est_lock); - /* Protects against soft lockup during large deletion */ static struct rb_root est_root = RB_ROOT; @@ -118,9 +115,8 @@ static void est_timer(unsigned long arg) u64 brate; u32 npackets; u32 rate; - + spin_lock(e->stats_lock); - read_lock(&est_lock); if (e->bstats == NULL) goto skip; @@ -136,7 +132,6 @@ static void est_timer(unsigned long arg) e->avpps += (rate >> e->ewma_log) - (e->avpps >> e->ewma_log); e->rate_est->pps = (e->avpps+0x1FF)>>10; skip: - read_unlock(&est_lock); spin_unlock(e->stats_lock); } @@ -270,9 +265,9 @@ void gen_kill_estimator(struct gnet_stat while ((e = gen_find_node(bstats, rate_est))) { rb_erase(&e->node, &est_root); - write_lock_bh(&est_lock); + spin_lock(e->stats_lock); e->bstats = NULL; - write_unlock_bh(&est_lock); + spin_unlock(e->stats_lock); list_del_rcu(&e->list); call_rcu(&e->e_rcu, __gen_kill_estimator);
Don't need a read/write lock here sinc there already is a spin lock that is being acquired. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html