Re: [Xen-devel] [PATCH v7 2/9] mm: Place unscrubbed pages at the end of pagelist

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 08/15/2017 04:18 AM, Jan Beulich wrote:
>>>> On 14.08.17 at 16:29, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> On 08/14/2017 06:37 AM, Jan Beulich wrote:
>>>>>> On 08.08.17 at 23:45, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>>> --- 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;
>>>> +            bool need_tlbflush:1;
>>>> +
>>>> +            /*
>>>> +             * Index of the first *possibly* unscrubbed page in the buddy.
>>>> +             * One more bit than maximum possible order to accommodate
>>>> +             * INVALID_DIRTY_IDX.
>>>> +             */
>>>> +#define INVALID_DIRTY_IDX ((1UL << (MAX_ORDER + 1)) - 1)
>>>> +            unsigned long first_dirty:MAX_ORDER + 1;
>>>>          } free;
>>> I think generated code will be better with the two fields swapped:
>>> That way reading first_dirty won't involve a shift, and accessing a
>>> single bit doesn't require shifts at all on many architectures.
>> Ok, I will then keep need_tlbflush as the last field so the final struct
>> (as defined in patch 7) will look like
>>
>> struct {
>>         unsigned long first_dirty:MAX_ORDER + 1;
>>         unsigned long scrub_state:2;
>>         bool need_tlbflush:1;
>> };
> Hmm, actually - why do you need bitfields on the x86 side at all?
> They're needed for 32-bit architectures only, 64-bit ones ought
> to be fine with
>
> struct {
>         unsigned int first_dirty;
>         bool need_tlbflush;
>         uint8_t scrub_state;
> };

IIRC it was exactly because of ARM32 and at some point you suggested to
switch both x86 and ARM to bitfields.


>
> (plus a suitable BUILD_BUG_ON() to make sure first_dirty has
> at least MAX_ORDER + 1 bits). The ARM maintainers will know
> whether they would want to also differentiate ARM32 and
> ARM64 here.

Isn't using bitfields the only possibility for 32-bit? We can't fit
first_dirty into 2 bytes.

-boris



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