On Fri, 15 Jun 2018 10:00:00 -0400 Pavel Tatashin <pasha.tatas...@oracle.com> 
wrote:

> > > Signed-off-by: Naoya Horiguchi <n-horigu...@ah.jp.nec.com>
> > > Tested-by: Oscar Salvador <osalva...@suse.de>
> > 
> > OK, this makes sense to me. It is definitely much better than the
> > original attempt.
> > 
> > Unless I am missing something this should be correct
> > Acked-by: Michal Hocko <mho...@suse.com>
> 
> First of all thank you Naoya for finding and root causing this issue.
> 
> So, with this fix we reserve any hole and !E820_TYPE_RAM or
> !E820_TYPE_RESERVED_KERN in e820.  I think, this will work because we
> do pfn_valid() check in zero_resv_unavail(), so the ranges that do not have
> backing struct pages will be skipped. But, I am worried on the performance
> implications of when the holes of invalid memory are rather large. We would
> have to loop through it in zero_resv_unavail() one pfn at a time.
> 
> Therefore, we might also need to optimize zero_resv_unavail() a little like
> this:
> 
> 6407                  if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages)))
> 6408                          continue;
> 
> Add before "continue":
>       pfn = ALIGN_DOWN(pfn, pageblock_nr_pages) + pageblock_nr_pageas - 1.
> At least, this way, we would skip a section of invalid memory at a time.
> 
> For the patch above:
> Reviewed-by: Pavel Tatashin <pasha.tatas...@oracle.com>
> 
> But, I think the 2nd patch with the optimization above should go along this
> this fix.

So I expect this patch needs a cc:stable, which I'll add.

The optimiation patch seems less important and I'd like to hold that
off for 4.19-rc1?

Reply via email to