Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755406AbbFBDhb (ORCPT ); Mon, 1 Jun 2015 23:37:31 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:34570 "EHLO mail-ob0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754353AbbFBDhW convert rfc822-to-8bit (ORCPT ); Mon, 1 Jun 2015 23:37:22 -0400 MIME-Version: 1.0 In-Reply-To: <556D16F4.3060307@miraclelinux.com> References: <1433188983-23207-1-git-send-email-xerofoify@gmail.com> <556D16F4.3060307@miraclelinux.com> Date: Mon, 1 Jun 2015 20:37:21 -0700 Message-ID: Subject: Re: [PATCH] neighbour: Convert if statment in the function, neigh_add_timer to a WARN_ON From: Cong Wang To: =?UTF-8?B?WU9TSElGVUpJIEhpZGVha2kv5ZCJ6Jek6Iux5piO?= Cc: Nicholas Krause , David Miller , "Eric W. Biederman" , Eric Dumazet , Hideaki YOSHIFUJI , johannes.berg@intel.com, Nicolas Dichtel , Linux Kernel Network Developers , LKML Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1537 Lines: 42 On Mon, Jun 1, 2015 at 7:37 PM, YOSHIFUJI Hideaki/吉藤英明 wrote: > Nicholas Krause wrote: >> This converts the if statement for dumping the stack into a >> WARN_ON in order to make this function's debugging check >> simpler and have a cleaner output when this condition >> occurs inside this function for when bugs related to >> adding a duplicate neighbour table timer arise. >> >> Signed-off-by: Nicholas Krause >> --- >> net/core/neighbour.c | 6 +----- >> 1 file changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/net/core/neighbour.c b/net/core/neighbour.c >> index 3de6542..0bf71da 100644 >> --- a/net/core/neighbour.c >> +++ b/net/core/neighbour.c >> @@ -165,11 +165,7 @@ static int neigh_forced_gc(struct neigh_table *tbl) >> static void neigh_add_timer(struct neighbour *n, unsigned long when) >> { >> neigh_hold(n); >> - if (unlikely(mod_timer(&n->timer, when))) { >> - printk("NEIGH: BUG, double timer add, state is %x\n", >> - n->nud_state); >> - dump_stack(); >> - } >> + WARN_ON(unlikely(mod_timer(&n->timer, when))); >> } > > NACK, please do not use WARN_ON for things with side effects. Just: int ret = mod_timer(...); WARN_ON(ret); ... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/