From patchwork Fri Sep 18 00:15:04 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 33810 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.176.167]) by ozlabs.org (Postfix) with ESMTP id 944D2B7B68 for ; Fri, 18 Sep 2009 10:15:53 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754820AbZIRAPL (ORCPT ); Thu, 17 Sep 2009 20:15:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754286AbZIRAPK (ORCPT ); Thu, 17 Sep 2009 20:15:10 -0400 Received: from smtp1.linux-foundation.org ([140.211.169.13]:39648 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754738AbZIRAPE (ORCPT ); Thu, 17 Sep 2009 20:15:04 -0400 Received: from imap1.linux-foundation.org (imap1.linux-foundation.org [140.211.169.55]) by smtp1.linux-foundation.org (8.14.2/8.13.5/Debian-3ubuntu1.1) with ESMTP id n8I0F4rR017556 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 17 Sep 2009 17:15:05 -0700 Received: from akpm.mtv.corp.google.com (localhost [127.0.0.1]) by imap1.linux-foundation.org (8.13.5.20060308/8.13.5/Debian-3ubuntu1.1) with SMTP id n8I0F4Df017411; Thu, 17 Sep 2009 17:15:04 -0700 Date: Thu, 17 Sep 2009 17:15:04 -0700 From: Andrew Morton To: David Miller Cc: JBeulich@novell.com, linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org Subject: Re: [PATCH] fix BUILD_BUG_ON() and a couple of bogus uses of it Message-Id: <20090917171504.649ba29b.akpm@linux-foundation.org> In-Reply-To: <20090917.170156.152594715.davem@davemloft.net> References: <4A8AEBFD0200007800010580@vpn.id2.novell.com> <20090917162437.b1a890de.akpm@linux-foundation.org> <20090917.170156.152594715.davem@davemloft.net> X-Mailer: Sylpheed version 2.2.4 (GTK+ 2.8.20; i486-pc-linux-gnu) Mime-Version: 1.0 X-Spam-Status: No, hits=-5.013 required=5 tests=AWL, BAYES_00, OSDL_HEADER_SUBJECT_BRACKETED, PATCH_SUBJECT_OSDL X-Spam-Checker-Version: SpamAssassin 3.2.4-osdl_revision__1.47__ X-MIMEDefang-Filter: lf$Revision: 1.188 $ X-Scanned-By: MIMEDefang 2.63 on 140.211.169.13 Sender: sparclinux-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: sparclinux@vger.kernel.org On Thu, 17 Sep 2009 17:01:56 -0700 (PDT) David Miller wrote: > From: Andrew Morton > Date: Thu, 17 Sep 2009 16:24:37 -0700 > > > sparc64: > > > > In file included from arch/sparc/kernel/vio.c:17: > > /usr/src/devel/arch/sparc/include/asm/vio.h: In function `vio_dring_avail': > > /usr/src/devel/arch/sparc/include/asm/vio.h:261: error: bit-field `' width not an integer constant > > > > static inline u32 vio_dring_avail(struct vio_dring_state *dr, > > unsigned int ring_size) > > { > > BUILD_BUG_ON(!is_power_of_2(ring_size)); > > > > return (dr->pending - > > ((dr->prod - dr->cons) & (ring_size - 1))); > > } > > > > changing it to MAYBE_BUILD_BUG_ON seems to have fixed it. > > That's completely bogus. > > First of all, arch/sparc/kernel/vio.c never calls this function > so it should never be evaluated. > > Second of all, all the places that do call this function only > pass pure constants as the 'ring_size' parameter. > > drivers/block/sunvdc.c: > > static inline u32 vdc_tx_dring_avail(struct vio_dring_state *dr) > { > return vio_dring_avail(dr, VDC_TX_RING_SIZE); > } > > drivers/net/sunvnet.c: > > static inline u32 vnet_tx_dring_avail(struct vio_dring_state *dr) > { > return vio_dring_avail(dr, VNET_TX_RING_SIZE); > } > > Making this MAYBE_BUILD_BUG_ON shouldn't be necessary. There's a shortcoming in the current BUILD_BUG_ON() - it silently does nothing if passed a non-constant arg. I suspect that in the 2.6.31 code, that BUILD_BUG_ON() just does nothing at all, and that Jan's patch is now exposing this. It might be compiler-version dependent too. Yup, on base 2.6.31, this: --- a/arch/sparc/kernel/vio.c~a +++ a/arch/sparc/kernel/vio.c @@ -23,6 +23,8 @@ static const struct vio_device_id *vio_m const char *type, *compat; int len; + vio_dring_avail(NULL, 33); + type = dev->type; compat = dev->compat; len = dev->compat_len;