lkml.org 
[lkml]   [2020]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [External] Re: [PATCH v7 00/15] Free some vmemmap pages of hugetlb page
On Fri, Dec 04, 2020 at 11:39:31AM +0800, Muchun Song wrote:
> On Fri, Dec 4, 2020 at 7:49 AM Mike Kravetz <mike.kravetz@oracle.com> wrote:
> > As previously mentioned, I feel qualified to review the hugetlb changes
> > and some other closely related changes. However, this patch set is
> > touching quite a few areas and I do not feel qualified to make authoritative
> > statements about them all. I too hope others will take a look.
>
> Agree. I also hope others can take a look at other modules(e.g.
> sparse-vmemmap, memory-hotplug). Thanks for everyone's efforts
> on this.

I got sidetracked by some other stuff but I plan to continue reviewing
this series.

One thing that came to my mind is that if we do as David suggested in
patch#4, and we move it towards the end to actually __enable__ this
once all the infrastructure is there (unless hstate->nr_vmemmap_pages
differs from 0 we should not be doing any work AFAIK), we could also
move patch#6 to the end (right before the enablement), kill patch#7
and only leave patch#13.

The reason for that (killing patch#7 and leaving patch#13 only)
is that it does not make much sense to me to disable PMD-mapped vmemmap
depending on the CONFIG_HUGETLB_xxxxx as that is enabled by default
to replace that later by the boot kernel parameter.
It looks more natural to me to disable it when we introduce the kernel
boot parameter, before the actual enablement of the feature.

As I said, I plan to start the review again, but the order above would
make more sense to me.

thanks

--
Oscar Salvador
SUSE L3

\
 
 \ /
  Last update: 2020-12-07 19:40    [W:0.497 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site