Re: [PATCH v2 2/9] mm: Place unscrubbed pages at the end of pagelist

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 04/04/2017 11:50 AM, Jan Beulich wrote:
>>>> On 04.04.17 at 17:39, <boris.ostrovsky@oracle.com> wrote:
>> On 04/04/2017 11:29 AM, Jan Beulich wrote:
>>>>>> On 04.04.17 at 17:14, <boris.ostrovsky@oracle.com> wrote:
>>>> On 04/04/2017 10:46 AM, Jan Beulich wrote:
>>>>>> @@ -933,6 +952,10 @@ static bool_t can_merge(struct page_info *buddy, unsigned int node,
>>>>>>           (phys_to_nid(page_to_maddr(buddy)) != node) )
>>>>>>          return false;
>>>>>>  
>>>>>> +    if ( need_scrub !=
>>>>>> +         !!test_bit(_PGC_need_scrub, &buddy->count_info) )
>>>>>> +        return false;
>>>>> I don't think leaving the tree in a state where larger order chunks
>>>>> don't become available for allocation right away is going to be
>>>>> acceptable. Hence with this issue being dealt with only in patch 7
>>>>> as it seems, you should state clearly and visibly that (at least)
>>>>> patches 2...7 should only be committed together.
>>>> The dirty pages are available for allocation as result of this patch but
>>>> they might not be merged with higher orders (which is what this check is
>>>> for)
>>> The individual chunks are available for allocation, but not the
>>> combined one (for a suitably high order request). Or am I
>>> missing something?
>>
>> Correct, but this is not changed by any later patch (including patch 7).
>> We only merge with a buddy with the same level of cleanliness (so to
>> speak ;-))
> Hmm, that aspect was one of the main things I had objected to
> back when one of your colleagues had a first take at this.

I thought your objections were over having a period of time when a chunk
is removed from heap for scrubbing and this is not available at all.


>
>> But then we will always have to scan the buddy during allocation to see
>> if any pages are dirty.
> There could be a summary flag to avoid this for entirely clean
> buddies. Plus perhaps some auxiliary indication where the first
> unclean part is, to speed up the scanning.


This should be doable. Let me work on this.

-boris


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel