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: <20210106035027.GA1160@open-light-1.localdomain> <CAKgT0Ue=hXEarqjW-h2G1fODA5UiSjHsKz3FpcyTcaTtQR4iMw@mail.gmail.com>
In-Reply-To: <CAKgT0Ue=hXEarqjW-h2G1fODA5UiSjHsKz3FpcyTcaTtQR4iMw@mail.gmail.com>
From:   Liang Li <liliang324@gmail.com>
Date:   Thu, 7 Jan 2021 11:57:09 +0800
Message-ID: <CA+2MQi9MxE_DWW3BHLJbvYDsOppCmfL6AHkdRwtHX0gBDpDebA@mail.gmail.com>
Subject: Re: [PATCH 4/6] hugetlb: avoid allocation failed when page reporting
 is on going
To:     Alexander Duyck <alexander.duyck@gmail.com>
Cc:     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>,
        Mike Kravetz <mike.kravetz@oracle.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

> > Page reporting isolates free pages temporarily when reporting
> > free pages information. It will reduce the actual free pages
> > and may cause application failed for no enough available memory.
> > This patch try to solve this issue, when there is no free page
> > and page repoting is on going, wait until it is done.
> >
> > Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
> Please don't use this email address for me anymore. Either use
> alexander.duyck@gmail.com or alexanderduyck@fb.com. I am getting
> bounces when I reply to this thread because of the old address.

No problem.

> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index eb533995cb49..0fccd5f96954 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -2320,6 +2320,12 @@ struct page *alloc_huge_page(struct vm_area_struct *vma,
> >                 goto out_uncharge_cgroup_reservation;
> >
> >         spin_lock(&hugetlb_lock);
> > +       while (h->free_huge_pages <= 1 && h->isolated_huge_pages) {
> > +               spin_unlock(&hugetlb_lock);
> > +               mutex_lock(&h->mtx_prezero);
> > +               mutex_unlock(&h->mtx_prezero);
> > +               spin_lock(&hugetlb_lock);
> > +       }
>
> This seems like a bad idea. It kind of defeats the whole point of
> doing the page zeroing outside of the hugetlb_lock. Also it is
> operating on the assumption that the only way you might get a page is
> from the page zeroing logic.
>
> With the page reporting code we wouldn't drop the count to zero. We
> had checks that were going through and monitoring the watermarks and
> if we started to hit the low watermark we would stop page reporting
> and just assume there aren't enough pages to report. You might need to
> look at doing something similar here so that you can avoid colliding
> with the allocator.

For hugetlb, things are a little different, Just like Mike points out:
     "On some systems, hugetlb pages are a precious resource and
      the sysadmin carefully configures the number needed by
      applications.  Removing a hugetlb page (even for a very short
      period of time) could cause serious application failure."

Just keeping some pages in the freelist is not enough to prevent that from
happening, because these pages may be allocated while zero out is on
going, and application may still run into a situation for not available free
pages.

Thanks
Liang