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
