From patchwork Wed May 20 04:54:22 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 27441 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 9FFE6B7067 for ; Wed, 20 May 2009 14:54:47 +1000 (EST) Received: by ozlabs.org (Postfix) id 93799DE142; Wed, 20 May 2009 14:54:47 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 2996CDE0D8 for ; Wed, 20 May 2009 14:54:47 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751217AbZETEyj (ORCPT ); Wed, 20 May 2009 00:54:39 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751105AbZETEyj (ORCPT ); Wed, 20 May 2009 00:54:39 -0400 Received: from gw1.cosmosbay.com ([212.99.114.194]:33246 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751038AbZETEyj convert rfc822-to-8bit (ORCPT ); Wed, 20 May 2009 00:54:39 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) by gw1.cosmosbay.com (8.13.7/8.13.7) with ESMTP id n4K4sNEO029786; Wed, 20 May 2009 06:54:24 +0200 Message-ID: <4A138CFE.5070901@cosmosbay.com> Date: Wed, 20 May 2009 06:54:22 +0200 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) MIME-Version: 1.0 To: David Miller CC: nhorman@tuxdriver.com, jarkao2@gmail.com, lav@yar.ru, shemminger@linux-foundation.org, netdev@vger.kernel.org Subject: [PATCH] net: fix length computation in rt_check_expire() References: <20090519162048.GB28034@hmsreliant.think-freely.org> <4A12FEDA.7040806@cosmosbay.com> <20090519192450.GF28034@hmsreliant.think-freely.org> <20090519.150517.62361946.davem@davemloft.net> In-Reply-To: <20090519.150517.62361946.davem@davemloft.net> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Wed, 20 May 2009 06:54:25 +0200 (CEST) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org David Miller a écrit : > From: Neil Horman > Date: Tue, 19 May 2009 15:24:50 -0400 > >>> Moving whole group in front would defeat the purpose of move, actually, >>> since rank in chain is used to decay the timeout in garbage collector. >>> (search for tmo >>= 1; ) >>> >> Argh, so the list is implicitly ordered by expiration time. That >> really defeats the entire purpose of doing grouping in the ilst at >> all. If thats the case, then I agree, its probably better to to >> take the additional visitation hit in in check_expire above than to >> try and preserve ordering. > > Yes, this seems best. > > I was worried that somehow the ordering also influences lookups, > because the TOS bits don't go into the hash so I worried that it would > be important that explicit TOS values appear before wildcard ones. > But it doesn't appear that this is an issue, we don't have wildcard > TOSs in the rtable entries, they are always explicit. > > So I would like to see an explicit final patch from Eric so we can get > this fixed now. > I would like to split patches because we have two bugs indeed, and I prefer to get attention for both problems, I dont remember Neil acknowledged the length computation problem. First and small patch, candidate for net-2.6 and stable (for 2.6.29) : Thank you [PATCH] net: fix length computation in rt_check_expire() rt_check_expire() computes average and standard deviation of chain lengths, but not correclty reset length to 0 at beginning of each chain. This probably gives overflows for sum2 (and sum) on loaded machines instead of meaningful results. Signed-off-by: Eric Dumazet Acked-by: Neil Horman --- net/ipv4/route.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) -- 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 diff --git a/net/ipv4/route.c b/net/ipv4/route.c index c4c60e9..869cf1c 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -785,7 +785,7 @@ static void rt_check_expire(void) static unsigned int rover; unsigned int i = rover, goal; struct rtable *rth, **rthp; - unsigned long length = 0, samples = 0; + unsigned long samples = 0; unsigned long sum = 0, sum2 = 0; u64 mult; @@ -795,9 +795,9 @@ static void rt_check_expire(void) goal = (unsigned int)mult; if (goal > rt_hash_mask) goal = rt_hash_mask + 1; - length = 0; for (; goal > 0; goal--) { unsigned long tmo = ip_rt_gc_timeout; + unsigned long length; i = (i + 1) & rt_hash_mask; rthp = &rt_hash_table[i].chain; @@ -809,6 +809,7 @@ static void rt_check_expire(void) if (*rthp == NULL) continue; + length = 0; spin_lock_bh(rt_hash_lock_addr(i)); while ((rth = *rthp) != NULL) { if (rt_is_expired(rth)) {