Re: [PATCH v3 04/14] mm/memremap: add ZONE_DEVICE support for compound pages

[Joao Martins <joao.m.martins@oracle.com>]

On 7/22/21 1:38 AM, Jane Chu wrote:
> 
> On 7/15/2021 5:52 AM, Joao Martins wrote:
>>>> +               __init_zone_device_page(page + i, pfn + i, zone_idx,
>>>> +                                       nid, pgmap);
>>>> +               prep_compound_tail(page, i);
>>>> +
>>>> +               /*
>>>> +                * The first and second tail pages need to
>>>> +                * initialized first, hence the head page is
>>>> +                * prepared last.
>>> I'd change this comment to say why rather than restate what can be
>>> gleaned from the code. It's actually not clear to me why this order is
>>> necessary.
>>>
>> So the first tail page stores mapcount_ptr and compound order, and the
>> second tail page stores pincount_ptr. prep_compound_head() does this:
>>
>> 	set_compound_order(page, order);
>> 	atomic_set(compound_mapcount_ptr(page), -1);
>> 	if (hpage_pincount_available(page))
>> 		atomic_set(compound_pincount_ptr(page), 0);
>>
>> So we need those tail pages initialized first prior to initializing the head.
>>
>> I can expand the comment above to make it clear why we need first and second tail pages.
>>
> 
> Perhaps just say
>    The reason prep_compound_head() is called after the 1st and 2nd tail
>    pages have been initialized is: so it overwrites some of the tail page
>    fields setup by __init_zone_device_page(), rather than the other way 
> around.
> ?

Yeap, something along those lines is what I was thinking. Perhaps explicitly mentioning
the struct page fields that 1st and 2nd tail pages store to avoid the reader thinking it's
arbitrarily picked.

	Joao