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