Re: [Xen-devel] [PATCH v5 1/8] mm: Place unscrubbed pages at the end of pagelist

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 07/31/17 6:03 PM >>>
On 07/31/2017 10:45 AM, Jan Beulich wrote:
>>>> Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 07/23/17 4:01 AM >>>
>> On 06/27/2017 01:06 PM, Jan Beulich wrote:
>>>>>> Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 06/22/17 8:55 PM >>>
>>>>> --- a/xen/include/asm-x86/mm.h
>>>>> +++ b/xen/include/asm-x86/mm.h
>>>>> @@ -88,7 +88,15 @@ struct page_info
>>>>>           /* Page is on a free list: ((count_info & PGC_count_mask) == 
>>>>> 0). */
>>>>>           struct {
>>>>>               /* Do TLBs need flushing for safety before next page use? */
>>>>> -            bool_t need_tlbflush;
>>>>> +            unsigned long need_tlbflush:1;
>>>>> +
>>>>> +            /*
>>>>> +             * Index of the first *possibly* unscrubbed page in the 
>>>>> buddy.
>>>>> +             * One more than maximum possible order (MAX_ORDER+1) to
>>>> Why +1 here and hence ...
>>> Don't we have MAX_ORDER+1 orders?
>> So here there might be a simple misunderstanding: I understand the
>> parenthesized MAX_ORDER+1 to represent "maximum possible
>> order", i.e. excluding the "one more than", not the least because of
>> the ...
>>
>>>> +             * accommodate INVALID_DIRTY_IDX.
>>>> +             */
>>>> +#define INVALID_DIRTY_IDX (-1UL & (((1UL<<MAX_ORDER) + 2) - 1))
>>>> +            unsigned long first_dirty:MAX_ORDER + 2;
>> +2 here.
>>
>>>> ... why +2 instead of +1? And isn't the expression INVALID_DIRTY_IDX 
>>>> wrongly
>>>> parenthesized (apart from lacking blanks around the shift operator)? I'd
>>>> expect you want a value with MAX_ORDER+1 set bits, i.e.
>>>> (1UL << (MAX_ORDER + 1)) - 1. ANDing with -1UL seems quite pointless too.
>>> Yes to parentheses and AND. Should be (1UL << (MAX_ORDER + 2)) - 1
>> I.e. I would still expect it to be (1UL << (MAX_ORDER + 1)) - 1
>> here.
>
>
>Sorry, I still don't get it.
>
>Say, MAX_ORDER is 1. Since this implies that indexes 0, 1, 2 and 3 are
>all valid (because we can have up to 2^(MAX_ORDER+1) pages), don't we
>need 3 bits to indicate an invalid index?

Why 0, 1, 2, and 3? When MAX_ORDER is 1, we only have a single bit, i.e.
valid values 0 and 1 (plus one more for the invalid indicator), i.e. need 2 bits
for representation of all used values.

Jan


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