Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-12 Thread Nicolas Saenz Julienne 
On Thu, 2019-12-05 at 16:30 -0600, Bjorn Helgaas wrote:
> You got the "n" on "down" in the subject, but still missing "of" ;)

Yes, sorry about that, I tend to re-read what I meant to say instead of what
it's actually written.

> On Tue, Dec 03, 2019 at 12:47:40PM +0100, Nicolas Saenz Julienne wrote:
> > Some users need to make sure their rounding function accepts and returns
> > 64bit long variables regardless of the architecture. Sadly
> > roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
> > out ilog2() already handles 32/64bit calculations properly, and being
> > the building block to the round functions we can rework them as a
> > wrapper around it.
> 
> Missing "of" in the function names here.
> s/a wrapper/wrappers/

Noted

> IIUC the point of this is that roundup_pow_of_two() returned
> "unsigned long", which can be either 32 or 64 bits (worth pointing
> out, I think), and many callers need something that returns
> "unsigned long long" (always 64 bits).

I'll update the commit message to be a more explicit.

> It's a nice simplification to remove the "__" variants.  Just as a
> casual reader of this commit message, I'd like to know why we had both
> the roundup and the __roundup versions in the first place, and why we
> no longer need both.

So, the commit that introduced it (312a0c170945b) meant to use the '__' variant
as a helper, but, due to the fact this is a header file, some found it and made
use of it. I went over some if the commits introducing '__' usages and none of
them seem to acknowledge its use as opposed to the macro version. I think it's
fair to say it's a case of cargo-culting.

> > -#define roundup_pow_of_two(n)  \
> > -(  \
> > -   __builtin_constant_p(n) ? ( \
> > -   (n == 1) ? 1 :  \
> > -   (1UL << (ilog2((n) - 1) + 1))   \
> > -  ) :  \
> > -   __roundup_pow_of_two(n) \
> > - )
> > +#define roundup_pow_of_two(n)\
> > +(\
> > +   (__builtin_constant_p(n) && ((n) == 1)) ? \
> > +   1 : (1ULL << (ilog2((n) - 1) + 1))\
> > +)
> 
> Should the resulting type of this expression always be a ULL, even
> when n==1, i.e., should it be this?
> 
>   1ULL : (1ULL << (ilog2((n) - 1) + 1))\
> 
> Or maybe there's no case where that makes a difference?

It should be 1ULL on either case.

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-12 Thread Nicolas Saenz Julienne 
Hi Robin,

On Thu, 2019-12-05 at 17:48 +, Robin Murphy wrote:
> On 03/12/2019 11:47 am, Nicolas Saenz Julienne wrote:
> > Some users need to make sure their rounding function accepts and returns
> > 64bit long variables regardless of the architecture. Sadly
> > roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
> > out ilog2() already handles 32/64bit calculations properly, and being
> > the building block to the round functions we can rework them as a
> > wrapper around it.
> 
> Neat! Although all the additional ULL casts this introduces seem 
> somewhat unwelcome - I suppose the (1ULL << (ilog2(n))) makes it 
> effectively always return unsigned long long now. Might it make sense to 
> cast the return value to typeof(n) to avoid this slightly non-obvious 
> behaviour (and the associated churn)?

It might alleviate some of the churn alright but I don't think a cast is really
going to make the behaviour more obvious. Say your expression is a big mess,
you'll have to analyze it to infer the output type, keeping in mind things like
integer promotion. See this example, 'params->nelem_hint' and
'params->min_size' are u16:

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index bdb7e4cadf05..70908678c7a8 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -950,7 +950,7 @@ static size_t rounded_hashtable_size(const struct 
rhashtable_params *params)

if (params->nelem_hint)
retsize = max(roundup_pow_of_two(params->nelem_hint * 4 
/ 3),
- (unsigned long)params->min_size);
+ (unsigned long long)params->min_size);
else
retsize = max(HASH_DEFAULT_SIZE,
  (unsigned long)params->min_size);

With a cast the patch will look like this:

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index bdb7e4cadf05..70908678c7a8 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -950,7 +950,7 @@ static size_t rounded_hashtable_size(const struct 
rhashtable_params *params)

if (params->nelem_hint)
retsize = max(roundup_pow_of_two(params->nelem_hint * 4 
/ 3),
- (unsigned long)params->min_size);
+ (int)params->min_size);
else
retsize = max(HASH_DEFAULT_SIZE,
  (unsigned long)params->min_size);

To me it's even less obvious than with a fixed ULL.

My intuition tells me to keep it as similar as the old behaviour, at the
expense of the extra churn (which is not that different from the current status
quo anyway). That said, I'll be happy to change it.

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-05 Thread Bjorn Helgaas 
You got the "n" on "down" in the subject, but still missing "of" ;)

On Tue, Dec 03, 2019 at 12:47:40PM +0100, Nicolas Saenz Julienne wrote:
> Some users need to make sure their rounding function accepts and returns
> 64bit long variables regardless of the architecture. Sadly
> roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
> out ilog2() already handles 32/64bit calculations properly, and being
> the building block to the round functions we can rework them as a
> wrapper around it.

Missing "of" in the function names here.
s/a wrapper/wrappers/

IIUC the point of this is that roundup_pow_of_two() returned
"unsigned long", which can be either 32 or 64 bits (worth pointing
out, I think), and many callers need something that returns
"unsigned long long" (always 64 bits).

It's a nice simplification to remove the "__" variants.  Just as a
casual reader of this commit message, I'd like to know why we had both
the roundup and the __roundup versions in the first place, and why we
no longer need both.

> -#define roundup_pow_of_two(n)\
> -(\
> - __builtin_constant_p(n) ? ( \
> - (n == 1) ? 1 :  \
> - (1UL << (ilog2((n) - 1) + 1))   \
> -) :  \
> - __roundup_pow_of_two(n) \
> - )
> +#define roundup_pow_of_two(n)  \
> +(  \
> + (__builtin_constant_p(n) && ((n) == 1)) ? \
> + 1 : (1ULL << (ilog2((n) - 1) + 1))\
> +)

Should the resulting type of this expression always be a ULL, even
when n==1, i.e., should it be this?

  1ULL : (1ULL << (ilog2((n) - 1) + 1))\

Or maybe there's no case where that makes a difference?

Bjorn
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-05 Thread Robin Murphy 

On 03/12/2019 11:47 am, Nicolas Saenz Julienne wrote:

Some users need to make sure their rounding function accepts and returns
64bit long variables regardless of the architecture. Sadly
roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
out ilog2() already handles 32/64bit calculations properly, and being
the building block to the round functions we can rework them as a
wrapper around it.


Neat! Although all the additional ULL casts this introduces seem 
somewhat unwelcome - I suppose the (1ULL << (ilog2(n))) makes it 
effectively always return unsigned long long now. Might it make sense to 
cast the return value to typeof(n) to avoid this slightly non-obvious 
behaviour (and the associated churn)?


Robin.


Suggested-by: Robin Murphy 
Signed-off-by: Nicolas Saenz Julienne 
---
  drivers/clk/clk-divider.c|  8 ++--
  drivers/clk/sunxi/clk-sunxi.c|  2 +-
  drivers/infiniband/hw/hfi1/chip.c|  4 +-
  drivers/infiniband/hw/hfi1/init.c|  4 +-
  drivers/infiniband/hw/mlx4/srq.c |  2 +-
  drivers/infiniband/hw/mthca/mthca_srq.c  |  2 +-
  drivers/infiniband/sw/rxe/rxe_qp.c   |  4 +-
  drivers/iommu/intel-iommu.c  |  4 +-
  drivers/iommu/intel-svm.c|  4 +-
  drivers/iommu/intel_irq_remapping.c  |  2 +-
  drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |  4 +-
  drivers/net/ethernet/marvell/sky2.c  |  2 +-
  drivers/net/ethernet/rocker/rocker_hw.h  |  4 +-
  drivers/net/ethernet/sfc/ef10.c  |  2 +-
  drivers/net/ethernet/sfc/efx.h   |  2 +-
  drivers/net/ethernet/sfc/falcon/efx.h|  2 +-
  drivers/pci/msi.c|  2 +-
  include/linux/log2.h | 44 +---
  kernel/kexec_core.c  |  3 +-
  lib/rhashtable.c |  2 +-
  net/sunrpc/xprtrdma/verbs.c  |  2 +-
  21 files changed, 41 insertions(+), 64 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 098b2b01f0af..ba947e4c8193 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -222,7 +222,7 @@ static int _div_round_up(const struct clk_div_table *table,
int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
  
  	if (flags & CLK_DIVIDER_POWER_OF_TWO)

-   div = __roundup_pow_of_two(div);
+   div = roundup_pow_of_two(div);
if (table)
div = _round_up_table(table, div);
  
@@ -240,8 +240,8 @@ static int _div_round_closest(const struct clk_div_table *table,

down = parent_rate / rate;
  
  	if (flags & CLK_DIVIDER_POWER_OF_TWO) {

-   up = __roundup_pow_of_two(up);
-   down = __rounddown_pow_of_two(down);
+   up = roundup_pow_of_two(up);
+   down = rounddown_pow_of_two(down);
} else if (table) {
up = _round_up_table(table, up);
down = _round_down_table(table, down);
@@ -278,7 +278,7 @@ static int _next_div(const struct clk_div_table *table, int 
div,
div++;
  
  	if (flags & CLK_DIVIDER_POWER_OF_TWO)

-   return __roundup_pow_of_two(div);
+   return roundup_pow_of_two(div);
if (table)
return _round_up_table(table, div);
  
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c

index 27201fd26e44..faec99dc09c0 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -311,7 +311,7 @@ static void sun6i_get_ahb1_factors(struct factors_request 
*req)
  
  		calcm = DIV_ROUND_UP(div, 1 << calcp);

} else {
-   calcp = __roundup_pow_of_two(div);
+   calcp = roundup_pow_of_two(div);
calcp = calcp > 3 ? 3 : calcp;
}
  
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c

index 9b1fb84a3d45..96b1d343c32f 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -14199,10 +14199,10 @@ static int qos_rmt_entries(struct hfi1_devdata *dd, 
unsigned int *mp,
max_by_vl = krcvqs[i];
if (max_by_vl > 32)
goto no_qos;
-   m = ilog2(__roundup_pow_of_two(max_by_vl));
+   m = ilog2(roundup_pow_of_two(max_by_vl));
  
  	/* determine bits for vl */

-   n = ilog2(__roundup_pow_of_two(num_vls));
+   n = ilog2(roundup_pow_of_two(num_vls));
  
  	/* reject if too much is used */

if ((m + n) > 7)
diff --git a/drivers/infiniband/hw/hfi1/init.c 
b/drivers/infiniband/hw/hfi1/init.c
index 26b792bb1027..838c789c7cce 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -467,7 +467,7 @@ int hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, int 
numa,
 * MTU supported.
 */
if (rcd->egrbufs.size < hfi1_max_mtu) {
-

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-05 Thread Leon Romanovsky 
On Tue, Dec 03, 2019 at 12:47:40PM +0100, Nicolas Saenz Julienne wrote:
> Some users need to make sure their rounding function accepts and returns
> 64bit long variables regardless of the architecture. Sadly
> roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
> out ilog2() already handles 32/64bit calculations properly, and being
> the building block to the round functions we can rework them as a
> wrapper around it.
>
> Suggested-by: Robin Murphy 
> Signed-off-by: Nicolas Saenz Julienne 
> ---
>  drivers/clk/clk-divider.c|  8 ++--
>  drivers/clk/sunxi/clk-sunxi.c|  2 +-
>  drivers/infiniband/hw/hfi1/chip.c|  4 +-
>  drivers/infiniband/hw/hfi1/init.c|  4 +-
>  drivers/infiniband/hw/mlx4/srq.c |  2 +-
>  drivers/infiniband/hw/mthca/mthca_srq.c  |  2 +-
>  drivers/infiniband/sw/rxe/rxe_qp.c   |  4 +-


Thanks, for infiniband.
Reviewed-by: Leon Romanovsky 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-04 Thread Lu Baolu 

Hi,

On 12/3/19 7:47 PM, Nicolas Saenz Julienne wrote:

Some users need to make sure their rounding function accepts and returns
64bit long variables regardless of the architecture. Sadly
roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
out ilog2() already handles 32/64bit calculations properly, and being
the building block to the round functions we can rework them as a
wrapper around it.

Suggested-by: Robin Murphy 
Signed-off-by: Nicolas Saenz Julienne 
---
  drivers/clk/clk-divider.c|  8 ++--
  drivers/clk/sunxi/clk-sunxi.c|  2 +-
  drivers/infiniband/hw/hfi1/chip.c|  4 +-
  drivers/infiniband/hw/hfi1/init.c|  4 +-
  drivers/infiniband/hw/mlx4/srq.c |  2 +-
  drivers/infiniband/hw/mthca/mthca_srq.c  |  2 +-
  drivers/infiniband/sw/rxe/rxe_qp.c   |  4 +-
  drivers/iommu/intel-iommu.c  |  4 +-
  drivers/iommu/intel-svm.c|  4 +-
  drivers/iommu/intel_irq_remapping.c  |  2 +-


For changes in drivers/iommu/intel*.c,

Reviewed-by: Lu Baolu 

Best regards,
baolu


  drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |  4 +-
  drivers/net/ethernet/marvell/sky2.c  |  2 +-
  drivers/net/ethernet/rocker/rocker_hw.h  |  4 +-
  drivers/net/ethernet/sfc/ef10.c  |  2 +-
  drivers/net/ethernet/sfc/efx.h   |  2 +-
  drivers/net/ethernet/sfc/falcon/efx.h|  2 +-
  drivers/pci/msi.c|  2 +-
  include/linux/log2.h | 44 +---
  kernel/kexec_core.c  |  3 +-
  lib/rhashtable.c |  2 +-
  net/sunrpc/xprtrdma/verbs.c  |  2 +-
  21 files changed, 41 insertions(+), 64 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 098b2b01f0af..ba947e4c8193 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -222,7 +222,7 @@ static int _div_round_up(const struct clk_div_table *table,
int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
  
  	if (flags & CLK_DIVIDER_POWER_OF_TWO)

-   div = __roundup_pow_of_two(div);
+   div = roundup_pow_of_two(div);
if (table)
div = _round_up_table(table, div);
  
@@ -240,8 +240,8 @@ static int _div_round_closest(const struct clk_div_table *table,

down = parent_rate / rate;
  
  	if (flags & CLK_DIVIDER_POWER_OF_TWO) {

-   up = __roundup_pow_of_two(up);
-   down = __rounddown_pow_of_two(down);
+   up = roundup_pow_of_two(up);
+   down = rounddown_pow_of_two(down);
} else if (table) {
up = _round_up_table(table, up);
down = _round_down_table(table, down);
@@ -278,7 +278,7 @@ static int _next_div(const struct clk_div_table *table, int 
div,
div++;
  
  	if (flags & CLK_DIVIDER_POWER_OF_TWO)

-   return __roundup_pow_of_two(div);
+   return roundup_pow_of_two(div);
if (table)
return _round_up_table(table, div);
  
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c

index 27201fd26e44..faec99dc09c0 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -311,7 +311,7 @@ static void sun6i_get_ahb1_factors(struct factors_request 
*req)
  
  		calcm = DIV_ROUND_UP(div, 1 << calcp);

} else {
-   calcp = __roundup_pow_of_two(div);
+   calcp = roundup_pow_of_two(div);
calcp = calcp > 3 ? 3 : calcp;
}
  
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c

index 9b1fb84a3d45..96b1d343c32f 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -14199,10 +14199,10 @@ static int qos_rmt_entries(struct hfi1_devdata *dd, 
unsigned int *mp,
max_by_vl = krcvqs[i];
if (max_by_vl > 32)
goto no_qos;
-   m = ilog2(__roundup_pow_of_two(max_by_vl));
+   m = ilog2(roundup_pow_of_two(max_by_vl));
  
  	/* determine bits for vl */

-   n = ilog2(__roundup_pow_of_two(num_vls));
+   n = ilog2(roundup_pow_of_two(num_vls));
  
  	/* reject if too much is used */

if ((m + n) > 7)
diff --git a/drivers/infiniband/hw/hfi1/init.c 
b/drivers/infiniband/hw/hfi1/init.c
index 26b792bb1027..838c789c7cce 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -467,7 +467,7 @@ int hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, int 
numa,
 * MTU supported.
 */
if (rcd->egrbufs.size < hfi1_max_mtu) {
-   rcd->egrbufs.size = __roundup_pow_of_two(hfi1_max_mtu);
+   rcd->egrbufs.size = roundup_pow_of_two(hfi1_max_mtu);
hfi1_cdbg(PROC,
  "ctxt%u: eager bufs

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-04 Thread Martin Habets 
For the changes under drivers/net/ethernet/sfc:
Reviewed-by: Martin Habets 

On 03/12/2019 11:47, Nicolas Saenz Julienne wrote:
> Some users need to make sure their rounding function accepts and returns
> 64bit long variables regardless of the architecture. Sadly
> roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
> out ilog2() already handles 32/64bit calculations properly, and being
> the building block to the round functions we can rework them as a
> wrapper around it.
> 
> Suggested-by: Robin Murphy 
> Signed-off-by: Nicolas Saenz Julienne 
> ---
>  drivers/clk/clk-divider.c|  8 ++--
>  drivers/clk/sunxi/clk-sunxi.c|  2 +-
>  drivers/infiniband/hw/hfi1/chip.c|  4 +-
>  drivers/infiniband/hw/hfi1/init.c|  4 +-
>  drivers/infiniband/hw/mlx4/srq.c |  2 +-
>  drivers/infiniband/hw/mthca/mthca_srq.c  |  2 +-
>  drivers/infiniband/sw/rxe/rxe_qp.c   |  4 +-
>  drivers/iommu/intel-iommu.c  |  4 +-
>  drivers/iommu/intel-svm.c|  4 +-
>  drivers/iommu/intel_irq_remapping.c  |  2 +-
>  drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |  4 +-
>  drivers/net/ethernet/marvell/sky2.c  |  2 +-
>  drivers/net/ethernet/rocker/rocker_hw.h  |  4 +-
>  drivers/net/ethernet/sfc/ef10.c  |  2 +-
>  drivers/net/ethernet/sfc/efx.h   |  2 +-
>  drivers/net/ethernet/sfc/falcon/efx.h|  2 +-
>  drivers/pci/msi.c|  2 +-
>  include/linux/log2.h | 44 +---
>  kernel/kexec_core.c  |  3 +-
>  lib/rhashtable.c |  2 +-
>  net/sunrpc/xprtrdma/verbs.c  |  2 +-
>  21 files changed, 41 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 098b2b01f0af..ba947e4c8193 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -222,7 +222,7 @@ static int _div_round_up(const struct clk_div_table 
> *table,
>   int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
>  
>   if (flags & CLK_DIVIDER_POWER_OF_TWO)
> - div = __roundup_pow_of_two(div);
> + div = roundup_pow_of_two(div);
>   if (table)
>   div = _round_up_table(table, div);
>  
> @@ -240,8 +240,8 @@ static int _div_round_closest(const struct clk_div_table 
> *table,
>   down = parent_rate / rate;
>  
>   if (flags & CLK_DIVIDER_POWER_OF_TWO) {
> - up = __roundup_pow_of_two(up);
> - down = __rounddown_pow_of_two(down);
> + up = roundup_pow_of_two(up);
> + down = rounddown_pow_of_two(down);
>   } else if (table) {
>   up = _round_up_table(table, up);
>   down = _round_down_table(table, down);
> @@ -278,7 +278,7 @@ static int _next_div(const struct clk_div_table *table, 
> int div,
>   div++;
>  
>   if (flags & CLK_DIVIDER_POWER_OF_TWO)
> - return __roundup_pow_of_two(div);
> + return roundup_pow_of_two(div);
>   if (table)
>   return _round_up_table(table, div);
>  
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 27201fd26e44..faec99dc09c0 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -311,7 +311,7 @@ static void sun6i_get_ahb1_factors(struct factors_request 
> *req)
>  
>   calcm = DIV_ROUND_UP(div, 1 << calcp);
>   } else {
> - calcp = __roundup_pow_of_two(div);
> + calcp = roundup_pow_of_two(div);
>   calcp = calcp > 3 ? 3 : calcp;
>   }
>  
> diff --git a/drivers/infiniband/hw/hfi1/chip.c 
> b/drivers/infiniband/hw/hfi1/chip.c
> index 9b1fb84a3d45..96b1d343c32f 100644
> --- a/drivers/infiniband/hw/hfi1/chip.c
> +++ b/drivers/infiniband/hw/hfi1/chip.c
> @@ -14199,10 +14199,10 @@ static int qos_rmt_entries(struct hfi1_devdata *dd, 
> unsigned int *mp,
>   max_by_vl = krcvqs[i];
>   if (max_by_vl > 32)
>   goto no_qos;
> - m = ilog2(__roundup_pow_of_two(max_by_vl));
> + m = ilog2(roundup_pow_of_two(max_by_vl));
>  
>   /* determine bits for vl */
> - n = ilog2(__roundup_pow_of_two(num_vls));
> + n = ilog2(roundup_pow_of_two(num_vls));
>  
>   /* reject if too much is used */
>   if ((m + n) > 7)
> diff --git a/drivers/infiniband/hw/hfi1/init.c 
> b/drivers/infiniband/hw/hfi1/init.c
> index 26b792bb1027..838c789c7cce 100644
> --- a/drivers/infiniband/hw/hfi1/init.c
> +++ b/drivers/infiniband/hw/hfi1/init.c
> @@ -467,7 +467,7 @@ int hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, int 
> numa,
>* MTU supported.
>*/
>   if (rcd->egrbufs.size < hfi1_max_mtu) {
> - rcd->egrbufs.size = __roundup_pow_of_two(hfi1_max_mtu);
> + rcd->egrbufs.size =

Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-03 Thread Chuck Lever 



> On Dec 3, 2019, at 6:47 AM, Nicolas Saenz Julienne  
> wrote:
> 
> Some users need to make sure their rounding function accepts and returns
> 64bit long variables regardless of the architecture. Sadly
> roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
> out ilog2() already handles 32/64bit calculations properly, and being
> the building block to the round functions we can rework them as a
> wrapper around it.
> 
> Suggested-by: Robin Murphy 
> Signed-off-by: Nicolas Saenz Julienne 
> ---
> drivers/clk/clk-divider.c|  8 ++--
> drivers/clk/sunxi/clk-sunxi.c|  2 +-
> drivers/infiniband/hw/hfi1/chip.c|  4 +-
> drivers/infiniband/hw/hfi1/init.c|  4 +-
> drivers/infiniband/hw/mlx4/srq.c |  2 +-
> drivers/infiniband/hw/mthca/mthca_srq.c  |  2 +-
> drivers/infiniband/sw/rxe/rxe_qp.c   |  4 +-
> drivers/iommu/intel-iommu.c  |  4 +-
> drivers/iommu/intel-svm.c|  4 +-
> drivers/iommu/intel_irq_remapping.c  |  2 +-
> drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |  4 +-
> drivers/net/ethernet/marvell/sky2.c  |  2 +-
> drivers/net/ethernet/rocker/rocker_hw.h  |  4 +-
> drivers/net/ethernet/sfc/ef10.c  |  2 +-
> drivers/net/ethernet/sfc/efx.h   |  2 +-
> drivers/net/ethernet/sfc/falcon/efx.h|  2 +-
> drivers/pci/msi.c|  2 +-
> include/linux/log2.h | 44 +---
> kernel/kexec_core.c  |  3 +-
> lib/rhashtable.c |  2 +-
> net/sunrpc/xprtrdma/verbs.c  |  2 +-
> 21 files changed, 41 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 098b2b01f0af..ba947e4c8193 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -222,7 +222,7 @@ static int _div_round_up(const struct clk_div_table 
> *table,
>   int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
> 
>   if (flags & CLK_DIVIDER_POWER_OF_TWO)
> - div = __roundup_pow_of_two(div);
> + div = roundup_pow_of_two(div);
>   if (table)
>   div = _round_up_table(table, div);
> 
> @@ -240,8 +240,8 @@ static int _div_round_closest(const struct clk_div_table 
> *table,
>   down = parent_rate / rate;
> 
>   if (flags & CLK_DIVIDER_POWER_OF_TWO) {
> - up = __roundup_pow_of_two(up);
> - down = __rounddown_pow_of_two(down);
> + up = roundup_pow_of_two(up);
> + down = rounddown_pow_of_two(down);
>   } else if (table) {
>   up = _round_up_table(table, up);
>   down = _round_down_table(table, down);
> @@ -278,7 +278,7 @@ static int _next_div(const struct clk_div_table *table, 
> int div,
>   div++;
> 
>   if (flags & CLK_DIVIDER_POWER_OF_TWO)
> - return __roundup_pow_of_two(div);
> + return roundup_pow_of_two(div);
>   if (table)
>   return _round_up_table(table, div);
> 
> diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
> index 27201fd26e44..faec99dc09c0 100644
> --- a/drivers/clk/sunxi/clk-sunxi.c
> +++ b/drivers/clk/sunxi/clk-sunxi.c
> @@ -311,7 +311,7 @@ static void sun6i_get_ahb1_factors(struct factors_request 
> *req)
> 
>   calcm = DIV_ROUND_UP(div, 1 << calcp);
>   } else {
> - calcp = __roundup_pow_of_two(div);
> + calcp = roundup_pow_of_two(div);
>   calcp = calcp > 3 ? 3 : calcp;
>   }
> 
> diff --git a/drivers/infiniband/hw/hfi1/chip.c 
> b/drivers/infiniband/hw/hfi1/chip.c
> index 9b1fb84a3d45..96b1d343c32f 100644
> --- a/drivers/infiniband/hw/hfi1/chip.c
> +++ b/drivers/infiniband/hw/hfi1/chip.c
> @@ -14199,10 +14199,10 @@ static int qos_rmt_entries(struct hfi1_devdata *dd, 
> unsigned int *mp,
>   max_by_vl = krcvqs[i];
>   if (max_by_vl > 32)
>   goto no_qos;
> - m = ilog2(__roundup_pow_of_two(max_by_vl));
> + m = ilog2(roundup_pow_of_two(max_by_vl));
> 
>   /* determine bits for vl */
> - n = ilog2(__roundup_pow_of_two(num_vls));
> + n = ilog2(roundup_pow_of_two(num_vls));
> 
>   /* reject if too much is used */
>   if ((m + n) > 7)
> diff --git a/drivers/infiniband/hw/hfi1/init.c 
> b/drivers/infiniband/hw/hfi1/init.c
> index 26b792bb1027..838c789c7cce 100644
> --- a/drivers/infiniband/hw/hfi1/init.c
> +++ b/drivers/infiniband/hw/hfi1/init.c
> @@ -467,7 +467,7 @@ int hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, int 
> numa,
>* MTU supported.
>*/
>   if (rcd->egrbufs.size < hfi1_max_mtu) {
> - rcd->egrbufs.size = __roundup_pow_of_two(hfi1_max_mtu);
> + rcd->egrbufs.size = roundup_pow_of_two(hfi1_max_mtu);
>   hfi1_cdbg(PROC,
>

[PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()

2019-12-03 Thread Nicolas Saenz Julienne 
Some users need to make sure their rounding function accepts and returns
64bit long variables regardless of the architecture. Sadly
roundup/rounddown_pow_two() takes and returns unsigned longs. It turns
out ilog2() already handles 32/64bit calculations properly, and being
the building block to the round functions we can rework them as a
wrapper around it.

Suggested-by: Robin Murphy 
Signed-off-by: Nicolas Saenz Julienne 
---
 drivers/clk/clk-divider.c|  8 ++--
 drivers/clk/sunxi/clk-sunxi.c|  2 +-
 drivers/infiniband/hw/hfi1/chip.c|  4 +-
 drivers/infiniband/hw/hfi1/init.c|  4 +-
 drivers/infiniband/hw/mlx4/srq.c |  2 +-
 drivers/infiniband/hw/mthca/mthca_srq.c  |  2 +-
 drivers/infiniband/sw/rxe/rxe_qp.c   |  4 +-
 drivers/iommu/intel-iommu.c  |  4 +-
 drivers/iommu/intel-svm.c|  4 +-
 drivers/iommu/intel_irq_remapping.c  |  2 +-
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c |  4 +-
 drivers/net/ethernet/marvell/sky2.c  |  2 +-
 drivers/net/ethernet/rocker/rocker_hw.h  |  4 +-
 drivers/net/ethernet/sfc/ef10.c  |  2 +-
 drivers/net/ethernet/sfc/efx.h   |  2 +-
 drivers/net/ethernet/sfc/falcon/efx.h|  2 +-
 drivers/pci/msi.c|  2 +-
 include/linux/log2.h | 44 +---
 kernel/kexec_core.c  |  3 +-
 lib/rhashtable.c |  2 +-
 net/sunrpc/xprtrdma/verbs.c  |  2 +-
 21 files changed, 41 insertions(+), 64 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 098b2b01f0af..ba947e4c8193 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -222,7 +222,7 @@ static int _div_round_up(const struct clk_div_table *table,
int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate);
 
if (flags & CLK_DIVIDER_POWER_OF_TWO)
-   div = __roundup_pow_of_two(div);
+   div = roundup_pow_of_two(div);
if (table)
div = _round_up_table(table, div);
 
@@ -240,8 +240,8 @@ static int _div_round_closest(const struct clk_div_table 
*table,
down = parent_rate / rate;
 
if (flags & CLK_DIVIDER_POWER_OF_TWO) {
-   up = __roundup_pow_of_two(up);
-   down = __rounddown_pow_of_two(down);
+   up = roundup_pow_of_two(up);
+   down = rounddown_pow_of_two(down);
} else if (table) {
up = _round_up_table(table, up);
down = _round_down_table(table, down);
@@ -278,7 +278,7 @@ static int _next_div(const struct clk_div_table *table, int 
div,
div++;
 
if (flags & CLK_DIVIDER_POWER_OF_TWO)
-   return __roundup_pow_of_two(div);
+   return roundup_pow_of_two(div);
if (table)
return _round_up_table(table, div);
 
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 27201fd26e44..faec99dc09c0 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -311,7 +311,7 @@ static void sun6i_get_ahb1_factors(struct factors_request 
*req)
 
calcm = DIV_ROUND_UP(div, 1 << calcp);
} else {
-   calcp = __roundup_pow_of_two(div);
+   calcp = roundup_pow_of_two(div);
calcp = calcp > 3 ? 3 : calcp;
}
 
diff --git a/drivers/infiniband/hw/hfi1/chip.c 
b/drivers/infiniband/hw/hfi1/chip.c
index 9b1fb84a3d45..96b1d343c32f 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -14199,10 +14199,10 @@ static int qos_rmt_entries(struct hfi1_devdata *dd, 
unsigned int *mp,
max_by_vl = krcvqs[i];
if (max_by_vl > 32)
goto no_qos;
-   m = ilog2(__roundup_pow_of_two(max_by_vl));
+   m = ilog2(roundup_pow_of_two(max_by_vl));
 
/* determine bits for vl */
-   n = ilog2(__roundup_pow_of_two(num_vls));
+   n = ilog2(roundup_pow_of_two(num_vls));
 
/* reject if too much is used */
if ((m + n) > 7)
diff --git a/drivers/infiniband/hw/hfi1/init.c 
b/drivers/infiniband/hw/hfi1/init.c
index 26b792bb1027..838c789c7cce 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -467,7 +467,7 @@ int hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, int 
numa,
 * MTU supported.
 */
if (rcd->egrbufs.size < hfi1_max_mtu) {
-   rcd->egrbufs.size = __roundup_pow_of_two(hfi1_max_mtu);
+   rcd->egrbufs.size = roundup_pow_of_two(hfi1_max_mtu);
hfi1_cdbg(PROC,
  "ctxt%u: eager bufs size too small. Adjusting 
to %u\n",
rcd->ctxt, rcd->egrbufs.size);
@@ -1959,7 +1959,7 @@ int