Re: [PATCH 1/3] mm: consider zone which is not fully populated to have holes

[Vlastimil Babka]

On 04/18/2017 11:27 AM, Michal Hocko wrote:
> On Tue 18-04-17 10:45:23, Vlastimil Babka wrote:
>> On 04/15/2017 02:17 PM, Michal Hocko wrote:
>>> From: Michal Hocko <mhocko@xxxxxxxx>
>>>
>>
>> My issue with this is that PageReserved can be also set for other
>> reasons than offlined block, e.g. by a random driver. So there are two
>> suboptimal scenarios:
>>
>> - PageReserved is set on some page in the middle of pageblock. It won't
>> be detected by this patch. This violates the "it would be safer" argument.
>> - PageReserved is set on just the first (few) page(s) and because of
>> this patch, we skip it completely and won't compact the rest of it.
> 
> Why would that be a big problem? PageReserved is used only very seldom
> and few page blocks skipped would seem like a minor issue to me.

Yes it's not critical, just suboptimal. Can be improved later.

>> So if we decide we really need to check PageReserved to ensure safety,
>> then we have to check it on each page. But I hope the existing criteria
>> in compaction scanners are sufficient. Unless the semantic is that if
>> somebody sets PageReserved, he's free to repurpose the rest of flags at
>> his will (IMHO that's not the case).
> 
> I am not aware of any such user. PageReserved has always been about "the
> core mm should touch these pages and modify their state" AFAIR.
> But I believe that touching those holes just asks for problems so I
> would rather have them covered.

OK. I guess it's OK to use PageReserved of first pageblock page to
determine if we can trust page_zone(), because the memory offline
scenario should have sufficient granularity and not make holes inside
pageblock?

>> The pageblock-level check them becomes a performance optimization so
>> when there's an "offline hole", compaction won't iterate it page by
>> page. But the downside is the false positive resulting in skipping whole
>> pageblock due to single page.
>> I guess it's uncommon for a longlived offline holes to exist, so we
>> could simply just drop this?
> 
> This is hard to tell but I can imagine that some memory hotplug
> balloning drivers might want to offline hole into existing zones.

OK.