[PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use

[PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use

[Andy Shevchenko]

There are users already and will be more of BITS_TO_BYTES() macro.
Move it to bitops.h for wider use.

In the case of ocfs2 the replacement is identical.

As for bnx2x, there are two places where floor version is used.
In the first case to calculate the amount of structures that can fit
one memory page. In this case obviously the ceiling variant is correct and
original code might have a potential bug, if amount of bits % 8 is not 0.
In the second case the macro is used to calculate bytes transmitted in one
microsecond. This will work for all speeds which is multiply of 1Gbps without
any change, for the rest new code will give ceiling value, for instance 100Mbps
will give 13 bytes, while old code gives 12 bytes and the arithmetically
correct one is 12.5 bytes. Further the value is used to setup timer threshold
which in any case has its own margins due to certain resolution. I don't see
here an issue with slightly shifting thresholds for low speed connections, the
card is supposed to utilize highest available rate, which is usually 10Gbps.

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
- described bnx2x cases in the commit message
- appended Rb (for ocfs2)

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h | 1 -
 fs/ocfs2/dlm/dlmcommon.h                         | 4 ----
 include/linux/bitops.h                           | 1 +
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
index 066765fbef06..0a59a09ef82f 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
@@ -296,7 +296,6 @@ static inline void bnx2x_dcb_config_qm(struct bnx2x *bp, enum cos_mode mode,
  *    possible, the driver should only write the valid vnics into the internal
  *    ram according to the appropriate port mode.
  */
-#define BITS_TO_BYTES(x) ((x)/8)
 
 /* CMNG constants, as derived from system spec calculations */
 
diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h
index aaf24548b02a..0463dce65bb2 100644
--- a/fs/ocfs2/dlm/dlmcommon.h
+++ b/fs/ocfs2/dlm/dlmcommon.h
@@ -688,10 +688,6 @@ struct dlm_begin_reco
 	__be32 pad2;
 };
 
-
-#define BITS_PER_BYTE 8
-#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
-
 struct dlm_query_join_request
 {
 	u8 node_idx;
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index cf074bce3eb3..79d80f5ddf7b 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -5,6 +5,7 @@
 #include <linux/bits.h>
 
 #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
+#define BITS_TO_BYTES(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE)
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
 
 extern unsigned int __sw_hweight8(unsigned int w);
-- 
2.23.0.rc1

Re: [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use

[Andy Shevchenko]

On Tue, Aug 27, 2019 at 07:37:17PM +0300, Andy Shevchenko wrote:
> There are users already and will be more of BITS_TO_BYTES() macro.
> Move it to bitops.h for wider use.
> 
> In the case of ocfs2 the replacement is identical.
> 
> As for bnx2x, there are two places where floor version is used.
> In the first case to calculate the amount of structures that can fit
> one memory page. In this case obviously the ceiling variant is correct and
> original code might have a potential bug, if amount of bits % 8 is not 0.
> In the second case the macro is used to calculate bytes transmitted in one
> microsecond. This will work for all speeds which is multiply of 1Gbps without
> any change, for the rest new code will give ceiling value, for instance 100Mbps
> will give 13 bytes, while old code gives 12 bytes and the arithmetically
> correct one is 12.5 bytes. Further the value is used to setup timer threshold
> which in any case has its own margins due to certain resolution. I don't see
> here an issue with slightly shifting thresholds for low speed connections, the
> card is supposed to utilize highest available rate, which is usually 10Gbps.

Anybody to comment on bnx2 change?
Can we survive with this applied?

> 
> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> - described bnx2x cases in the commit message
> - appended Rb (for ocfs2)
> 
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h | 1 -
>  fs/ocfs2/dlm/dlmcommon.h                         | 4 ----
>  include/linux/bitops.h                           | 1 +
>  3 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> index 066765fbef06..0a59a09ef82f 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> @@ -296,7 +296,6 @@ static inline void bnx2x_dcb_config_qm(struct bnx2x *bp, enum cos_mode mode,
>   *    possible, the driver should only write the valid vnics into the internal
>   *    ram according to the appropriate port mode.
>   */
> -#define BITS_TO_BYTES(x) ((x)/8)
>  
>  /* CMNG constants, as derived from system spec calculations */
>  
> diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h
> index aaf24548b02a..0463dce65bb2 100644
> --- a/fs/ocfs2/dlm/dlmcommon.h
> +++ b/fs/ocfs2/dlm/dlmcommon.h
> @@ -688,10 +688,6 @@ struct dlm_begin_reco
>  	__be32 pad2;
>  };
>  
> -
> -#define BITS_PER_BYTE 8
> -#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
> -
>  struct dlm_query_join_request
>  {
>  	u8 node_idx;
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index cf074bce3eb3..79d80f5ddf7b 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -5,6 +5,7 @@
>  #include <linux/bits.h>
>  
>  #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
> +#define BITS_TO_BYTES(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE)
>  #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
>  
>  extern unsigned int __sw_hweight8(unsigned int w);
> -- 
> 2.23.0.rc1
> 

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use

[Sudarsana Reddy Kalluru]

> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Andy Shevchenko
> Sent: Monday, December 9, 2019 10:14 PM
> To: Mark Fasheh <mark@fasheh.com>; Joel Becker <jlbec@evilplan.org>;
> Joseph Qi <joseph.qi@linux.alibaba.com>; ocfs2-devel@oss.oracle.com; Ariel
> Elior <aelior@marvell.com>; Sudarsana Reddy Kalluru
> <skalluru@marvell.com>; GR-everest-linux-l2 <GR-everest-linux-
> l2@marvell.com>; David S. Miller <davem@davemloft.net>;
> netdev@vger.kernel.org; Colin Ian King <colin.king@canonical.com>
> Subject: Re: [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for
> wider use
> 
> On Tue, Aug 27, 2019 at 07:37:17PM +0300, Andy Shevchenko wrote:
> > There are users already and will be more of BITS_TO_BYTES() macro.
> > Move it to bitops.h for wider use.
> >
> > In the case of ocfs2 the replacement is identical.
> >
> > As for bnx2x, there are two places where floor version is used.
> > In the first case to calculate the amount of structures that can fit
> > one memory page. In this case obviously the ceiling variant is correct
> > and original code might have a potential bug, if amount of bits % 8 is not 0.
> > In the second case the macro is used to calculate bytes transmitted in
> > one microsecond. This will work for all speeds which is multiply of
> > 1Gbps without any change, for the rest new code will give ceiling
> > value, for instance 100Mbps will give 13 bytes, while old code gives
> > 12 bytes and the arithmetically correct one is 12.5 bytes. Further the
> > value is used to setup timer threshold which in any case has its own
> > margins due to certain resolution. I don't see here an issue with
> > slightly shifting thresholds for low speed connections, the card is supposed
> to utilize highest available rate, which is usually 10Gbps.
> 
> Anybody to comment on bnx2 change?
> Can we survive with this applied?
> 
> >
> > Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > - described bnx2x cases in the commit message
> > - appended Rb (for ocfs2)
> >
> >  drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h | 1 -
> >  fs/ocfs2/dlm/dlmcommon.h                         | 4 ----
> >  include/linux/bitops.h                           | 1 +
> >  3 files changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> > b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> > index 066765fbef06..0a59a09ef82f 100644
> > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
> > @@ -296,7 +296,6 @@ static inline void bnx2x_dcb_config_qm(struct bnx2x
> *bp, enum cos_mode mode,
> >   *    possible, the driver should only write the valid vnics into the internal
> >   *    ram according to the appropriate port mode.
> >   */
> > -#define BITS_TO_BYTES(x) ((x)/8)
> >
> >  /* CMNG constants, as derived from system spec calculations */
> >
> > diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h index
> > aaf24548b02a..0463dce65bb2 100644
> > --- a/fs/ocfs2/dlm/dlmcommon.h
> > +++ b/fs/ocfs2/dlm/dlmcommon.h
> > @@ -688,10 +688,6 @@ struct dlm_begin_reco
> >  	__be32 pad2;
> >  };
> >
> > -
> > -#define BITS_PER_BYTE 8
> > -#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
> > -
> >  struct dlm_query_join_request
> >  {
> >  	u8 node_idx;
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h index
> > cf074bce3eb3..79d80f5ddf7b 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -5,6 +5,7 @@
> >  #include <linux/bits.h>
> >
> >  #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
> > +#define BITS_TO_BYTES(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE)
> >  #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
> >
> >  extern unsigned int __sw_hweight8(unsigned int w);
> > --
> > 2.23.0.rc1
> >
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Thanks for the changes.

Acked-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>

Re: [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use

[Joseph Qi]

Add akpm as well.

On 19/12/11 11:32, Sudarsana Reddy Kalluru wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
>> Behalf Of Andy Shevchenko
>> Sent: Monday, December 9, 2019 10:14 PM
>> To: Mark Fasheh <mark@fasheh.com>; Joel Becker <jlbec@evilplan.org>;
>> Joseph Qi <joseph.qi@linux.alibaba.com>; ocfs2-devel@oss.oracle.com; Ariel
>> Elior <aelior@marvell.com>; Sudarsana Reddy Kalluru
>> <skalluru@marvell.com>; GR-everest-linux-l2 <GR-everest-linux-
>> l2@marvell.com>; David S. Miller <davem@davemloft.net>;
>> netdev@vger.kernel.org; Colin Ian King <colin.king@canonical.com>
>> Subject: Re: [PATCH v2] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for
>> wider use
>>
>> On Tue, Aug 27, 2019 at 07:37:17PM +0300, Andy Shevchenko wrote:
>>> There are users already and will be more of BITS_TO_BYTES() macro.
>>> Move it to bitops.h for wider use.
>>>
>>> In the case of ocfs2 the replacement is identical.
>>>
>>> As for bnx2x, there are two places where floor version is used.
>>> In the first case to calculate the amount of structures that can fit
>>> one memory page. In this case obviously the ceiling variant is correct
>>> and original code might have a potential bug, if amount of bits % 8 is not 0.
>>> In the second case the macro is used to calculate bytes transmitted in
>>> one microsecond. This will work for all speeds which is multiply of
>>> 1Gbps without any change, for the rest new code will give ceiling
>>> value, for instance 100Mbps will give 13 bytes, while old code gives
>>> 12 bytes and the arithmetically correct one is 12.5 bytes. Further the
>>> value is used to setup timer threshold which in any case has its own
>>> margins due to certain resolution. I don't see here an issue with
>>> slightly shifting thresholds for low speed connections, the card is supposed
>> to utilize highest available rate, which is usually 10Gbps.
>>
>> Anybody to comment on bnx2 change?
>> Can we survive with this applied?
>>
>>>
>>> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> ---
>>> - described bnx2x cases in the commit message
>>> - appended Rb (for ocfs2)
>>>
>>>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h | 1 -
>>>  fs/ocfs2/dlm/dlmcommon.h                         | 4 ----
>>>  include/linux/bitops.h                           | 1 +
>>>  3 files changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
>>> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
>>> index 066765fbef06..0a59a09ef82f 100644
>>> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
>>> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h
>>> @@ -296,7 +296,6 @@ static inline void bnx2x_dcb_config_qm(struct bnx2x
>> *bp, enum cos_mode mode,
>>>   *    possible, the driver should only write the valid vnics into the internal
>>>   *    ram according to the appropriate port mode.
>>>   */
>>> -#define BITS_TO_BYTES(x) ((x)/8)
>>>
>>>  /* CMNG constants, as derived from system spec calculations */
>>>
>>> diff --git a/fs/ocfs2/dlm/dlmcommon.h b/fs/ocfs2/dlm/dlmcommon.h index
>>> aaf24548b02a..0463dce65bb2 100644
>>> --- a/fs/ocfs2/dlm/dlmcommon.h
>>> +++ b/fs/ocfs2/dlm/dlmcommon.h
>>> @@ -688,10 +688,6 @@ struct dlm_begin_reco
>>>  	__be32 pad2;
>>>  };
>>>
>>> -
>>> -#define BITS_PER_BYTE 8
>>> -#define BITS_TO_BYTES(bits) (((bits)+BITS_PER_BYTE-1)/BITS_PER_BYTE)
>>> -
>>>  struct dlm_query_join_request
>>>  {
>>>  	u8 node_idx;
>>> diff --git a/include/linux/bitops.h b/include/linux/bitops.h index
>>> cf074bce3eb3..79d80f5ddf7b 100644
>>> --- a/include/linux/bitops.h
>>> +++ b/include/linux/bitops.h
>>> @@ -5,6 +5,7 @@
>>>  #include <linux/bits.h>
>>>
>>>  #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
>>> +#define BITS_TO_BYTES(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE)
>>>  #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
>>>
>>>  extern unsigned int __sw_hweight8(unsigned int w);
>>> --
>>> 2.23.0.rc1
>>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>>
> 
> Thanks for the changes.
> 
> Acked-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
>