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:42, <boris.ostrovsky@xxxxxxxxxx> wrote:

>>> @@ -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?
> 
> For both of these questions --- we don't want to miss a poisoned page.
> For example, if a page was poisoned but for some reason is not marked
> PGC_need_scrub.

While I can accept this as an answer to the first question, I don't
see how it relates to the second one: When MEMF_no_scrub is
clear, the code above obviously scrubs the page in the first if()
body - what's the point of passing it to check_one_page() right
afterwards?

Jan


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