On Tue, Sep 15, 2020 at 11:42 PM Matthew Wilcox [off-list ref] wrote:

On Tue, Sep 15, 2020 at 11:28:01PM +0800, Muchun Song wrote:

quoted

On Tue, Sep 15, 2020 at 10:32 PM Matthew Wilcox [off-list ref] wrote:

quoted

On Tue, Sep 15, 2020 at 08:59:23PM +0800, Muchun Song wrote:

quoted

This patch series will free some vmemmap pages(struct page structures)
associated with each hugetlbpage when preallocated to save memory.

It would be lovely to be able to do this.  Unfortunately, it's completely
impossible right now.  Consider, for example, get_user_pages() called
on the fifth page of a hugetlb page.

Can you elaborate on the problem? Thanks so much.

OK, let's say you want to do a 2kB I/O to offset 0x5000 of a 2MB page
on a 4kB base page system.  Today, that results in a bio_vec containing
{head+5, 0, 0x800}.  Then we call page_to_phys() on that (head+5) struct
page to get the physical address of the I/O, and we turn it into a struct
scatterlist, which similarly has a reference to the page (head+5).

As I know, in this case, the get_user_pages() will get a reference
to the head page (head+0) before returning such that the hugetlb
page can not be freed. Although get_user_pages() returns the
page (head+5) and the scatterlist has a reference to the page
(head+5), this patch series can handle this situation. I can not
figure out where the problem is. What I missed? Thanks.

If you're returning page (head+1) from get_user_pages(), all the
calculations will be wrong and you'll do DMA to the wrong address.

What needs to happen is a conversion to get_user_bvecs().  That way we
can return a bvec which is {head, 0x5000, 0x800} and the calculations
will work.  So that's going to involve touching a lot of device drivers.
Christoph has a start on it here:
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/gup-bvec

Overlapping with that is a desire to change the biovec so that it
only stores {phys_addr, length} rather than {page, offset, length}.
We're also going to have to rework the scatterlist so that it doesn't
carry a struct page either.

-- 
Yours,
Muchun