Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210106034918.GA1154@open-light-1.localdomain> <3bfdbe48-5818-7470-4c3b-96e62d387fb4@oracle.com>
In-Reply-To: <3bfdbe48-5818-7470-4c3b-96e62d387fb4@oracle.com>
From:   Liang Li <liliang324@gmail.com>
Date:   Mon, 11 Jan 2021 12:09:18 +0800
Message-ID: <CA+2MQi8Z0se7AGdzyXVqMeUZkHBq8KXEGQcyKHeH256ZvHTX8w@mail.gmail.com>
Subject: Re: [PATCH 3/6] hugetlb: add free page reporting support
To:     Mike Kravetz <mike.kravetz@oracle.com>
Cc:     Alexander Duyck <alexander.h.duyck@linux.intel.com>,
        Mel Gorman <mgorman@techsingularity.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Dan Williams <dan.j.williams@intel.com>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        David Hildenbrand <david@redhat.com>,
        Jason Wang <jasowang@redhat.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Michal Hocko <mhocko@suse.com>,
        Liang Li <liliangleo@didiglobal.com>,
        linux-mm <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>,
        virtualization@lists.linux-foundation.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 8, 2021 at 6:04 AM Mike Kravetz <mike.kravetz@oracle.com> wrote:
>
> On 1/5/21 7:49 PM, Liang Li wrote:
> > hugetlb manages its page in hstate's free page list, not in buddy
> > system, this patch try to make it works for hugetlbfs. It canbe
> > used for memory overcommit in virtualization and hugetlb pre zero
> > out.
>
> I am not looking closely at the hugetlb changes yet.  There seem to be
> higher level questions about page reporting/etc.  Once those are sorted,
> I will be happy to take a closer look.  One quick question below.
>
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -41,6 +41,7 @@
> >  #include <linux/node.h>
> >  #include <linux/userfaultfd_k.h>
> >  #include <linux/page_owner.h>
> > +#include "page_reporting.h"
> >  #include "internal.h"
> >
> >  int hugetlb_max_hstate __read_mostly;
> > @@ -1028,6 +1029,9 @@ static void enqueue_huge_page(struct hstate *h, struct page *page)
> >       list_move(&page->lru, &h->hugepage_freelists[nid]);
> >       h->free_huge_pages++;
> >       h->free_huge_pages_node[nid]++;
> > +     if (hugepage_reported(page))
> > +             __ClearPageReported(page);
> > +     hugepage_reporting_notify_free(h->order);
> >  }
> >
> >  static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid)
> > @@ -5531,6 +5535,21 @@ follow_huge_pgd(struct mm_struct *mm, unsigned long address, pgd_t *pgd, int fla
> >       return pte_page(*(pte_t *)pgd) + ((address & ~PGDIR_MASK) >> PAGE_SHIFT);
> >  }
> >
> > +void isolate_free_huge_page(struct page *page, struct hstate *h, int nid)
> > +{
> > +     VM_BUG_ON_PAGE(!PageHead(page), page);
> > +
> > +     list_move(&page->lru, &h->hugepage_activelist);
> > +     set_page_refcounted(page);
> > +}
> > +
> > +void putback_isolate_huge_page(struct hstate *h, struct page *page)
> > +{
> > +     int nid = page_to_nid(page);
> > +
> > +     list_move(&page->lru, &h->hugepage_freelists[nid]);
> > +}
>
> The above routines move pages between the free and active lists without any
> update to free page counts.  How does that work?  Will the number of entries
> on the free list get out of sync with the free_huge_pages counters?
> --
> Mike Kravetz
>
Yes. the  free_huge_pages counters will be out of sync with the free list.
There are two reasons for the above code: 1. Hide the free page reporting
to the user; 2. Simplify the logic to sync 'free_huge_pages' and
'resv_huge_pages'.  I am not sure if it will break something else.

Thanks
Liang