From patchwork Thu Nov 4 11:21:39 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 70122 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 50F73B7043 for ; Thu, 4 Nov 2010 22:21:58 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755544Ab0KDLVp (ORCPT ); Thu, 4 Nov 2010 07:21:45 -0400 Received: from mail-wy0-f174.google.com ([74.125.82.174]:48211 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755185Ab0KDLVo (ORCPT ); Thu, 4 Nov 2010 07:21:44 -0400 Received: by wyf28 with SMTP id 28so1805705wyf.19 for ; Thu, 04 Nov 2010 04:21:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=/iOXvTeZTlbmQ8mE6rq8MEtsA3GMEbMY3OvbyoJgbiY=; b=NPzf436qo32tbQtCKTCVQP7HrhV4JidTcw1x5KXpa+UKcYDxyy2YrD1zwWzJmuyvMv xUSis8Upw57ZeSGIBDUW3qZJimrtqg7ond4L1RPGSTjmbb2KFsFab3+JStX89lXG2IP2 uffUEj47mJFRipMBtHfHrg+SvLdGPj6LvV5Og= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=WR1m7YcUvI5NNEjPbdEdcTEypi1Ij3jAJX9w21XPtuDVRm4VsDn3yx41RJfDSTxQUI obgKTbrgeizbZeRwianJssBksxcDMCNM+xM1Xk7M7b70hbhl2G2PiXgHZX0BpY6rVLVD eZ3DknYga7IDNmrAAz47ECNVxg+c/IVFMqpZs= Received: by 10.227.136.143 with SMTP id r15mr572577wbt.151.1288869701837; Thu, 04 Nov 2010 04:21:41 -0700 (PDT) Received: from [10.150.51.210] (gw0.net.jmsp.net [212.23.165.14]) by mx.google.com with ESMTPS id a17sm8453666wbe.0.2010.11.04.04.21.40 (version=SSLv3 cipher=RC4-MD5); Thu, 04 Nov 2010 04:21:41 -0700 (PDT) Subject: Re: Freeing alive fib_info caused by ebc0ffae5 From: Eric Dumazet To: michael@ellerman.id.au Cc: netdev@vger.kernel.org In-Reply-To: <1288866626.2659.71.camel@edumazet-laptop> References: <1288866186.30549.10.camel@concordia> <1288866626.2659.71.camel@edumazet-laptop> Date: Thu, 04 Nov 2010 12:21:39 +0100 Message-ID: <1288869699.2659.77.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le jeudi 04 novembre 2010 à 11:30 +0100, Eric Dumazet a écrit : > Le jeudi 04 novembre 2010 à 21:23 +1100, Michael Ellerman a écrit : > > Hi all, > > > > I'm running Linus' latest or thereabouts (ff8b16d), and I'm seeing > > "Freeing alive fib_info" messages, from free_fib_info(). > > > > Actually I only get one per boot, when network interfaces come up. > > Seemingly related I am getting refcount problems when I shutdown, ie. > > unregister_netdevice() sees a usage count of 1, which never decrements. > > > > Bisect says it's ebc0ffae5 which causes the problem, or makes it appear. > > > > fib: RCU conversion of fib_lookup() > > > > fib_lookup() converted to be called in RCU protected context, no > > reference taken and released on a contended cache line (fib_clntref) > > > > > > Is this a bug in that commit, or a driver bug exposed? > > Hi Michael, thanks for the report (and painful bisection I guess) > > Thats hard to say... Is it reproductable on my machine ? > Hmm, a review of the code spotted a bug in fib_result_assign() Please try following patch : Thanks again ! [PATCH] fib: fib_result_assign() should not change fib refcounts After commit ebc0ffae5 (RCU conversion of fib_lookup()), fib_result_assign() should not change fib refcounts anymore. Thanks to Michael who did the bisection and bug report. Reported-by: Michael Ellerman Signed-off-by: Eric Dumazet --- net/ipv4/fib_lookup.h | 5 +---- 1 file changed, 1 insertion(+), 4 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/fib_lookup.h b/net/ipv4/fib_lookup.h index a29edf2..c079cc0 100644 --- a/net/ipv4/fib_lookup.h +++ b/net/ipv4/fib_lookup.h @@ -47,11 +47,8 @@ extern int fib_detect_death(struct fib_info *fi, int order, static inline void fib_result_assign(struct fib_result *res, struct fib_info *fi) { - if (res->fi != NULL) - fib_info_put(res->fi); + /* we used to play games with refcounts, but we now use RCU */ res->fi = fi; - if (fi != NULL) - atomic_inc(&fi->fib_clntref); } #endif /* _FIB_LOOKUP_H */