[lustre-devel] [PATCH 0/4] Remove uses and definition of IS_PO2

[lustre-devel] [PATCH 0/4] Remove uses and definition of IS_PO2

[Aya Mahfouz]

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(-)

-- 
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 1/4] staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2

[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.

Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
 drivers/staging/lustre/lustre/ldlm/ldlm_extent.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
index 57b997d..3e7e97e 100644
--- a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c
@@ -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);
-- 
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 2/4] staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

[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.

Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
 drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
index 6f4c7d4..4b5e79a 100644
--- a/drivers/staging/lustre/lustre/libcfs/hash.c
+++ b/drivers/staging/lustre/lustre/libcfs/hash.c
@@ -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();
-- 
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 3/4] staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2

[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.

Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
 drivers/staging/lustre/lustre/libcfs/workitem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/libcfs/workitem.c b/drivers/staging/lustre/lustre/libcfs/workitem.c
index e1143a5..377e1ea 100644
--- a/drivers/staging/lustre/lustre/libcfs/workitem.c
+++ b/drivers/staging/lustre/lustre/libcfs/workitem.c
@@ -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);
 
-- 
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

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

[Aya Mahfouz]

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

Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
---
 drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
index 385ced1..c78a147 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
@@ -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))
 
 /*
-- 
2.4.2


-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

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

[kbuild test robot]

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 <bergwolf@gmail.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
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>

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

[kbuild test robot]

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 <bergwolf@gmail.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
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>

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

[kbuild test robot]

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 <bergwolf@gmail.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
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>

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

[Greg KH]

On Sat, Oct 17, 2015 at 12:07:28AM +0200, Aya Mahfouz wrote:
> Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have
> been replaced by is_power_of_2
> 
> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> ---
>  drivers/staging/lustre/include/linux/libcfs/libcfs.h | 7 -------
>  1 file changed, 7 deletions(-)

You didn't test build this patch :(

[lustre-devel] [PATCH 2/4] staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

[Greg KH]

On Sat, Oct 17, 2015 at 12:06:28AM +0200, Aya Mahfouz wrote:
> 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.
> 
> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> ---
>  drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
> index 6f4c7d4..4b5e79a 100644
> --- a/drivers/staging/lustre/lustre/libcfs/hash.c
> +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
> @@ -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,

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

[lustre-devel] [PATCH 3/4] staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2

[Greg KH]

On Sat, Oct 17, 2015 at 12:06:59AM +0200, Aya Mahfouz wrote:
> 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.
> 
> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> ---
>  drivers/staging/lustre/lustre/libcfs/workitem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/lustre/lustre/libcfs/workitem.c b/drivers/staging/lustre/lustre/libcfs/workitem.c
> index e1143a5..377e1ea 100644
> --- a/drivers/staging/lustre/lustre/libcfs/workitem.c
> +++ b/drivers/staging/lustre/lustre/libcfs/workitem.c
> @@ -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,

Same question about 0 here as well.

thanks,

greg k-h

[lustre-devel] [PATCH 1/4] staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2

[Greg KH]

On Sat, Oct 17, 2015 at 12:05:44AM +0200, Aya Mahfouz wrote:
> 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 :(

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

[Aya Mahfouz]

On Fri, Oct 16, 2015 at 10:21:05PM -0700, Greg KH wrote:
> On Sat, Oct 17, 2015 at 12:07:28AM +0200, Aya Mahfouz wrote:
> > Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have
> > been replaced by is_power_of_2
> > 
> > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > ---
> >  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.

-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 1/4] staging: lustre: ldlm_extent.c: replace IS_PO2 by is_power_of_2

[Aya Mahfouz]

On Fri, Oct 16, 2015 at 10:41:09PM -0700, Greg KH wrote:
> On Sat, Oct 17, 2015 at 12:05:44AM +0200, Aya Mahfouz wrote:
> > 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.

-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 2/4] staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

[Aya Mahfouz]

On Fri, Oct 16, 2015 at 10:40:25PM -0700, Greg KH wrote:
> On Sat, Oct 17, 2015 at 12:06:28AM +0200, Aya Mahfouz wrote:
> > 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.
> > 
> > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > ---
> >  drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
> > index 6f4c7d4..4b5e79a 100644
> > --- a/drivers/staging/lustre/lustre/libcfs/hash.c
> > +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
> > @@ -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,
> 
> 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?

> thanks,
> 
> greg k-h

-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 2/4] staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

[Julia Lawall]

On Sat, 17 Oct 2015, Aya Mahfouz wrote:

> On Fri, Oct 16, 2015 at 10:40:25PM -0700, Greg KH wrote:
> > On Sat, Oct 17, 2015 at 12:06:28AM +0200, Aya Mahfouz wrote:
> > > 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.
> > > 
> > > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > > ---
> > >  drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
> > > index 6f4c7d4..4b5e79a 100644
> > > --- a/drivers/staging/lustre/lustre/libcfs/hash.c
> > > +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
> > > @@ -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,
> > 
> > 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?

What is the actual result in each case?

julia

[lustre-devel] [PATCH 2/4] staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

[Aya Mahfouz]

On Sat, Oct 17, 2015 at 12:47:13PM +0200, Julia Lawall wrote:
> On Sat, 17 Oct 2015, Aya Mahfouz wrote:
> 
> > On Fri, Oct 16, 2015 at 10:40:25PM -0700, Greg KH wrote:
> > > On Sat, Oct 17, 2015 at 12:06:28AM +0200, Aya Mahfouz wrote:
> > > > 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.
> > > > 
> > > > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > > > ---
> > > >  drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
> > > > index 6f4c7d4..4b5e79a 100644
> > > > --- a/drivers/staging/lustre/lustre/libcfs/hash.c
> > > > +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
> > > > @@ -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,
> > > 
> > > 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?
> 
> 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

> julia

-- 
Kind Regards,
Aya Saif El-yazal Mahfouz

[lustre-devel] [PATCH 2/4] staging: lustre: hash.c: replace IS_PO2 by is_power_of_2

[Dilger, Andreas]

On Oct 17, 2015, at 07:23, Aya Mahfouz <mahfouz.saif.elyazal@gmail.com> wrote:
> 
>> On Sat, Oct 17, 2015 at 12:47:13PM +0200, Julia Lawall wrote:
>>> On Sat, 17 Oct 2015, Aya Mahfouz wrote:
>>> 
>>>> On Fri, Oct 16, 2015 at 10:40:25PM -0700, Greg KH wrote:
>>>>> On Sat, Oct 17, 2015 at 12:06:28AM +0200, Aya Mahfouz wrote:
>>>>> 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.
>>>>> 
>>>>> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
>>>>> ---
>>>>> drivers/staging/lustre/lustre/libcfs/hash.c | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/drivers/staging/lustre/lustre/libcfs/hash.c b/drivers/staging/lustre/lustre/libcfs/hash.c
>>>>> index 6f4c7d4..4b5e79a 100644
>>>>> --- a/drivers/staging/lustre/lustre/libcfs/hash.c
>>>>> +++ b/drivers/staging/lustre/lustre/libcfs/hash.c
>>>>> @@ -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,
>>>> 
>>>> 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?
>> 
>> 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:

Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>

Cheers, Andreas

[lustre-devel] [PATCH 3/4] staging: lustre: workitem.c: replace IS_PO2 with is_power_of_2

[Dilger, Andreas]

On Oct 16, 2015, at 23:40, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
>> On Sat, Oct 17, 2015 at 12:06:59AM +0200, Aya Mahfouz wrote:
>> 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.
>> 
>> Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
>> ---
>> drivers/staging/lustre/lustre/libcfs/workitem.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/libcfs/workitem.c b/drivers/staging/lustre/lustre/libcfs/workitem.c
>> index e1143a5..377e1ea 100644
>> --- a/drivers/staging/lustre/lustre/libcfs/workitem.c
>> +++ b/drivers/staging/lustre/lustre/libcfs/workitem.c
>> @@ -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,
> 
> 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:

Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>

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

[Aya Mahfouz]

On Fri, Oct 16, 2015 at 10:21:05PM -0700, Greg KH wrote:
> On Sat, Oct 17, 2015 at 12:07:28AM +0200, Aya Mahfouz wrote:
> > Removes IS_PO2 and __is_po2 since the uses of IS_PO2 have
> > been replaced by is_power_of_2
> > 
> > Signed-off-by: Aya Mahfouz <mahfouz.saif.elyazal@gmail.com>
> > ---
> >  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.

-- 
Kind Regards,
Aya Saif El-yazal Mahfouz