Re: [External] Re: [patch 004/192] mm: hugetlb: free the vmemmap pages associated with each HugeTLB page

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 1, 2021 at 11:46 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Jun 30, 2021 at 6:47 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > From: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > Subject: mm: hugetlb: free the vmemmap pages associated with each HugeTLB page
> >
> > Every HugeTLB has more than one struct page structure.  We __know__ that
> > we only use the first 4 (__NR_USED_SUBPAGE) struct page structures to
> > store metadata associated with each HugeTLB.
> >
> > There are a lot of struct page structures associated with each HugeTLB
> > page.  For tail pages, the value of compound_head is the same.  So we can
> > reuse first page of tail page structures.   [..]
>
> I think this means to say that we can reuse the _second_ page of the
> tail page structures, since the first page is special and also
> contains the first (non-tail) 'struct page'.
>
> Or maybe the intent is to say that that second page is the "first page
> of purely tail page structures"?

Hi Linus,

Right. This is what I mean. Evey 2MB hugepage has 8 vmemmap
pages (32KB), the 2nd vmemmap page is reused here. The remapping
details can refer to the head of mm/hugetlb_vmemmap.c.

>
> Anyway, this HugeTLB 'struct page' vmemmap patch-series doesn't look
> _wrong_ to me, but it does look like it is a nightmare to debug if
> something ever goes wrong. And it looks like a lot of things _could_
> go wrong. It all looks very subtle.

In order to make things work well, some addresses of vmemmap are
also mapped with read only to catch invalid usage from other modules
(e.g. write operation). I didn't get the point of "a lot of things _could_ go
wrong". Would you like to describe the details? Thanks.

>
> Put another way: I'm not objecting to this series, but it does make me
> nervous, and I just want to give a heads-up that if we start seeing
> problems with this, I think people need to be ready to very
> aggressively revert it unless the fixes are obvious.
>
> How much testing has this series gotten on loads that are heavy users
> of hugetlb?

This series was tested by Huawei, AWS and Bytedance. In our company,
this feature was proposed in 2020.03, we have tested several months on our
servers (we have a lot of virtual machines). We didn't find any issues.

Thanks.

>
>                 Linus



[Index of Archives]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux