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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.17 at 20:33, <boris.ostrovsky@xxxxxxxxxx> wrote:
> .. so that it's easy to find pages that need to be scrubbed (those pages are
> now marked with _PGC_need_scrub bit).
> 
> We keep track of the first unscrubbed page in a page buddy using first_dirty
> field. For now it can have two values, 0 (whole buddy needs scrubbing) or
> INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent patches
> will allow scrubbing to be interrupted, resulting in first_dirty taking any
> value.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one remark:

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -261,7 +261,11 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
>  #ifdef CONFIG_X86
>      const unsigned long *badpage = NULL;
>      unsigned int i, array_size;
> +
> +    BUILD_BUG_ON(8 * sizeof(((struct page_info *)0)->u.free.first_dirty) <
> +                 MAX_ORDER + 1);
>  #endif
> +    BUILD_BUG_ON(sizeof(((struct page_info *)0)->u) != sizeof(unsigned 
> long));

As I'm generally opposed to casts whenever one can get away
without, I dislike these as well. In the case here, short of a local
variable of suitable type, I'd suggest using frame_table instead
of the open-coded cast. If you're fine with that, this can easily
be done while committing.

Jan


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