Re: [PATCH v3 1/4] mm/page_isolation: make page isolation a standalone bit.

[Zi Yan <ziy@nvidia.com>]

>>> @@ -426,7 +460,12 @@ void set_pageblock_migratetype(struct page *page, int migratetype)
>>>  		     migratetype < MIGRATE_PCPTYPES))
>>>  		migratetype = MIGRATE_UNMOVABLE;
>>>
>>> -	set_pfnblock_flags_mask(page, (unsigned long)migratetype,
>>> +#ifdef CONFIG_MEMORY_ISOLATION
>>> +	if (migratetype == MIGRATE_ISOLATE)
>>> +		set_pageblock_isolate(page);
>>
>> Are there paths actually doing this after the second patch?
>>
>> There are many instances that want to *read* the migratetype or
>> MIGRATE_ISOLATE, but only isolation code should be manipulating that
>> bit through the dedicated set/toggle_pageblock_isolate API.
>>
>> If there isn't one, it might be good to enforce this with a VM_WARN
>> instead.
>
> I checked all set_pageblock_migratetype() callers and do not see
> one using it for pageblock isolation. Let me replace the code
> with a VM_WARN and add a comment to tell users to use dedicated
> pageblock isolation APIs.
>

Actually, move_freepages_block_isolate() calls __move_freepages_block()
to move free pages to MIGRATE_ISOLATE pageblock and
set_pageblock_migratetype() is used inside __move_freepages_block().
So the branch has to stay. Will use the suggestion below.

>>
>>> +	else
>>> +#endif
>>> +		set_pfnblock_flags_mask(page, (unsigned long)migratetype,
>>>  				page_to_pfn(page), MIGRATETYPE_MASK);
>>
>> If the branch stays, you could add a `return' to the MIGRATE_ISOLATE
>> leg, drop the else and indent this line normally.


Best Regards,
Yan, Zi