Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
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 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
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 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
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 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
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()
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 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
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 siz
Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
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 = roun
Re: [PATCH v4 7/8] linux/log2.h: Fix 64bit calculations in roundup/down_pow_two()
> 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()
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 hfi1_setup_eagerbuf