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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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;
};

(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.

Jan


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