From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick McHardy Subject: Re: [NET]: gen_estimator: fix locking and timer related bugs [Re: [Bugme-new] [Bug 8668] New: HTB Deadlock] Date: Thu, 28 Jun 2007 14:23:36 +0200 Message-ID: <4683A848.5040907@trash.net> References: <20070627114521.GA3762@ff.dom.local> <46824D88.1090300@trash.net> <20070627121013.GB3762@ff.dom.local> <468279FC.3070502@trash.net> <46827DB1.3060509@trash.net> <46828179.5030404@trash.net> <20070628091339.GC1618@ff.dom.local> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: David Miller , Andrew Morton , netdev@vger.kernel.org, "bugme-daemon@kernel-bugs.osdl.org" , ranko@spidernet.net To: Jarek Poplawski Return-path: Received: from stinky.trash.net ([213.144.137.162]:49341 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757146AbXF1MZG (ORCPT ); Thu, 28 Jun 2007 08:25:06 -0400 In-Reply-To: <20070628091339.GC1618@ff.dom.local> Sender: netdev-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Jarek Poplawski wrote: >> @@ -202,7 +201,6 @@ void gen_kill_estimator(struct gnet_stats_basic *bstats, >> struct gen_estimator *est, **pest; >> >> for (idx=0; idx <= EST_MAX_INTERVAL; idx++) { >> - int killed = 0; >> pest = &elist[idx].list; >> while ((est=*pest) != NULL) { > > So, maybe this list walking here needs some locking too? It depends on whether estimators should be able to rely on the rtnl in the future or be completely responsible for their own locking. My patch yesterday was made under the assumption that they shouldn't rely on external locking, which seemed to be the right thing for a "generic" implementation. OTOH its still specific to networking, so relying on the rtnl doesn't sound too unreasonable too. I'm beginning to thing I made the wrong choice with my patch. I'm busy right now, would you mind looking into a patch that only deals with the timer races, but still relies on the rtnl?