[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1275926151.2545.126.camel@edumazet-laptop>
Date: Mon, 07 Jun 2010 17:55:51 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Changli Gao <xiaosuo@...il.com>
Cc: David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
Stephen Hemminger <shemminger@...tta.com>,
Jarek Poplawski <jarkao2@...il.com>
Subject: Re: [PATCH net-next-2.6] pkt_sched: gen_estimator: kill est_lock
rwlock
Le lundi 07 juin 2010 à 17:30 +0200, Eric Dumazet a écrit :
> Le lundi 07 juin 2010 à 22:53 +0800, Changli Gao a écrit :
>
> > Hmm. I don't think it is correct.
> >
> > Look at this code:
> >
> > void xt_rateest_put(struct xt_rateest *est)
> > {
> > mutex_lock(&xt_rateest_mutex);
> > if (--est->refcnt == 0) {
> > hlist_del(&est->list);
> > gen_kill_estimator(&est->bstats, &est->rstats);
> > kfree(est);
> > }
> > mutex_unlock(&xt_rateest_mutex);
> > }
> >
> > est will be released after gen_kill_estimator() returns. After est is
> > freed, there may still be some other CPUs running in the branch:
> > if (bstats) {
> > ....
> > }
> >
>
> Oh well, I think I knew this from a previous attempt, but then I
> forgot :)
>
> I'll provide an updated patch, so that no other attempt is tried next
> yer, thanks !
>
>
For your information, bug is already there before my patch.
So this est_lock is a wrong protection, in the sense its so convoluted
that nobody but you and me even noticed it was buggy in the first place.
(see commit 5d944c640b4 for a first patch)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists