Hi all:
When I investigate the mm/hugetlb.c code again, I found there are a few possible issues
with avoid_reserve case. (It's really hard to follow the relevant code for me.) Please take
a look at the below analysis:
1.avoid_reserve issue with h->resv_huge_pages in alloc_huge_page.
Assume:
h->free_huge_pages 60
h->resv_huge_pages 30
spool->rsv_hpages 30
When avoid_reserve is true, after alloc_huge_page(), we will have:
spool->rsv_hpages 29 /* hugepage_subpool_get_pages decreases it. */
h->free_huge_pages 59
h->resv_huge_pages 30 /* rsv_hpages is used, but *h->resv_huge_pages is not modified accordingly*. */
If the hugetlb page is freed later, we will have:
spool->rsv_hpages 30 /* hugepage_subpool_put_pages increases it. */
h->free_huge_pages 60
h->resv_huge_pages 31 /* *increased wrongly* due to hugepage_subpool_put_pages(spool, 1) == 0. */
^^
2.avoid_reserve issue with hugetlb rsvd cgroup charge for private mappings in alloc_huge_page.
In general, if hugetlb pages are reserved, corresponding rsvd counters are charged in resv_maps
for private mappings. Otherwise they're charged in individual hugetlb pages. When alloc_huge_page()
is called with avoid_reserve == true, hugetlb_cgroup_charge_cgroup_rsvd() will be called to charge
the newly allocated hugetlb page even if there has a reservation for this page in resv_maps. Then
vma_commit_reservation() is called to indicate that the reservation is consumed. So the reservation
*can not be used, thus leaking* from now on because vma_needs_reservation always return 1 for it.
3.avoid_reserve issue with restore_reserve_on_error
There's a assumption in restore_reserve_on_error(): If HPageRestoreReserve is not set, this indicates
there is an entry in the reserve map added by alloc_huge_page or HPageRestoreReserve would be set on the
page. But this assumption *does not hold for avoid_reserve*. HPageRestoreReserve won't be set even if there
is already an entry in the reserve map for avoid_reserve case. So avoid_reserve should be considered in this
function, i.e. we need *a reliable way* to determine whether the entry is added by the alloc_huge_page().
Are above issues possible? Or am I miss something? These possible issues seem not easy to fix for me.
Any thoughts? Any response would be appreciated!
Thanks!
Miaohe Lin
