[ewg] Re: [PATCH v1] mlx4_ib: Optimize hugetlab pages support

[Roland Dreier]

OK, looks better.  However the patch had a bunch of whitespace problems
(run checkpatch.pl to see them).  Also:

 > +static int handle_hugetlb_user_mr(struct ib_pd *pd, struct mlx4_ib_mr *mr,
 > +				  u64 virt_addr, int access_flags)
 > +{
 > +#ifdef CONFIG_HUGETLB_PAGE
 > +	struct mlx4_ib_dev *dev = to_mdev(pd->device);
 > +	struct ib_umem_chunk *chunk;
 > +	unsigned dsize;
 > +	dma_addr_t daddr;
 > +	unsigned uninitialized_var(cur_size);
 > +	dma_addr_t uninitialized_var(cur_addr);
 > +	int restart;
 > +	int n;
 > +	struct ib_umem	*umem = mr->umem;
 > +	u64 *arr;
 > +	int err = 0;
 > +	int i;
 > +	int j = 0;
 > +
 > +        n = PAGE_ALIGN(umem->length + umem->offset) >> HPAGE_SHIFT;

seems this might underestimate by 1 if the region doesn't start/end on a
huge-page aligned boundary (but we would still want to use big pages to
register it).

 > +	arr = kmalloc(n * sizeof *arr, GFP_KERNEL);
 > +	if (!arr)
 > +		return -ENOMEM;
 > +
 > +	restart = 1;
 > +	list_for_each_entry(chunk, &umem->chunk_list, list)
 > +		for (i = 0; i < chunk->nmap; ++i) {
 > +			daddr = sg_dma_address(&chunk->page_list[i]);
 > +			dsize = sg_dma_len(&chunk->page_list[i]);
 > +			if (restart) {
 > +				cur_addr = daddr;
 > +				cur_size = dsize;
 > +				restart = 0;
 > +			} else if (cur_addr + cur_size != daddr) {
 > +				err = -EINVAL;
 > +				goto out;
 > +			} else
 > +                                cur_size += dsize;
 > +
 > +			if (cur_size > HPAGE_SIZE) {
 > +				err = -EINVAL;
 > +				goto out;
 > +			} else if (cur_size == HPAGE_SIZE) {
 > +				restart = 1;
 > +				arr[j++] = cur_addr;
 > +			}
 > +		}

I think we could avoid the uninitialized_var() stuff and having restart
at all by just doing cur_size = 0 at the start of the loop, and then
instead of if (restart) just test if cur_size is 0.

 - R.