[openib-general] [PATCH] huge pages support

[Roland Dreier]

My first reaction on reading this is that it looks like we need some
more help from mm/ to make this cleaner.

 > +			for (j = 0; j < umem->page_size / PAGE_SIZE; ++j)
 > +				put_page(chunk->page_list[i].page);

This just looks really bad... you're assuming that calling put_page()
on a different page than the one get_user_pages() gave you is OK.
Does that actually work?  (I don't remember the details of how hugetlb
pages work)

 > +	if (hpage == -1)
 > +		hpage = is_vm_hugetlb_page(vma);
 > +	else
 > +		if (hpage != is_vm_hugetlb_page(vma))
 > +			return -ENOMEM;

Users aren't allowed to register regions that span both hugetlb and
non-hugetlb pages??  Why not?

 > +	if (!(PAGE_SIZE / (sizeof (struct page *) << shift_diff))) {

shift_diff is used here...

 > +		ret = -ENOMEM;
 > +		goto exit_up;
 > +	}
 > +
 > +	page_size = 1 << page_shift;
 > +	page_mask = ~(page_size - 1);
 > +	shift_diff = page_shift - PAGE_SHIFT;

and set down here... don't you get a warning about "is used uninitialized"?

Also, if someone tries to register a hugetlb region and a regular page
isn't big enough to hold all the pointers, then they just lose??  This
would be a real problem if/when GB pages are implemented on amd64 --
there would be no way to register such a region.


Basically this patch seems to be trying to undo what follow_hugetlb_pages()
does to create normal pages for the pieces of a hugetlb page.  It
seems like it would be better to have a new function like get_user_pages()
that returned its result in a struct scatterlist so that it could give
back huge pages directly.

 - R.