Re: [External] Re: [PATCH v3 09/21] mm/hugetlb: Free the vmemmap pages associated with each hugetlb page

From: Oscar Salvador <osalvador@suse.de>
To: Muchun Song <songmuchun@bytedance.com>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	mingo@redhat.com, bp@alien8.de, x86@kernel.org, hpa@zytor.com,
	dave.hansen@linux.intel.com, luto@kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	viro@zeniv.linux.org.uk,
	Andrew Morton <akpm@linux-foundation.org>,
	paulmck@kernel.org, mchehab+huawei@kernel.org,
	pawan.kumar.gupta@linux.intel.com,
	Randy Dunlap <rdunlap@infradead.org>,
	oneukum@suse.com, anshuman.khandual@arm.com, jroedel@suse.de,
	Mina Almasry <almasrymina@google.com>,
	David Rientjes <rientjes@google.com>,
	Matthew Wilcox <willy@infradead.org>,
	Michal Hocko <mhocko@suse.com>,
	Xiongchun duan <duanxiongchun@bytedance.com>,
	linux-doc@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [External] Re: [PATCH v3 09/21] mm/hugetlb: Free the vmemmap pages associated with each hugetlb page
Date: Tue, 10 Nov 2020 14:52:15 +0100	[thread overview]
Message-ID: <20201110135210.GA29463@linux> (raw)
In-Reply-To: <CAMZfGtW0nwhdgwUwwq5SXgEAk3+6cyDfM5n28UerVuAxatwj4g@mail.gmail.com>

On Tue, Nov 10, 2020 at 06:47:08PM +0800, Muchun Song wrote:
> > That only refers to gigantic pages, right?
> 
> Yeah, now it only refers to gigantic pages. Originally, I also wanted to merge
> vmemmap PTE to PMD for normal 2MB HugeTLB pages. So I introduced
> those macros(e.g. freed_vmemmap_hpage). For 2MB HugeTLB pages, I
> haven't found an elegant solution. Hopefully, when you or someone have
> read all of the patch series, we can come up with an elegant solution to
> merge PTE.

Well, it is quite a lot of "tricky" code, so it takes some time.

> > > > > +static void free_huge_page_vmemmap(struct hstate *h, struct page *head)
> > > > > +{
> > > > > +     pmd_t *pmd;
> > > > > +     spinlock_t *ptl;
> > > > > +     LIST_HEAD(free_pages);
> > > > > +
> > > > > +     if (!free_vmemmap_pages_per_hpage(h))
> > > > > +             return;
> > > > > +
> > > > > +     pmd = vmemmap_to_pmd(head);
> > > > > +     ptl = vmemmap_pmd_lock(pmd);

I forgot about this one.
You might want to check whether vmemmap_to_pmd returns NULL or not.
If it does means that something went wrong anyways, but still we should handle
such case (and print a fat warning or something like that).

-- 
Oscar Salvador
SUSE L3