Re: [External] Re: [PATCH v7 00/15] Free some vmemmap pages of hugetlb page

[Muchun Song]

On Tue, Dec 8, 2020 at 2:38 AM Oscar Salvador <osalvador@xxxxxxx> wrote:
>
> 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@xxxxxxxxxx> 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.

Many thanks, Oscar.

>
> 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.

Thanks for your suggestions. I agree with you. :)

>
> 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



-- 
Yours,
Muchun