Re: [Xen-devel] [PATCH v5 6/8] mm: Keep heap accessible to others while scrubbing

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 06/22/17 8:56 PM >>>
> Changes in v5:
> * Fixed off-by-one error in setting first_dirty
> * Changed struct page_info.u.free to a union to permit use of ACCESS_ONCE in
>   check_and_stop_scrub()

I don't see the need for this:

> +static void check_and_stop_scrub(struct page_info *head)
> +{
> +    if ( head->u.free.scrub_state == BUDDY_SCRUBBING )
> +    {
> +        struct page_info pg;
> +
> +        head->u.free.scrub_state = BUDDY_SCRUB_ABORT;
> +        spin_lock_kick();
> +        for ( ; ; )
> +        {
> +            /* Can't ACCESS_ONCE() a bitfield. */
> +            pg.u.free.val = ACCESS_ONCE(head->u.free.val);

Something like ACCESS_ONCE(head->u.free).val ought to work (or read_atomic(),
due to the questionable scalar type check in ACCESS_ONCE()).

> @@ -1106,25 +1155,53 @@ bool scrub_free_pages(void)
>          do {
>              while ( !page_list_empty(&heap(node, zone, order)) )
>              {
> -                unsigned int i;
> +                unsigned int i, dirty_cnt;
> +                struct scrub_wait_state st;
>  
>                  /* Unscrubbed pages are always at the end of the list. */
>                  pg = page_list_last(&heap(node, zone, order));
>                  if ( pg->u.free.first_dirty == INVALID_DIRTY_IDX )
>                      break;
>  
> +                ASSERT(!pg->u.free.scrub_state);

Please use BUDDY_NOT_SCRUBBING here.

> @@ -1138,6 +1215,17 @@ bool scrub_free_pages(void)
>                      }
>                  }
>  
> +                st.pg = pg;
> +                st.first_dirty = (i >= (1UL << order) - 1) ?
> +                    INVALID_DIRTY_IDX : i + 1;

Would you mind explaining to me (again?) why you can't set pg's first_dirty
directly here? In case I'm not mistaken and this has been asked before, maybe
this is a hint that a comment might be warranted.

Jan

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