netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
@ 2019-08-20 16:31 Andy Shevchenko
  2019-08-21  1:29 ` Joseph Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-08-20 16:31 UTC (permalink / raw)
  To: Mark Fasheh, Joel Becker, Joseph Qi, ocfs2-devel, Ariel Elior,
	Sudarsana Kalluru, GR-everest-linux-l2, David S. Miller, netdev,
	Colin Ian King
  Cc: Andy Shevchenko

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 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


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

* Re: [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
  2019-08-20 16:31 [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use Andy Shevchenko
@ 2019-08-21  1:29 ` Joseph Qi
  2019-08-21  9:25   ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Joseph Qi @ 2019-08-21  1:29 UTC (permalink / raw)
  To: Andy Shevchenko, Mark Fasheh, Joel Becker, ocfs2-devel,
	Ariel Elior, Sudarsana Kalluru, GR-everest-linux-l2,
	David S. Miller, netdev, Colin Ian King



On 19/8/21 00:31, Andy Shevchenko wrote:
> There are users already and will be more of BITS_TO_BYTES() macro.
> Move it to bitops.h for wider use.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  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)>
I don't think this is a equivalent replace, or it is in fact
wrong before?

  
>  /* 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)
> -
For ocfs2 part, it looks good to me.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

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

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

* Re: [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
  2019-08-21  1:29 ` Joseph Qi
@ 2019-08-21  9:25   ` Andy Shevchenko
  2019-08-22  5:46     ` Sudarsana Reddy Kalluru
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-08-21  9:25 UTC (permalink / raw)
  To: Joseph Qi
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, Ariel Elior,
	Sudarsana Kalluru, GR-everest-linux-l2, David S. Miller, netdev,
	Colin Ian King

On Wed, Aug 21, 2019 at 09:29:04AM +0800, Joseph Qi wrote:
> On 19/8/21 00:31, Andy Shevchenko wrote:
> > There are users already and will be more of BITS_TO_BYTES() macro.
> > Move it to bitops.h for wider use.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  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)>
> I don't think this is a equivalent replace, or it is in fact
> wrong before?

I was thinking about this one and there are two applications:
- calculus of the amount of structures of certain type per PAGE
  (obviously off-by-one error in the original code IIUC purpose of STRUCT_SIZE)
- calculus of some threshold based on line speed in bytes per second
  (I dunno it will have any difference on the Gbs / 100 MBs speeds)

> >  /* 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)
> > -
> For ocfs2 part, it looks good to me.
> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

Thanks!

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

-- 
With Best Regards,
Andy Shevchenko



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

* RE: [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
  2019-08-21  9:25   ` Andy Shevchenko
@ 2019-08-22  5:46     ` Sudarsana Reddy Kalluru
  2019-08-22  9:08       ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Sudarsana Reddy Kalluru @ 2019-08-22  5:46 UTC (permalink / raw)
  To: Andy Shevchenko, Joseph Qi
  Cc: Mark Fasheh, Joel Becker, ocfs2-devel, Ariel Elior,
	GR-everest-linux-l2, David S. Miller, netdev, Colin Ian King


> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Andy Shevchenko
> Sent: Wednesday, August 21, 2019 2:56 PM
> To: Joseph Qi <joseph.qi@linux.alibaba.com>
> Cc: Mark Fasheh <mark@fasheh.com>; Joel Becker <jlbec@evilplan.org>;
> 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 v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for
> wider use
> 
> On Wed, Aug 21, 2019 at 09:29:04AM +0800, Joseph Qi wrote:
> > On 19/8/21 00:31, Andy Shevchenko wrote:
> > > There are users already and will be more of BITS_TO_BYTES() macro.
> > > Move it to bitops.h for wider use.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  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)>
> > I don't think this is a equivalent replace, or it is in fact wrong
> > before?
> 
> I was thinking about this one and there are two applications:
> - calculus of the amount of structures of certain type per PAGE
>   (obviously off-by-one error in the original code IIUC purpose of
> STRUCT_SIZE)
> - calculus of some threshold based on line speed in bytes per second
>   (I dunno it will have any difference on the Gbs / 100 MBs speeds)
> 
I see that both the implementations (existing vs new) yield same value for standard speeds 10G (i.e.,10000), 1G (1000) that device supports. Hence the change look to be ok.

> > >  /* 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)
> > > -
> > For ocfs2 part, it looks good to me.
> > Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> 
> Thanks!
> 
> >
> > >  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);
> > >
> 
> --
> With Best Regards,
> Andy Shevchenko
> 


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

* Re: [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
  2019-08-22  5:46     ` Sudarsana Reddy Kalluru
@ 2019-08-22  9:08       ` Andy Shevchenko
  2019-08-22  9:16         ` Sudarsana Reddy Kalluru
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2019-08-22  9:08 UTC (permalink / raw)
  To: Sudarsana Reddy Kalluru
  Cc: Joseph Qi, Mark Fasheh, Joel Becker, ocfs2-devel, Ariel Elior,
	GR-everest-linux-l2, David S. Miller, netdev, Colin Ian King

On Thu, Aug 22, 2019 at 05:46:07AM +0000, Sudarsana Reddy Kalluru wrote:
> 
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> > Behalf Of Andy Shevchenko
> > Sent: Wednesday, August 21, 2019 2:56 PM
> > To: Joseph Qi <joseph.qi@linux.alibaba.com>
> > Cc: Mark Fasheh <mark@fasheh.com>; Joel Becker <jlbec@evilplan.org>;
> > 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 v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for
> > wider use
> > 
> > On Wed, Aug 21, 2019 at 09:29:04AM +0800, Joseph Qi wrote:
> > > On 19/8/21 00:31, Andy Shevchenko wrote:
> > > > There are users already and will be more of BITS_TO_BYTES() macro.
> > > > Move it to bitops.h for wider use.

> > > > -#define BITS_TO_BYTES(x) ((x)/8)>
> > > I don't think this is a equivalent replace, or it is in fact wrong
> > > before?
> > 
> > I was thinking about this one and there are two applications:
> > - calculus of the amount of structures of certain type per PAGE
> >   (obviously off-by-one error in the original code IIUC purpose of
> > STRUCT_SIZE)
> > - calculus of some threshold based on line speed in bytes per second
> >   (I dunno it will have any difference on the Gbs / 100 MBs speeds)
> > 
> I see that both the implementations (existing vs new) yield same value for standard speeds 10G (i.e.,10000), 1G (1000) that device supports. Hence the change look to be ok.

Thank you for testing, may I use your Tested-by tag?

-- 
With Best Regards,
Andy Shevchenko



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

* RE: [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use
  2019-08-22  9:08       ` Andy Shevchenko
@ 2019-08-22  9:16         ` Sudarsana Reddy Kalluru
  0 siblings, 0 replies; 6+ messages in thread
From: Sudarsana Reddy Kalluru @ 2019-08-22  9:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Joseph Qi, Mark Fasheh, Joel Becker, ocfs2-devel, Ariel Elior,
	GR-everest-linux-l2, David S. Miller, netdev, Colin Ian King


> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Thursday, August 22, 2019 2:39 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Joseph Qi <joseph.qi@linux.alibaba.com>; Mark Fasheh
> <mark@fasheh.com>; Joel Becker <jlbec@evilplan.org>; ocfs2-
> devel@oss.oracle.com; Ariel Elior <aelior@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 v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for
> wider use
> 
> On Thu, Aug 22, 2019 at 05:46:07AM +0000, Sudarsana Reddy Kalluru wrote:
> >
> > > -----Original Message-----
> > > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org>
> On
> > > Behalf Of Andy Shevchenko
> > > Sent: Wednesday, August 21, 2019 2:56 PM
> > > To: Joseph Qi <joseph.qi@linux.alibaba.com>
> > > Cc: Mark Fasheh <mark@fasheh.com>; Joel Becker <jlbec@evilplan.org>;
> > > 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 v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h
> > > for wider use
> > >
> > > On Wed, Aug 21, 2019 at 09:29:04AM +0800, Joseph Qi wrote:
> > > > On 19/8/21 00:31, Andy Shevchenko wrote:
> > > > > There are users already and will be more of BITS_TO_BYTES() macro.
> > > > > Move it to bitops.h for wider use.
> 
> > > > > -#define BITS_TO_BYTES(x) ((x)/8)>
> > > > I don't think this is a equivalent replace, or it is in fact wrong
> > > > before?
> > >
> > > I was thinking about this one and there are two applications:
> > > - calculus of the amount of structures of certain type per PAGE
> > >   (obviously off-by-one error in the original code IIUC purpose of
> > > STRUCT_SIZE)
> > > - calculus of some threshold based on line speed in bytes per second
> > >   (I dunno it will have any difference on the Gbs / 100 MBs speeds)
> > >
> > I see that both the implementations (existing vs new) yield same value for
> standard speeds 10G (i.e.,10000), 1G (1000) that device supports. Hence the
> change look to be ok.
> 
> Thank you for testing, may I use your Tested-by tag?
Sorry, I didn't test the actual driver flows. Was only referring to the output values for 1000/10000 speed values.

> 
> --
> With Best Regards,
> Andy Shevchenko
> 


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

end of thread, other threads:[~2019-08-22  9:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20 16:31 [PATCH v1] ocfs2/dlm: Move BITS_TO_BYTES() to bitops.h for wider use Andy Shevchenko
2019-08-21  1:29 ` Joseph Qi
2019-08-21  9:25   ` Andy Shevchenko
2019-08-22  5:46     ` Sudarsana Reddy Kalluru
2019-08-22  9:08       ` Andy Shevchenko
2019-08-22  9:16         ` Sudarsana Reddy Kalluru

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