lkml.org 
[lkml]   [2018]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] Revert "mm/page_alloc: fix memmap_init_zone pageblock alignment"
    On 15 March 2018 at 10:14, Michal Hocko <mhocko@kernel.org> wrote:
    > On Wed 14-03-18 15:54:16, Ard Biesheuvel wrote:
    >> On 14 March 2018 at 14:54, Michal Hocko <mhocko@kernel.org> wrote:
    >> > On Wed 14-03-18 14:35:12, Ard Biesheuvel wrote:
    >> >> On 14 March 2018 at 14:13, Michal Hocko <mhocko@kernel.org> wrote:
    >> >> > Does http://lkml.kernel.org/r/20180313224240.25295-1-neelx@redhat.com
    >> >> > fix your issue? From the debugging info you provided it should because
    >> >> > the patch prevents jumping backwards.
    >> >> >
    >> >>
    >> >> The patch does fix the boot hang.
    >> >>
    >> >> But I am concerned that we are papering over a fundamental flaw in
    >> >> memblock_next_valid_pfn().
    >> >
    >> > It seems that memblock_next_valid_pfn is doing the right thing here. It
    >> > is the alignment which moves the pfn back AFAICS. I am not really
    >> > impressed about the original patch either, to be completely honest.
    >> > It just looks awfully tricky. I still didn't manage to wrap my head
    >> > around the original issue though so I do not have much better ideas to
    >> > be honest.
    >>
    >> So first of all, memblock_next_valid_pfn() never refers to its max_pfn
    >> argument, which is odd nut easily fixed.
    >
    > There is a patch to remove that parameter sitting in the mmotm tree.
    >
    >> Then, the whole idea of substracting one so that the pfn++ will
    >> produce the expected value is rather hacky,
    >
    > Absolutely agreed!
    >
    >> But the real problem is that rounding down pfn for the next iteration
    >> is dodgy, because early_pfn_valid() isn't guaranteed to return true
    >> for the rounded down value. I know it is probably fine in reality, but
    >> dodgy as hell.
    >
    > Yes, that is what I meant when saying I was not impressed... I am always
    > nervous when a loop makes jumps back and forth. I _think_ the main
    > problem here is that we try to initialize a partial pageblock even
    > though a part of it is invalid. We should simply ignore struct pages
    > for those pfns. We don't do that and that is mostly because of the
    > disconnect between what the page allocator and early init code refers to
    > as a unit of memory to care about. I do not remember exactly why but I
    > strongly suspect this is mostly a performance optimization on the page
    > allocator side so that we do not have to check each and every pfn. Maybe
    > we should signal partial pageblocks from an early code and drop the
    > optimization in the page allocator init code.
    >
    >> The same applies to the call to early_pfn_in_nid() btw
    >
    > Why?

    By 'the same' I mean it isn't guaranteed to return true for the
    rounded down value *at the API level*. I understand it will be mostly
    fine in reality, but juggling (in)valid PFNs like this is likely to
    end badly.

    \
     
     \ /
      Last update: 2018-03-15 11:18    [W:2.722 / U:0.232 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site