linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: lustre: Replace a bit shift by a use of BIT.
@ 2017-03-22  2:39 Arushi Singhal
  2017-03-22 12:12 ` Dilger, Andreas
  0 siblings, 1 reply; 3+ messages in thread
From: Arushi Singhal @ 2017-03-22  2:39 UTC (permalink / raw)
  To: oleg.drokin
  Cc: Andreas Dilger, James Simmons, Greg Kroah-Hartman, lustre-devel,
	devel, linux-kernel, outreachy-kernel

This patch replaces bit shifting on 1 with the BIT(x) macro.
This was done with coccinelle:
@@
constant c;
@@

-1 << c
+BIT(c)

Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
---
 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c           | 2 +-
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c        | 8 ++++----
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c | 2 +-
 drivers/staging/lustre/lnet/lnet/lib-ptl.c                    | 4 ++--
 drivers/staging/lustre/lnet/lnet/net_fault.c                  | 8 ++++----
 drivers/staging/lustre/lustre/llite/lproc_llite.c             | 4 ++--
 drivers/staging/lustre/lustre/obdclass/lustre_handles.c       | 2 +-
 drivers/staging/lustre/lustre/osc/osc_request.c               | 2 +-
 8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 79321e4aaf30..449f04f125b7 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -2241,7 +2241,7 @@ static int kiblnd_hdev_get_attr(struct kib_hca_dev *hdev)
 	 * matching that of the native system
 	 */
 	hdev->ibh_page_shift = PAGE_SHIFT;
-	hdev->ibh_page_size  = 1 << PAGE_SHIFT;
+	hdev->ibh_page_size  = BIT(PAGE_SHIFT);
 	hdev->ibh_page_mask  = ~((__u64)hdev->ibh_page_size - 1);
 
 	hdev->ibh_mr_size = hdev->ibh_ibdev->attrs.max_mr_size;
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
index eaa4399e6a2e..5bef8a7e053b 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
@@ -1906,14 +1906,14 @@ ksocknal_connect(struct ksock_route *route)
 		if (retry_later) /* needs reschedule */
 			break;
 
-		if (wanted & (1 << SOCKLND_CONN_ANY)) {
+		if (wanted & (BIT(SOCKLND_CONN_ANY))) {
 			type = SOCKLND_CONN_ANY;
-		} else if (wanted & (1 << SOCKLND_CONN_CONTROL)) {
+		} else if (wanted & (BIT(SOCKLND_CONN_CONTROL))) {
 			type = SOCKLND_CONN_CONTROL;
-		} else if (wanted & (1 << SOCKLND_CONN_BULK_IN)) {
+		} else if (wanted & (BIT(SOCKLND_CONN_BULK_IN))) {
 			type = SOCKLND_CONN_BULK_IN;
 		} else {
-			LASSERT(wanted & (1 << SOCKLND_CONN_BULK_OUT));
+			LASSERT(wanted & (BIT(SOCKLND_CONN_BULK_OUT)));
 			type = SOCKLND_CONN_BULK_OUT;
 		}
 
diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c
index fc7eec83ac07..2d1e3479cf7e 100644
--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c
+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c
@@ -71,7 +71,7 @@ static int typed_conns = 1;
 module_param(typed_conns, int, 0444);
 MODULE_PARM_DESC(typed_conns, "use different sockets for bulk");
 
-static int min_bulk = 1 << 10;
+static int min_bulk = BIT(10);
 module_param(min_bulk, int, 0644);
 MODULE_PARM_DESC(min_bulk, "smallest 'large' message");
 
diff --git a/drivers/staging/lustre/lnet/lnet/lib-ptl.c b/drivers/staging/lustre/lnet/lnet/lib-ptl.c
index 63cce0c4a065..b980c669705e 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-ptl.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-ptl.c
@@ -332,7 +332,7 @@ lnet_mt_test_exhausted(struct lnet_match_table *mtable, int pos)
 	LASSERT(pos <= LNET_MT_HASH_IGNORE);
 	/* mtable::mt_mhash[pos] is marked as exhausted or not */
 	bmap = &mtable->mt_exhausted[pos >> LNET_MT_BITS_U64];
-	pos &= (1 << LNET_MT_BITS_U64) - 1;
+	pos &= (BIT(LNET_MT_BITS_U64)) - 1;
 
 	return (*bmap & (1ULL << pos));
 }
@@ -347,7 +347,7 @@ lnet_mt_set_exhausted(struct lnet_match_table *mtable, int pos, int exhausted)
 
 	/* set mtable::mt_mhash[pos] as exhausted/non-exhausted */
 	bmap = &mtable->mt_exhausted[pos >> LNET_MT_BITS_U64];
-	pos &= (1 << LNET_MT_BITS_U64) - 1;
+	pos &= (BIT(LNET_MT_BITS_U64)) - 1;
 
 	if (!exhausted)
 		*bmap &= ~(1ULL << pos);
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));
 
 	mutex_init(&delay_dd.dd_mutex);
 	spin_lock_init(&delay_dd.dd_lock);
diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
index 40f1fcf8b5c0..366833dd8aa3 100644
--- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
+++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
@@ -1308,7 +1308,7 @@ static void ll_display_extents_info(struct ll_rw_extents_info *io_extents,
 			   r, pct(r, read_tot), pct(read_cum, read_tot),
 			   w, pct(w, write_tot), pct(write_cum, write_tot));
 		start = end;
-		if (start == 1 << 10) {
+		if (start == BIT(10)) {
 			start = 1;
 			units += 10;
 			unitp++;
@@ -1506,7 +1506,7 @@ void ll_rw_stats_tally(struct ll_sb_info *sbi, pid_t pid,
 		lprocfs_oh_clear(&io_extents->pp_extents[cur].pp_w_hist);
 	}
 
-	for (i = 0; (count >= (1 << LL_HIST_START << i)) &&
+	for (i = 0; (count >= (BIT(LL_HIST_START) << i)) &&
 	     (i < (LL_HIST_MAX - 1)); i++)
 		;
 	if (rw == 0) {
diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
index c9445e5ec271..11e32b5174f2 100644
--- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
+++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
@@ -49,7 +49,7 @@ static struct handle_bucket {
 	struct list_head	head;
 } *handle_hash;
 
-#define HANDLE_HASH_SIZE (1 << 16)
+#define HANDLE_HASH_SIZE (BIT(16))
 #define HANDLE_HASH_MASK (HANDLE_HASH_SIZE - 1)
 
 /*
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index d8aa3fb468c7..8739abf639a7 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2847,7 +2847,7 @@ static int __init osc_init(void)
 	register_shrinker(&osc_cache_shrinker);
 
 	/* This is obviously too much memory, only prevent overflow here */
-	if (osc_reqpool_mem_max >= 1 << 12 || osc_reqpool_mem_max == 0) {
+	if (osc_reqpool_mem_max >= BIT(12) || osc_reqpool_mem_max == 0) {
 		rc = -EINVAL;
 		goto out_type;
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] staging: lustre: Replace a bit shift by a use of BIT.
  2017-03-22  2:39 [PATCH] staging: lustre: Replace a bit shift by a use of BIT Arushi Singhal
@ 2017-03-22 12:12 ` Dilger, Andreas
  2017-03-23 20:43   ` Dilger, Andreas
  0 siblings, 1 reply; 3+ messages in thread
From: Dilger, Andreas @ 2017-03-22 12:12 UTC (permalink / raw)
  To: Arushi Singhal
  Cc: Drokin, Oleg, James Simmons, Greg Kroah-Hartman, lustre-devel,
	devel, linux-kernel, outreachy-kernel

On Mar 21, 2017, at 22:39, Arushi Singhal <arushisinghal19971997@gmail.com> wrote:
> 
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
> 
> -1 << c
> +BIT(c)

Did you take the time to look at what this Coccinelle script actually did, to see
if it actually makes sense?  Some of the cases where this replacement was done
makes sense because a specific bit is being accessed (i.e. a bitmask).  Elsewhere,
it doesn't make sense to use BIT() to specify numeric values (e.g. PAGE_SHIFT,
1 << 12 = 1024, etc) since they are not bitmasks.

> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c           | 2 +-
> drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c        | 8 ++++----
> drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c | 2 +-
> drivers/staging/lustre/lnet/lnet/lib-ptl.c                    | 4 ++--
> drivers/staging/lustre/lnet/lnet/net_fault.c                  | 8 ++++----
> drivers/staging/lustre/lustre/llite/lproc_llite.c             | 4 ++--
> drivers/staging/lustre/lustre/obdclass/lustre_handles.c       | 2 +-
> drivers/staging/lustre/lustre/osc/osc_request.c               | 2 +-
> 8 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 79321e4aaf30..449f04f125b7 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -2241,7 +2241,7 @@ static int kiblnd_hdev_get_attr(struct kib_hca_dev *hdev)
> 	 * matching that of the native system
> 	 */
> 	hdev->ibh_page_shift = PAGE_SHIFT;
> -	hdev->ibh_page_size  = 1 << PAGE_SHIFT;
> +	hdev->ibh_page_size  = BIT(PAGE_SHIFT);

This should just be replaced with "PAGE_SIZE".

> 	hdev->ibh_page_mask  = ~((__u64)hdev->ibh_page_size - 1);
> 
> 	hdev->ibh_mr_size = hdev->ibh_ibdev->attrs.max_mr_size;
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> index eaa4399e6a2e..5bef8a7e053b 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
> @@ -1906,14 +1906,14 @@ ksocknal_connect(struct ksock_route *route)
> 		if (retry_later) /* needs reschedule */
> 			break;
> 
> -		if (wanted & (1 << SOCKLND_CONN_ANY)) {
> +		if (wanted & (BIT(SOCKLND_CONN_ANY))) {

There shouldn't be any need to put parenthesis around (BIT(x)) here, or
any of the cases below, or the BIT macro is broken.  But besides that, the
use of BIT() here looks OK.

> 			type = SOCKLND_CONN_ANY;
> -		} else if (wanted & (1 << SOCKLND_CONN_CONTROL)) {
> +		} else if (wanted & (BIT(SOCKLND_CONN_CONTROL))) {
> 			type = SOCKLND_CONN_CONTROL;
> -		} else if (wanted & (1 << SOCKLND_CONN_BULK_IN)) {
> +		} else if (wanted & (BIT(SOCKLND_CONN_BULK_IN))) {
> 			type = SOCKLND_CONN_BULK_IN;
> 		} else {
> -			LASSERT(wanted & (1 << SOCKLND_CONN_BULK_OUT));
> +			LASSERT(wanted & (BIT(SOCKLND_CONN_BULK_OUT)));
> 			type = SOCKLND_CONN_BULK_OUT;
> 		}
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c
> index fc7eec83ac07..2d1e3479cf7e 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_modparams.c
> @@ -71,7 +71,7 @@ static int typed_conns = 1;
> module_param(typed_conns, int, 0444);
> MODULE_PARM_DESC(typed_conns, "use different sockets for bulk");
> 
> -static int min_bulk = 1 << 10;
> +static int min_bulk = BIT(10);

This is a scalar value and not a bitmask.  It could just be "1024", but using
BIT() is not correct.

> module_param(min_bulk, int, 0644);
> MODULE_PARM_DESC(min_bulk, "smallest 'large' message");
> 
> diff --git a/drivers/staging/lustre/lnet/lnet/lib-ptl.c b/drivers/staging/lustre/lnet/lnet/lib-ptl.c
> index 63cce0c4a065..b980c669705e 100644
> --- a/drivers/staging/lustre/lnet/lnet/lib-ptl.c
> +++ b/drivers/staging/lustre/lnet/lnet/lib-ptl.c
> @@ -332,7 +332,7 @@ lnet_mt_test_exhausted(struct lnet_match_table *mtable, int pos)
> 	LASSERT(pos <= LNET_MT_HASH_IGNORE);
> 	/* mtable::mt_mhash[pos] is marked as exhausted or not */
> 	bmap = &mtable->mt_exhausted[pos >> LNET_MT_BITS_U64];
> -	pos &= (1 << LNET_MT_BITS_U64) - 1;
> +	pos &= (BIT(LNET_MT_BITS_U64)) - 1;

The use of BIT() above is probably not correct.  This is creating a mask to align
pos to a multiple of (1 << LNET_MT_BITS_U64), and IMHO BIT() shouldn't be used
just any time a value is being shifted but rather only when a one-bit mask is needed.

Conversely, the below usage could be converted to use the macro since this is
actually a bitmask:

       return *bmap & BIT(pos);

> 	return (*bmap & (1ULL << pos));
> }
> @@ -347,7 +347,7 @@ lnet_mt_set_exhausted(struct lnet_match_table *mtable, int pos, int exhausted)
> 
> 	/* set mtable::mt_mhash[pos] as exhausted/non-exhausted */
> 	bmap = &mtable->mt_exhausted[pos >> LNET_MT_BITS_U64];
> -	pos &= (1 << LNET_MT_BITS_U64) - 1;
> +	pos &= (BIT(LNET_MT_BITS_U64)) - 1;
> 
> 	if (!exhausted)
> 		*bmap &= ~(1ULL << pos);

Ditto.

> 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 

> diff --git a/drivers/staging/lustre/lustre/llite/lproc_llite.c b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> index 40f1fcf8b5c0..366833dd8aa3 100644
> --- a/drivers/staging/lustre/lustre/llite/lproc_llite.c
> +++ b/drivers/staging/lustre/lustre/llite/lproc_llite.c
> @@ -1308,7 +1308,7 @@ static void ll_display_extents_info(struct ll_rw_extents_info *io_extents,
> 			   r, pct(r, read_tot), pct(read_cum, read_tot),
> 			   w, pct(w, write_tot), pct(write_cum, write_tot));
> 		start = end;
> -		if (start == 1 << 10) {
> +		if (start == BIT(10)) {

This should just be 1024, or could be left as-is left as-is.

> 			start = 1;
> 			units += 10;
> 			unitp++;
> @@ -1506,7 +1506,7 @@ void ll_rw_stats_tally(struct ll_sb_info *sbi, pid_t pid,
> 		lprocfs_oh_clear(&io_extents->pp_extents[cur].pp_w_hist);
> 	}
> 
> -	for (i = 0; (count >= (1 << LL_HIST_START << i)) &&
> +	for (i = 0; (count >= (BIT(LL_HIST_START) << i)) &&

This is also not a bitmask, but rather a scalar value.

> 	     (i < (LL_HIST_MAX - 1)); i++)
> 		;
> 	if (rw == 0) {
> diff --git a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> index c9445e5ec271..11e32b5174f2 100644
> --- a/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> +++ b/drivers/staging/lustre/lustre/obdclass/lustre_handles.c
> @@ -49,7 +49,7 @@ static struct handle_bucket {
> 	struct list_head	head;
> } *handle_hash;
> 
> -#define HANDLE_HASH_SIZE (1 << 16)
> +#define HANDLE_HASH_SIZE (BIT(16))

Also not a bitmask, but a scalar value.

> #define HANDLE_HASH_MASK (HANDLE_HASH_SIZE - 1)
> 
> /*
> diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
> index d8aa3fb468c7..8739abf639a7 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_request.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_request.c
> @@ -2847,7 +2847,7 @@ static int __init osc_init(void)
> 	register_shrinker(&osc_cache_shrinker);
> 
> 	/* This is obviously too much memory, only prevent overflow here */
> -	if (osc_reqpool_mem_max >= 1 << 12 || osc_reqpool_mem_max == 0) {
> +	if (osc_reqpool_mem_max >= BIT(12) || osc_reqpool_mem_max == 0) {

Also a scalar value.  Could be 4096.


Cheers, Andreas

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] staging: lustre: Replace a bit shift by a use of BIT.
  2017-03-22 12:12 ` Dilger, Andreas
@ 2017-03-23 20:43   ` Dilger, Andreas
  0 siblings, 0 replies; 3+ messages in thread
From: Dilger, Andreas @ 2017-03-23 20:43 UTC (permalink / raw)
  To: Arushi Singhal
  Cc: Drokin, Oleg, James Simmons, Greg Kroah-Hartman, lustre-devel,
	devel, linux-kernel, outreachy-kernel

On Mar 22, 2017, at 06:12, Dilger, Andreas <andreas.dilger@intel.com> wrote:
> 
> On Mar 21, 2017, at 22:39, Arushi Singhal <arushisinghal19971997@gmail.com> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-23 20:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22  2:39 [PATCH] staging: lustre: Replace a bit shift by a use of BIT Arushi Singhal
2017-03-22 12:12 ` Dilger, Andreas
2017-03-23 20:43   ` Dilger, Andreas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).