Re: [Xen-devel] [PATCH 4/5] mm: Don't request scrubbing until dom0 is running

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 08/30/2017 04:42 AM, Wei Liu wrote:
> On Tue, Aug 29, 2017 at 01:09:16PM -0400, Boris Ostrovsky wrote:
>> There is no need to scrub pages freed during dom0 construction
>> since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()).
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> ---
>> Changes in v2:
>> * Removed '#ifdef CONFIG_SCRUB_DEBUG'
>>
>>  xen/common/page_alloc.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>> index 3db77c5..6c08983 100644
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -2247,16 +2247,12 @@ void free_domheap_pages(struct page_info *pg, 
>> unsigned int order)
>>  
>>              spin_unlock_recursive(&d->page_alloc_lock);
>>  
>> -#ifndef CONFIG_SCRUB_DEBUG
>>              /*
>>               * Normally we expect a domain to clear pages before freeing 
>> them,
>>               * if it cares about the secrecy of their contents. However, 
>> after
>>               * a domain has died we assume responsibility for erasure.
>>               */
>> -            scrub = !!d->is_dying;
>> -#else
>> -            scrub = true;
>> -#endif
>> +            scrub = !!d->is_dying | scrub_debug;
> Use logical or here please. And the !! for is_dying is not necessary.

Yes, I'll change this.

>
> Also please expand the comment to say why scrub_debug is also checked.

TBH, I think variable name (scrub_debug) should be sufficient. I could
say something like "when scrub debugging is on we always scrub" but I am
not sure how useful that would be. (Previous name, boot_scrub_done,
would have indeed be helped by a comment).

-boris

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