Re: [Xen-devel] [PATCH v2] mm: Don't scrub pages while holding heap lock in alloc_heap_pages()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.09.17 at 16:11, <boris.ostrovsky@xxxxxxxxxx> wrote:
> I couldn't convince myself that just marking the head as PGC_state_inuse
> under the lock is safe, mostly because of how MCE handler may write the
> state while the allocator is walking (lock-free) the buddy.

Good point.

> @@ -974,13 +972,39 @@ static struct page_info *alloc_heap_pages(
>           * guest can control its own visibility of/through the cache.
>           */
>          flush_page_to_ram(page_to_mfn(&pg[i]), !(memflags & 
> MEMF_no_icache_flush));
> -
> -        if ( !(memflags & MEMF_no_scrub) )
> -            check_one_page(&pg[i]);
>      }
>  
>      spin_unlock(&heap_lock);
>  
> +    if ( first_dirty != INVALID_DIRTY_IDX ||
> +         (scrub_debug && !(memflags & MEMF_no_scrub)) )

Why does scrub_debug matter here?.

> +    {
> +        for ( i = 0; i < (1U << order); i++ )
> +        {
> +            if ( test_bit(_PGC_need_scrub, &pg[i].count_info) )
> +            {
> +                if ( !(memflags & MEMF_no_scrub) )
> +                    scrub_one_page(&pg[i]);
> +
> +                dirty_cnt++;
> +
> +                spin_lock(&heap_lock);
> +                pg[i].count_info &= ~PGC_need_scrub;
> +                spin_unlock(&heap_lock);
> +            }
> +
> +            if ( !(memflags & MEMF_no_scrub) )
> +                check_one_page(&pg[i]);

Wouldn't this better be "else if", as checking a page just scrubbed
doesn't look very useful?

Jan


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