On 3/18/20 4:36 PM, Dave Hansen wrote: > On 3/18/20 3:52 PM, Mike Kravetz wrote: >> Sounds good. I'll incorporate those changes into a v2, unless someone >> else with has a different opinion. >> >> BTW, this patch should not really change the way the code works today. >> It is mostly a movement of code. Unless I am missing something, the >> existing code will always allow setup of PMD_SIZE hugetlb pages. > > Hah, I totally skipped over the old code in the diff. > > It looks like we'll disable hugetblfs *entirely* if PSE isn't supported. > I think this is actually wrong, but nobody ever noticed. I think you'd > have to be running as a guest under a hypervisor that's lying about PSE > not being supported *and* care about 1GB pages. Nobody does that. Actually, !PSE will disable hugetlbfs a little later in the boot process. You are talking about hugepages_supported() correct? I think something really bad could happen in this situation (!PSE and X86_FEATURE_GBPAGES). When parsing 'hugepages=' for gigantic pages we immediately allocate from bootmem. This happens before later checks in hugetlb_init for hugepages_supported(). So, I think we would end up allocating GB pages from bootmem and not be able to use or free them. :( Perhaps it would be best to check hugepages_supported() when parsing hugetlb command line options. If not enabled, throw an error. This will be much easier to do after moving all command line parsing to arch independent code. Is that a sufficient way to address this concern? I think it is a good change in any case. -- Mike Kravetz
