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

[Joseph Qi <joseph.qi@linux.alibaba.com>]

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