Jarek Poplawski wrote:

On Thu, Jun 28, 2007 at 02:23:36PM +0200, Patrick McHardy wrote:

quoted

Jarek Poplawski wrote:

quoted

quoted

@@ -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?

In that case this patch looks OK & enough.

Its overkill in that case. The concurrent additions and removals
can't happen.