Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756528AbdCWUp0 (ORCPT ); Thu, 23 Mar 2017 16:45:26 -0400 Received: from mga02.intel.com ([134.134.136.20]:59986 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751390AbdCWUpX (ORCPT ); Thu, 23 Mar 2017 16:45:23 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,211,1486454400"; d="scan'208";a="70119016" From: "Dilger, Andreas" To: Arushi Singhal CC: "Drokin, Oleg" , James Simmons , Greg Kroah-Hartman , "lustre-devel@lists.lustre.org" , "devel@driverdev.osuosl.org" , "linux-kernel@vger.kernel.org" , "outreachy-kernel@googlegroups.com" Subject: Re: [PATCH] staging: lustre: Replace a bit shift by a use of BIT. Thread-Topic: [PATCH] staging: lustre: Replace a bit shift by a use of BIT. Thread-Index: AQHSorWUJXYM4QRhU0W8UJ3XDL8FaKGhOv0AgAIhUQA= Date: Thu, 23 Mar 2017 20:43:59 +0000 Message-ID: References: <20170322023933.GA11339@arushi-HP-Pavilion-Notebook> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.66.79] Content-Type: text/plain; charset="us-ascii" Content-ID: <6E6772234CB189468F4F41A343F12FA2@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2NKjZit025357 Content-Length: 1720 Lines: 49 On Mar 22, 2017, at 06:12, Dilger, Andreas wrote: > > On Mar 21, 2017, at 22:39, Arushi Singhal wrote: >> >> This patch replaces bit shifting on 1 with the BIT(x) macro. >> This was done with coccinelle: [snip] >> diff --git a/drivers/staging/lustre/lnet/lnet/net_fault.c b/drivers/staging/lustre/lnet/lnet/net_fault.c >> index 18183cbb9859..e83761a77d22 100644 >> --- a/drivers/staging/lustre/lnet/lnet/net_fault.c >> +++ b/drivers/staging/lustre/lnet/lnet/net_fault.c >> @@ -997,10 +997,10 @@ lnet_fault_ctl(int opc, struct libcfs_ioctl_data *data) >> int >> lnet_fault_init(void) >> { >> - BUILD_BUG_ON(LNET_PUT_BIT != 1 << LNET_MSG_PUT); >> - BUILD_BUG_ON(LNET_ACK_BIT != 1 << LNET_MSG_ACK); >> - BUILD_BUG_ON(LNET_GET_BIT != 1 << LNET_MSG_GET); >> - BUILD_BUG_ON(LNET_REPLY_BIT != 1 << LNET_MSG_REPLY); >> + BUILD_BUG_ON(LNET_PUT_BIT != BIT(LNET_MSG_PUT)); >> + BUILD_BUG_ON(LNET_ACK_BIT != BIT(LNET_MSG_ACK)); >> + BUILD_BUG_ON(LNET_GET_BIT != BIT(LNET_MSG_GET)); >> + BUILD_BUG_ON(LNET_REPLY_BIT != BIT(LNET_MSG_REPLY)); > > This looks reasonable at first glance, though on further thought it seems kind of > pointless since this is really: > > #defined LET_PUT_BIT BIT(LNET_MSG_PUT) > > BUILD_BUG_ON(BIT(LNET_MSG_PUT) != BIT(LNET_MSG_PUT)) > > so it is just checking that the macro's value is the same when called two times. > I'd suggest just getting rid of these BUILD_BUG_ON() checks completely . Arushi, it would be great if you could submit a patch to remove the above BUILD_BUG_ON() lines completely. I don't think they have any value anymore. Cheers, Andreas -- Andreas Dilger Lustre Principal Architect Intel Corporation