On Fri, 15 Jun 2018 10:00:00 -0400 Pavel Tatashin <pasha.tatashin@xxxxxxxxxx> wrote: > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> > > > Tested-by: Oscar Salvador <osalvador@xxxxxxx> > > > > 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 <mhocko@xxxxxxxx> > > 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.tatashin@xxxxxxxxxx> > > 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?
