Skip to contentSkip to conversation

Remove uses and definition of IS_PO2

PATCH Superseded Aya Mahfouz 6 participants 20 messages Oct 16, 2015 – Oct 18, 2015 Staging

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover Remove uses and definition of IS_PO2

Concerned with the removal of IS_PO2 by replacing its uses with is_power_of_2 and then removing the definition

Aya Mahfouz (4):
staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2
staging: lustre: hash.c: replace IS_PO2 by is_power_of_2
staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2
staging: lustre: remove IS_PO2 and __is_po2

 drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c     | 5 ++++-
 drivers/staging/lustre/lustre/libcfs/hash.c          | 4 +++-
 drivers/staging/lustre/lustre/libcfs/workitem.c      | 4 +++-
 4 files changed, 10 insertions(+), 10 deletions(-)
Signature · 6 lines
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz
#1 lore raw

PATCH 1/4 staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is LASSERT. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/ldlm/ldlm_extent.c +4 -1
index 57b997d..3e7e97e 100644
@@ -50,6 +50,9 @@
  */
 
 #define DEBUG_SUBSYSTEM S_LDLM
+
+#include <linux/log2.h>
+
 #include "../../include/linux/libcfs/libcfs.h"
 #include "../include/lustre_dlm.h"
 #include "../include/obd_support.h"
@@ -149,7 +152,7 @@ static inline int lock_mode_to_index(ldlm_mode_t mode)
 	int index;
 
 	LASSERT(mode != 0);
-	LASSERT(IS_PO2(mode));
+	LASSERT(is_power_of_2(mode));
 	for (index = -1; mode; index++)
 		mode >>= 1;
 	LASSERT(index < LCK_MODE_NUM);
Signature · 6 lines
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz
maintainer
#2 lore raw

PATCH 1/4 staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2

Quoted 7 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is LASSERT. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

All of this info about the types mean nothing for this patch :(

#3 lore raw

PATCH 1/4 staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2

Quoted 11 lines · Greg KH
Quoted 7 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is LASSERT. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

All of this info about the types mean nothing for this patch :(

Hmm, my problem is that I realized the difference of return types between the macro IS_PO2 and is_power_of_2. I tested the bool datatype with a user space program to see what I will get and I get 0s and 1s. I can do it with a simple kernel driver to see what happens there too if you would like.

Kindly let me know what kind of information you would like to see in the commit message.

Signature · 2 lines
Kind Regards,
Aya Saif El-yazal Mahfouz
#4 lore raw

PATCH 2/4 staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

In reply to Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/hash.c +3 -1
index 6f4c7d4..4b5e79a 100644
@@ -109,6 +109,8 @@
 
 #include "../../include/linux/libcfs/libcfs.h"
 #include <linux/seq_file.h>
+#include <linux/log2.h>
+
 
 #if CFS_HASH_DEBUG_LEVEL >= CFS_HASH_DEBUG_1
 static unsigned int warn_on_depth = 8;
@@ -1785,7 +1787,7 @@ cfs_hash_rehash_cancel_locked(struct cfs_hash *hs)
 	for (i = 2; cfs_hash_is_rehashing(hs); i++) {
 		cfs_hash_unlock(hs, 1);
 		/* raise console warning while waiting too long */
-		CDEBUG(IS_PO2(i >> 3) ? D_WARNING : D_INFO,
+		CDEBUG(is_power_of_2(i >> 3) ? D_WARNING : D_INFO,
 		       "hash %s is still rehashing, rescheded %d\n",
 		       hs->hs_name, i - 1);
 		cond_resched();
Signature · 6 lines
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz
maintainer
#5 lore raw

PATCH 2/4 staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

Quoted 27 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/hash.c +2 -0
index 6f4c7d4..4b5e79a 100644
@@ -109,6 +109,8 @@
 
 #include "../../include/linux/libcfs/libcfs.h"
 #include <linux/seq_file.h>
+#include <linux/log2.h>
+
 
 #if CFS_HASH_DEBUG_LEVEL >= CFS_HASH_DEBUG_1
 static unsigned int warn_on_depth = 8;

drivers/staging/lustre/lustre/libcfs/hash.c cfs_hash_rehash_cancel_locked(struct cfs_hash *hs) · line 1790

Quoted 5 lines
drivers/staging/lustre/lustre/libcfs/hash.c +1 -1
@@ -1785,7 +1787,7 @@ cfs_hash_rehash_cancel_locked(struct cfs_hash *hs)
 	for (i = 2; cfs_hash_is_rehashing(hs); i++) {
 		cfs_hash_unlock(hs, 1);
 		/* raise console warning while waiting too long */
-		CDEBUG(IS_PO2(i >> 3) ? D_WARNING : D_INFO,
+		CDEBUG(is_power_of_2(i >> 3) ? D_WARNING : D_INFO,

is_power_of_2() works differently than IS_PO2(), are you _sure_ that the value here can not be 0? If so, this will do something different :(

thanks,

greg k-h

#6 lore raw

PATCH 2/4 staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

Quoted 25 lines · Greg KH
Quoted 24 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/hash.c +2 -0
index 6f4c7d4..4b5e79a 100644
@@ -109,6 +109,8 @@
 
 #include "../../include/linux/libcfs/libcfs.h"
 #include <linux/seq_file.h>
+#include <linux/log2.h>
+
 
Quoted 12 lines

drivers/staging/lustre/lustre/libcfs/hash.c cfs_hash_rehash_cancel_locked(struct cfs_hash *hs) · line 1790

Quoted 8 lines
drivers/staging/lustre/lustre/libcfs/hash.c +1 -1
 #if CFS_HASH_DEBUG_LEVEL >= CFS_HASH_DEBUG_1
 static unsigned int warn_on_depth = 8;
@@ -1785,7 +1787,7 @@ cfs_hash_rehash_cancel_locked(struct cfs_hash *hs)
 	for (i = 2; cfs_hash_is_rehashing(hs); i++) {
 		cfs_hash_unlock(hs, 1);
 		/* raise console warning while waiting too long */
-		CDEBUG(IS_PO2(i >> 3) ? D_WARNING : D_INFO,
+		CDEBUG(is_power_of_2(i >> 3) ? D_WARNING : D_INFO,

is_power_of_2() works differently than IS_PO2(), are you _sure_ that the value here can not be 0? If so, this will do something different :(

This is actually an interesting point to raise. __is_po2 the inline function used by IS_PO2 should actually check if the number is greater than 0. The current implementation of __is_po2 would allow the comparison of 0 with 2^(size of unsigned long long)-1. Is this correct? Or is this something intentional?

Quoted 3 lines

thanks,

greg k-h

Signature · 2 lines
Kind Regards,
Aya Saif El-yazal Mahfouz
#7 lore raw

PATCH 2/4 staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

Quoted 39 lines · Aya Mahfouz
Quoted 25 lines · Greg KH
Quoted 24 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/hash.c +2 -0
index 6f4c7d4..4b5e79a 100644
@@ -109,6 +109,8 @@
 
 #include "../../include/linux/libcfs/libcfs.h"
 #include <linux/seq_file.h>
+#include <linux/log2.h>
+
 
Quoted 12 lines

drivers/staging/lustre/lustre/libcfs/hash.c cfs_hash_rehash_cancel_locked(struct cfs_hash *hs) · line 1790

Quoted 8 lines
drivers/staging/lustre/lustre/libcfs/hash.c +1 -1
 #if CFS_HASH_DEBUG_LEVEL >= CFS_HASH_DEBUG_1
 static unsigned int warn_on_depth = 8;
@@ -1785,7 +1787,7 @@ cfs_hash_rehash_cancel_locked(struct cfs_hash *hs)
 	for (i = 2; cfs_hash_is_rehashing(hs); i++) {
 		cfs_hash_unlock(hs, 1);
 		/* raise console warning while waiting too long */
-		CDEBUG(IS_PO2(i >> 3) ? D_WARNING : D_INFO,
+		CDEBUG(is_power_of_2(i >> 3) ? D_WARNING : D_INFO,

is_power_of_2() works differently than IS_PO2(), are you _sure_ that the value here can not be 0? If so, this will do something different :(

Quoted 5 lines

This is actually an interesting point to raise. __is_po2 the inline function used by IS_PO2 should actually check if the number is greater than 0. The current implementation of __is_po2 would allow the comparison of 0 with 2^(size of unsigned long long)-1. Is this correct? Or is this something intentional?

What is the actual result in each case?

julia

#8 lore raw

PATCH 2/4 staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

Quoted 41 lines · Julia Lawall
Quoted 39 lines · Aya Mahfouz
Quoted 37 lines · Greg KH
Quoted 27 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/hash.c +2 -0
index 6f4c7d4..4b5e79a 100644
@@ -109,6 +109,8 @@
 
 #include "../../include/linux/libcfs/libcfs.h"
 #include <linux/seq_file.h>
+#include <linux/log2.h>
+
 
 #if CFS_HASH_DEBUG_LEVEL >= CFS_HASH_DEBUG_1
 static unsigned int warn_on_depth = 8;

drivers/staging/lustre/lustre/libcfs/hash.c cfs_hash_rehash_cancel_locked(struct cfs_hash *hs) · line 1790

Quoted 5 lines
drivers/staging/lustre/lustre/libcfs/hash.c +1 -1
@@ -1785,7 +1787,7 @@ cfs_hash_rehash_cancel_locked(struct cfs_hash *hs)
 	for (i = 2; cfs_hash_is_rehashing(hs); i++) {
 		cfs_hash_unlock(hs, 1);
 		/* raise console warning while waiting too long */
-		CDEBUG(IS_PO2(i >> 3) ? D_WARNING : D_INFO,
+		CDEBUG(is_power_of_2(i >> 3) ? D_WARNING : D_INFO,

is_power_of_2() works differently than IS_PO2(), are you _sure_ that the value here can not be 0? If so, this will do something different :(

Quoted 8 lines
Quoted 5 lines

This is actually an interesting point to raise. __is_po2 the inline function used by IS_PO2 should actually check if the number is greater than 0. The current implementation of __is_po2 would allow the comparison of 0 with 2^(size of unsigned long long)-1. Is this correct? Or is this something intentional?

What is the actual result in each case?

for __is_po2, if the number is 0 or power of 2 i.e. 1,2,4,8,16,32 etc then return 1, 0 otherwise
for is_power_of_2 if the number is power of 2 return 1, 0 otherwise

Quoted 1 line

julia

Signature · 2 lines
Kind Regards,
Aya Saif El-yazal Mahfouz
#9 lore raw

PATCH 2/4 staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

Quoted 49 lines · Aya Mahfouz
Quoted 49 lines · Julia Lawall
Quoted 46 lines · Aya Mahfouz
Quoted 27 lines · Greg KH
Quoted 25 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/hash.c +2 -0
index 6f4c7d4..4b5e79a 100644
@@ -109,6 +109,8 @@
 
 #include "../../include/linux/libcfs/libcfs.h"
 #include <linux/seq_file.h>
+#include <linux/log2.h>
+
 
Quoted 12 lines

drivers/staging/lustre/lustre/libcfs/hash.c cfs_hash_rehash_cancel_locked(struct cfs_hash *hs) · line 1790

Quoted 8 lines
drivers/staging/lustre/lustre/libcfs/hash.c +1 -1
 #if CFS_HASH_DEBUG_LEVEL >= CFS_HASH_DEBUG_1
 static unsigned int warn_on_depth = 8;
@@ -1785,7 +1787,7 @@ cfs_hash_rehash_cancel_locked(struct cfs_hash *hs)
   for (i = 2; cfs_hash_is_rehashing(hs); i++) {
       cfs_hash_unlock(hs, 1);
       /* raise console warning while waiting too long */
-        CDEBUG(IS_PO2(i >> 3) ? D_WARNING : D_INFO,
+        CDEBUG(is_power_of_2(i >> 3) ? D_WARNING : D_INFO,

is_power_of_2() works differently than IS_PO2(), are you _sure_ that the value here can not be 0? If so, this will do something different :(

This is actually an interesting point to raise. __is_po2 the inline function used by IS_PO2 should actually check if the number is greater than 0. The current implementation of __is_po2 would allow the comparison of 0 with 2^(size of unsigned long long)-1. Is this correct? Or is this something intentional?

Quoted 6 lines
Quoted 2 lines

What is the actual result in each case?

for __is_po2, if the number is 0 or power of 2 i.e. 1,2,4,8,16,32 etc then return 1, 0 otherwise
for is_power_of_2 if the number is power of 2 return 1, 0 otherwise

It looks to me that the new behavior is actually more correct than the old one. The message shouldn't print onto the console until it has been tried several times, which is the
case with is_power_of_2().

This hasn't been noticed in the past since it only happens if the hash table is still being resized at the same time it is being destroyed.

In summary, I think the existing patch is fine. You can add:

Cheers, Andreas

#10 lore raw

PATCH 3/4 staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2

In reply to Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/workitem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/workitem.c +3 -1
index e1143a5..377e1ea 100644
@@ -41,6 +41,8 @@
 
 #define DEBUG_SUBSYSTEM S_LNET
 
+#include <linux/log2.h>
+
 #include "../../include/linux/libcfs/libcfs.h"
 
 #define CFS_WS_NAME_LEN	 16
@@ -325,7 +327,7 @@ cfs_wi_sched_destroy(struct cfs_wi_sched *sched)
 
 	spin_lock(&cfs_wi_data.wi_glock);
 	while (sched->ws_nthreads > 0) {
-		CDEBUG(IS_PO2(++i) ? D_WARNING : D_NET,
+		CDEBUG(is_power_of_2(++i) ? D_WARNING : D_NET,
 		       "waiting for %d threads of WI sched[%s] to terminate\n",
 		       sched->ws_nthreads, sched->ws_name);
 
Signature · 6 lines
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz
maintainer
#11 lore raw

PATCH 3/4 staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2

Quoted 27 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
 drivers/staging/lustre/lustre/libcfs/workitem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/workitem.c +2 -0
index e1143a5..377e1ea 100644
@@ -41,6 +41,8 @@
 
 #define DEBUG_SUBSYSTEM S_LNET
 
+#include <linux/log2.h>
+
 #include "../../include/linux/libcfs/libcfs.h"
 
 #define CFS_WS_NAME_LEN	 16

drivers/staging/lustre/lustre/libcfs/workitem.c cfs_wi_sched_destroy(struct cfs_wi_sched *sched) · line 330

Quoted 5 lines
drivers/staging/lustre/lustre/libcfs/workitem.c +1 -1
@@ -325,7 +327,7 @@ cfs_wi_sched_destroy(struct cfs_wi_sched *sched)
 
 	spin_lock(&cfs_wi_data.wi_glock);
 	while (sched->ws_nthreads > 0) {
-		CDEBUG(IS_PO2(++i) ? D_WARNING : D_NET,
+		CDEBUG(is_power_of_2(++i) ? D_WARNING : D_NET,

Same question about 0 here as well.

thanks,

greg k-h

#12 lore raw

PATCH 3/4 staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2

Quoted 30 lines · Greg KH
Quoted 30 lines · Aya Mahfouz

Replaces IS_PO2 by is_power_of_2. IS_PO2 is used with several debug macros. In this case, it is CDEBUG. Note that the replacement changes the types involved, because the parameter of IS_PO2 is of type long long and the return type is int, while the parameter of is_power_of_2 is of type long and the return type is bool. This, however, has no impact, because the actual argument is always of type int, and the return value is always used as a boolean.

---
drivers/staging/lustre/lustre/libcfs/workitem.c | 4 +++-

1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/lustre/lustre/libcfs/workitem.c +2 -0
index e1143a5..377e1ea 100644
@@ -41,6 +41,8 @@
 
 #define DEBUG_SUBSYSTEM S_LNET
 
+#include <linux/log2.h>
+
 #include "../../include/linux/libcfs/libcfs.h"
 
 #define CFS_WS_NAME_LEN     16
@@ -325,7 +327,7 @@ cfs_wi_sched_destroy(struct cfs_wi_sched *sched)
 
Quoted 6 lines

drivers/staging/lustre/lustre/libcfs/workitem.c cfs_wi_sched_destroy(struct cfs_wi_sched *sched) · line 330

Quoted 4 lines
drivers/staging/lustre/lustre/libcfs/workitem.c +1 -1
   spin_lock(&cfs_wi_data.wi_glock);
   while (sched->ws_nthreads > 0) {
-        CDEBUG(IS_PO2(++i) ? D_WARNING : D_NET,
+        CDEBUG(is_power_of_2(++i) ? D_WARNING : D_NET,

Same question about 0 here as well.

The initial value of "i" is 2 so the distinction doesn't make a difference in this case. You can add:

#13 lore raw

PATCH 4/4 staging: lustre: remove IS_PO2 and __is_po2

In reply to Aya Mahfouz

Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have been replaced by is_power_of_2

---
 drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
 1 file changed, 7 deletions(-)
drivers/staging/lustre/include/linux/libcfs/libcfs.h +0 -7
index 385ced1..c78a147 100644
@@ -42,13 +42,6 @@
 
 #include "curproc.h"
 
-static inline int __is_po2(unsigned long long val)
-{
-	return !(val & (val - 1));
-}
-
-#define IS_PO2(val) __is_po2((unsigned long long)(val))
-
 #define LOWEST_BIT_SET(x)       ((x) & ~((x) - 1))
 
 /*
Signature · 6 lines
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz
#14 lore raw

[lustre-devel] [PATCH 4/4] staging: lustre: remove IS_PO2 and __is_po2

reproduce:

Hi Aya,

[auto build test ERROR on staging/staging-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Aya-Mahfouz/Remove-uses-and-definition-of-IS_PO2/20151017-060923
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:40:0,
from drivers/staging/lustre/lnet/selftest/conrpc.c:43:
drivers/staging/lustre/lnet/selftest/conrpc.c: In function 'lstcon_rpc_cleanup_wait':

>> drivers/staging/lustre/lnet/selftest/selftest.h:588:10: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
             ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/linux/libcfs.h:109:25: note: in definition of macro '__CHECK_STACK'
      (msgdata)->msg_mask = mask;        \
                            ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:214:2: note: in expansion of macro 'CFS_CHECK_STACK'
     CFS_CHECK_STACK(&msgdata, mask, cdls);    \
     ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
>> drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
>> drivers/staging/lustre/lnet/selftest/conrpc.c:1359:2: note: in expansion of macro 'lst_wait_until'
     lst_wait_until((atomic_read(&console_session.ses_rpc_counter) == 0),
     ^

cc1: some warnings being treated as errors
--
In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:40:0,
from drivers/staging/lustre/lnet/selftest/selftest.h:46,
from drivers/staging/lustre/lnet/selftest/framework.c:44:
drivers/staging/lustre/lnet/selftest/framework.c: In function 'sfw_shutdown':

>> drivers/staging/lustre/lnet/selftest/selftest.h:588:10: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
             ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/linux/libcfs.h:109:25: note: in definition of macro '__CHECK_STACK'
      (msgdata)->msg_mask = mask;        \
                            ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:214:2: note: in expansion of macro 'CFS_CHECK_STACK'
     CFS_CHECK_STACK(&msgdata, mask, cdls);    \
     ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
>> drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
>> drivers/staging/lustre/lnet/selftest/framework.c:1741:2: note: in expansion of macro 'lst_wait_until'
     lst_wait_until(sfw_data.fw_active_srpc == NULL, sfw_data.fw_lock,
     ^

cc1: some warnings being treated as errors
--
In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:40:0,
from drivers/staging/lustre/lnet/selftest/selftest.h:46,
from drivers/staging/lustre/lnet/selftest/timer.c:43:
drivers/staging/lustre/lnet/selftest/timer.c: In function 'stt_shutdown':

>> drivers/staging/lustre/lnet/selftest/selftest.h:588:10: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
             ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/linux/libcfs.h:109:25: note: in definition of macro '__CHECK_STACK'
      (msgdata)->msg_mask = mask;        \
                            ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:214:2: note: in expansion of macro 'CFS_CHECK_STACK'
     CFS_CHECK_STACK(&msgdata, mask, cdls);    \
     ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
>> drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
>> drivers/staging/lustre/lnet/selftest/timer.c:240:2: note: in expansion of macro 'lst_wait_until'
     lst_wait_until(stt_data.stt_nthreads == 0, stt_data.stt_lock,
     ^

cc1: some warnings being treated as errors
--
In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:40:0,
from drivers/staging/lustre/lnet/selftest/selftest.h:46,
from drivers/staging/lustre/lnet/selftest/rpc.c:47:
drivers/staging/lustre/lnet/selftest/rpc.c: In function 'srpc_service_add_buffers':

>> drivers/staging/lustre/lnet/selftest/selftest.h:588:10: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
             ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/linux/libcfs.h:109:25: note: in definition of macro '__CHECK_STACK'
      (msgdata)->msg_mask = mask;        \
                            ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:214:2: note: in expansion of macro 'CFS_CHECK_STACK'
     CFS_CHECK_STACK(&msgdata, mask, cdls);    \
     ^
>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
>> drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
>> drivers/staging/lustre/lnet/selftest/rpc.c:619:3: note: in expansion of macro 'lst_wait_until'
      lst_wait_until(scd->scd_buf_err != 0 ||
      ^

cc1: some warnings being treated as errors

vim +/IS_PO2 +588 drivers/staging/lustre/lnet/selftest/selftest.h

d3caf4d58 Peng Tao 2014-03-18  582  	} while (0)
d7e09d039 Peng Tao 2013-05-02  583  
d7e09d039 Peng Tao 2013-05-02  584  #define lst_wait_until(cond, lock, fmt, ...)				\
d7e09d039 Peng Tao 2013-05-02  585  do {									\
d7e09d039 Peng Tao 2013-05-02  586  	int __I = 2;							\
d7e09d039 Peng Tao 2013-05-02  587  	while (!(cond)) {						\
d7e09d039 Peng Tao 2013-05-02 @588  		CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,		\
d7e09d039 Peng Tao 2013-05-02  589  		       fmt, ## __VA_ARGS__);				\
d7e09d039 Peng Tao 2013-05-02  590  		spin_unlock(&(lock));					\
d7e09d039 Peng Tao 2013-05-02  591  									\

:::::: The code at line 588 was first introduced by commit
:::::: d7e09d0397e84eefbabfd9cb353221f3c6448d83 staging: add Lustre file system client support

:::::: TO: Peng Tao <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 51721 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20151017/c786db5b/attachment-0001.obj>

#15 lore raw

[lustre-devel] [PATCH 4/4] staging: lustre: remove IS_PO2 and __is_po2

In reply to Aya Mahfouz
reproduce:

Hi Aya,

[auto build test ERROR on staging/staging-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Aya-Mahfouz/Remove-uses-and-definition-of-IS_PO2/20151017-060923
config: i386-allyesconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

drivers/staging/lustre/lnet/selftest/conrpc.c: In function 'lstcon_rpc_cleanup_wait':

>> drivers/staging/lustre/lnet/selftest/conrpc.c:1359:386: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
   cc1: some warnings being treated as errors
--
   drivers/staging/lustre/lnet/selftest/framework.c: In function 'sfw_shutdown':
>> drivers/staging/lustre/lnet/selftest/framework.c:1741:386: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
   cc1: some warnings being treated as errors
--
   drivers/staging/lustre/lnet/selftest/timer.c: In function 'stt_shutdown':
>> drivers/staging/lustre/lnet/selftest/timer.c:240:386: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
   cc1: some warnings being treated as errors
--
   drivers/staging/lustre/lnet/selftest/rpc.c: In function 'srpc_service_add_buffers':
>> drivers/staging/lustre/lnet/selftest/rpc.c:619:387: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]

cc1: some warnings being treated as errors

vim +/IS_PO2 +1359 drivers/staging/lustre/lnet/selftest/conrpc.c

d7e09d039 Peng Tao       2013-05-02  1353  
d7e09d039 Peng Tao       2013-05-02  1354  		mutex_lock(&console_session.ses_mutex);
d7e09d039 Peng Tao       2013-05-02  1355  	}
d7e09d039 Peng Tao       2013-05-02  1356  
d7e09d039 Peng Tao       2013-05-02  1357  	spin_lock(&console_session.ses_rpc_lock);
d7e09d039 Peng Tao       2013-05-02  1358  
d7e09d039 Peng Tao       2013-05-02 @1359  	lst_wait_until((atomic_read(&console_session.ses_rpc_counter) == 0),
d7e09d039 Peng Tao       2013-05-02  1360  		       console_session.ses_rpc_lock,
eac2e8c6f Rashika Kheria 2013-10-17  1361  		       "Network is not accessible or target is down, waiting for %d console RPCs to being recycled\n",
d7e09d039 Peng Tao       2013-05-02  1362  		       atomic_read(&console_session.ses_rpc_counter));

:::::: The code at line 1359 was first introduced by commit
:::::: d7e09d0397e84eefbabfd9cb353221f3c6448d83 staging: add Lustre file system client support

:::::: TO: Peng Tao <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 51116 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20151017/f53c46c0/attachment-0001.obj>

#16 lore raw

[lustre-devel] [PATCH 4/4] staging: lustre: remove IS_PO2 and __is_po2

In reply to Aya Mahfouz
reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Aya,

[auto build test ERROR on staging/staging-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Aya-Mahfouz/Remove-uses-and-definition-of-IS_PO2/20151017-060923
config: s390-allmodconfig (attached as .config)

reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390

All errors (new ones prefixed by >>):

In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:111:0,
from drivers/staging/lustre/lnet/selftest/conrpc.c:43:
drivers/staging/lustre/lnet/selftest/conrpc.c: In function 'lstcon_rpc_cleanup_wait':

>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:212:16: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
     static struct libcfs_debug_msg_data msgdata;   \
                   ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
   drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
   drivers/staging/lustre/lnet/selftest/conrpc.c:1359:2: note: in expansion of macro 'lst_wait_until'
     lst_wait_until((atomic_read(&console_session.ses_rpc_counter) == 0),
     ^

cc1: some warnings being treated as errors
--
In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:111:0,
from drivers/staging/lustre/lnet/selftest/selftest.h:46,
from drivers/staging/lustre/lnet/selftest/framework.c:44:
drivers/staging/lustre/lnet/selftest/framework.c: In function 'sfw_shutdown':

>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:212:16: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
     static struct libcfs_debug_msg_data msgdata;   \
                   ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
   drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
   drivers/staging/lustre/lnet/selftest/framework.c:1741:2: note: in expansion of macro 'lst_wait_until'
     lst_wait_until(sfw_data.fw_active_srpc == NULL, sfw_data.fw_lock,
     ^

cc1: some warnings being treated as errors
--
In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:111:0,
from drivers/staging/lustre/lnet/selftest/selftest.h:46,
from drivers/staging/lustre/lnet/selftest/timer.c:43:
drivers/staging/lustre/lnet/selftest/timer.c: In function 'stt_shutdown':

>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:212:16: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
     static struct libcfs_debug_msg_data msgdata;   \
                   ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
   drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
   drivers/staging/lustre/lnet/selftest/timer.c:240:2: note: in expansion of macro 'lst_wait_until'
     lst_wait_until(stt_data.stt_nthreads == 0, stt_data.stt_lock,
     ^

cc1: some warnings being treated as errors
--
In file included from drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs.h:111:0,
from drivers/staging/lustre/lnet/selftest/selftest.h:46,
from drivers/staging/lustre/lnet/selftest/rpc.c:47:
drivers/staging/lustre/lnet/selftest/rpc.c: In function 'srpc_service_add_buffers':

>> drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:212:16: error: implicit declaration of function 'IS_PO2' [-Werror=implicit-function-declaration]
     static struct libcfs_debug_msg_data msgdata;   \
                   ^
   drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h:222:35: note: in expansion of macro '__CDEBUG'
    #define CDEBUG(mask, format, ...) __CDEBUG(NULL, mask, format, ## __VA_ARGS__)
                                      ^
   drivers/staging/lustre/lnet/selftest/selftest.h:588:3: note: in expansion of macro 'CDEBUG'
      CDEBUG(IS_PO2(++__I) ? D_WARNING : D_NET,  \
      ^
   drivers/staging/lustre/lnet/selftest/rpc.c:619:3: note: in expansion of macro 'lst_wait_until'
      lst_wait_until(scd->scd_buf_err != 0 ||
      ^

cc1: some warnings being treated as errors

vim +/IS_PO2 +212 drivers/staging/lustre/lnet/selftest/../../include/linux/libcfs/libcfs_debug.h

d7e09d039 Peng Tao 2013-05-02  206  	return mask & D_CANTMASK ||
d7e09d039 Peng Tao 2013-05-02  207  		((libcfs_debug & mask) && (libcfs_subsystem_debug & subsystem));
d7e09d039 Peng Tao 2013-05-02  208  }
d7e09d039 Peng Tao 2013-05-02  209  
d7e09d039 Peng Tao 2013-05-02  210  #define __CDEBUG(cdls, mask, format, ...)				\
d7e09d039 Peng Tao 2013-05-02  211  do {									\
d7e09d039 Peng Tao 2013-05-02 @212  	static struct libcfs_debug_msg_data msgdata;			\
d7e09d039 Peng Tao 2013-05-02  213  									\
d7e09d039 Peng Tao 2013-05-02  214  	CFS_CHECK_STACK(&msgdata, mask, cdls);				\
d7e09d039 Peng Tao 2013-05-02  215  									\

:::::: The code at line 212 was first introduced by commit
:::::: d7e09d0397e84eefbabfd9cb353221f3c6448d83 staging: add Lustre file system client support

:::::: TO: Peng Tao <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 38548 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20151017/ed392ff4/attachment-0001.obj>

#17 lore raw

PATCH 4/4 staging: lustre: remove IS_PO2 and __is_po2

In reply to Aya Mahfouz
Quoted 7 lines · Aya Mahfouz

Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have been replaced by is_power_of_2

---
 drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
 1 file changed, 7 deletions(-)

You didn't test build this patch :(

#18 lore raw

PATCH 4/4 staging: lustre: remove IS_PO2 and __is_po2

Quoted 10 lines · Greg KH
Quoted 7 lines · Aya Mahfouz

Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have been replaced by is_power_of_2

---
 drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
 1 file changed, 7 deletions(-)

You didn't test build this patch :(

I think I'm too old for this mistake. The way kbuild tests patch sets will cause errors for sure. It tests every patch independently and of course the removal of IS_PO2 will cause a problem if the previous patches were not applied.

Signature · 2 lines
Kind Regards,
Aya Saif El-yazal Mahfouz
#19 lore raw

PATCH 4/4 staging: lustre: remove IS_PO2 and __is_po2

In reply to Greg KH
Quoted 10 lines · Greg KH
Quoted 7 lines · Aya Mahfouz

Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have been replaced by is_power_of_2

---
 drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
 1 file changed, 7 deletions(-)

You didn't test build this patch :(

I'm sorry I didn't run spatch on the header files too. I reran the scripts on all lustre files including header and issued make clean and then make. Will be resending v2 soon. kbuild does apply all patches in a patch set in order.

Signature · 2 lines
Kind Regards,
Aya Saif El-yazal Mahfouz