| Message ID | 1472188577-14550-1-git-send-email-christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Acked-by: Majd Dibbiny <majd@mellanox.com> > On Aug 26, 2016, at 8:25 AM, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > The 2nd parameter of 'find_first_bit' is the number of bits to search. > In this case, we are passing 'sizeof(tmp)' which is likely to be 4 or 8 > because 'tmp' is an 'unsigned long'. > > It is likely that the number of bits of 'tmp' was expected here. So use > BITS_PER_LONG instead. > > It has been spotted by the following coccinelle script: > @@ > expression ret, x; > > @@ > * ret = \(find_first_bit \| find_first_zero_bit\) (x, sizeof(...)); > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > __ffs could be used to reduce code verbosity > --- > drivers/infiniband/hw/mlx5/mem.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/hw/mlx5/mem.c b/drivers/infiniband/hw/mlx5/mem.c > index 40df2cca0609..996b54e366b0 100644 > --- a/drivers/infiniband/hw/mlx5/mem.c > +++ b/drivers/infiniband/hw/mlx5/mem.c > @@ -71,7 +71,7 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr, int *count, int *shift, > > addr = addr >> page_shift; > tmp = (unsigned long)addr; > - m = find_first_bit(&tmp, sizeof(tmp)); > + m = find_first_bit(&tmp, BITS_PER_LONG); > skip = 1 << m; > mask = skip - 1; > i = 0; > @@ -81,7 +81,7 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr, int *count, int *shift, > for (k = 0; k < len; k++) { > if (!(i & mask)) { > tmp = (unsigned long)pfn; > - m = min_t(unsigned long, m, find_first_bit(&tmp, sizeof(tmp))); > + m = min_t(unsigned long, m, find_first_bit(&tmp, BITS_PER_LONG)); > skip = 1 << m; > mask = skip - 1; > base = pfn; > @@ -89,7 +89,7 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr, int *count, int *shift, > } else { > if (base + p != pfn) { > tmp = (unsigned long)p; > - m = find_first_bit(&tmp, sizeof(tmp)); > + m = find_first_bit(&tmp, BITS_PER_LONG); > skip = 1 << m; > mask = skip - 1; > base = pfn; > -- > 2.7.4 > > > --- > L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast. > https://www.avast.com/antivirus > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 26, 2016 at 07:16:17AM +0200, Christophe JAILLET wrote: > The 2nd parameter of 'find_first_bit' is the number of bits to search. > In this case, we are passing 'sizeof(tmp)' which is likely to be 4 or 8 > because 'tmp' is an 'unsigned long'. > > It is likely that the number of bits of 'tmp' was expected here. So use > BITS_PER_LONG instead. > > It has been spotted by the following coccinelle script: > @@ > expression ret, x; > > @@ > * ret = \(find_first_bit \| find_first_zero_bit\) (x, sizeof(...)); > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Thanks, Acked-by: Leon Romanovsky <leonro@mellanox.com>
On 8/26/2016 1:16 AM, Christophe JAILLET wrote: > The 2nd parameter of 'find_first_bit' is the number of bits to search. > In this case, we are passing 'sizeof(tmp)' which is likely to be 4 or 8 > because 'tmp' is an 'unsigned long'. > > It is likely that the number of bits of 'tmp' was expected here. So use > BITS_PER_LONG instead. Thanks, applied.
diff --git a/drivers/infiniband/hw/mlx5/mem.c b/drivers/infiniband/hw/mlx5/mem.c index 40df2cca0609..996b54e366b0 100644 --- a/drivers/infiniband/hw/mlx5/mem.c +++ b/drivers/infiniband/hw/mlx5/mem.c @@ -71,7 +71,7 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr, int *count, int *shift, addr = addr >> page_shift; tmp = (unsigned long)addr; - m = find_first_bit(&tmp, sizeof(tmp)); + m = find_first_bit(&tmp, BITS_PER_LONG); skip = 1 << m; mask = skip - 1; i = 0; @@ -81,7 +81,7 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr, int *count, int *shift, for (k = 0; k < len; k++) { if (!(i & mask)) { tmp = (unsigned long)pfn; - m = min_t(unsigned long, m, find_first_bit(&tmp, sizeof(tmp))); + m = min_t(unsigned long, m, find_first_bit(&tmp, BITS_PER_LONG)); skip = 1 << m; mask = skip - 1; base = pfn; @@ -89,7 +89,7 @@ void mlx5_ib_cont_pages(struct ib_umem *umem, u64 addr, int *count, int *shift, } else { if (base + p != pfn) { tmp = (unsigned long)p; - m = find_first_bit(&tmp, sizeof(tmp)); + m = find_first_bit(&tmp, BITS_PER_LONG); skip = 1 << m; mask = skip - 1; base = pfn;
The 2nd parameter of 'find_first_bit' is the number of bits to search. In this case, we are passing 'sizeof(tmp)' which is likely to be 4 or 8 because 'tmp' is an 'unsigned long'. It is likely that the number of bits of 'tmp' was expected here. So use BITS_PER_LONG instead. It has been spotted by the following coccinelle script: @@ expression ret, x; @@ * ret = \(find_first_bit \| find_first_zero_bit\) (x, sizeof(...)); Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- __ffs could be used to reduce code verbosity --- drivers/infiniband/hw/mlx5/mem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)