On Mon, Dec 24, 2018 at 04:32:23PM -0600, Shiraz Saleem wrote:
> This helper iterates through the SG list to find the best page size to use
> from a bitmap of HW supported page sizes. Drivers that support multiple
> page sizes, but not mixed pages in an MR can use this API.
>
> Suggested-by: Jason Gunthorpe <jgg@xxxxxxxx>
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl@xxxxxxxxx>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx>
> drivers/infiniband/core/umem.c | 86 ++++++++++++++++++++++++++++++++++++++++++
> include/rdma/ib_umem.h | 9 +++++
> 2 files changed, 95 insertions(+)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 64bacc5..b2f2d75 100644
> +++ b/drivers/infiniband/core/umem.c
> @@ -119,6 +119,92 @@ static void ib_umem_add_sg_table(struct scatterlist **cur,
> }
>
> /**
> + * ib_umem_find_pg_bit - Find the page bit to use for phyaddr
> + *
> + * @phyaddr: Physical address after DMA translation
> + * @supported_pgsz: bitmask of HW supported page sizes
> + */
> +static unsigned int ib_umem_find_pg_bit(unsigned long phyaddr,
> + unsigned long supported_pgsz)
> +{
> + unsigned long num_zeroes;
> + unsigned long pgsz;
> +
> + /* Trailing zero bits in the address */
> + num_zeroes = __ffs(phyaddr);
> +
> + /* Find page bit such that phyaddr is aligned to the highest supported
> + * HW page size
> + */
> + pgsz = supported_pgsz & (BIT_ULL(num_zeroes + 1) - 1);
> + if (!pgsz)
> + return __ffs(supported_pgsz);
> +
> + return (fls64(pgsz) - 1);
extra brackets
> +}
> +
> +/**
> + * ib_umem_find_single_pg_size - Find best HW page size to use for this MR
> + * @umem: umem struct
> + * @supported_pgsz: bitmask of HW supported page sizes
> + * @uvirt_addr: user-space virtual MR base address
> + *
> + * This helper is intended for HW that support multiple page
> + * sizes but can do only a single page size in an MR.
Returns 0 if the umem requires page sizes not supported by the driver
to be mapped. Drivers always supporting PAGE_SIZE will never see a 0
result.
> + */
> +unsigned long ib_umem_find_single_pg_size(struct ib_umem *umem,
> + unsigned long supported_pgsz,
> + unsigned long uvirt_addr)
> +{
> + struct scatterlist *sg;
> + unsigned int pg_bit_sg, min_pg_bit, best_pg_bit;
> + int i;
> +
> + if (!supported_pgsz)
> + return 0;
Maybe like:
if (WARN_ON(!(BIT_ULL(PAGE_SHIFT) & supported_pgsz)))
return 0;
?
Drivers are not allowed to not support the system page size.
And also I guess it would be reasonable to have something like
/* Driver only supports the system page size, so no further checks required */
if (supported_pgsz >> PAGE_SHIFT == 1)
return PAGE_SIZE;
Otherwise I think this looks fine
Jason
