[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 7/9] xen/arm: unpopulate memory when domain is static


  • To: Penny Zheng <Penny.Zheng@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 29 Jun 2022 08:19:06 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=bzkFdjR0q1QDQClsBvEwo+30DWf9JqVf5Be/ROD0dXk=; b=Wh9rqgv4byzTQNGTYpceh6telQ9/JnTFQgEgedu/KrcJceySX1WiCp9igdgNdbi/zT7/aW3ssu+0QS1DaZxWRm58kEH6KZfZgwiIwSHxGPErKhgpapo+fnFq8ZvVkZ7XaCH6V77Wt/cXg9EIJSX/i+tGjsq47UOxjYpZur05Mh8t5bQFJs/VazZy+n4KzCQqs3uVdfL6MShcA9XqTiVmw8afkqF0ueeMvf5yyRjCXqHcCmHqmElrO768vFfSJjUGa4GII8rLe87v1KamcveLU1CHB/d/GOotz7lOjUMSRsr7Uqjv23CgfedadoQKve/nNpbPWVmbri+GdqIYyVzDXQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Av1wnYBuepFT5CvvIAMsFPWT8pno5A2LBc2Ioi6B1tnKuEcx5q6MIPOOIHHL4j5v8szb4byuDE6pdkY/sBtDcuCKPmZTiEcZIzHEiaYlKSxEo9Rcp4+DvF7OHsf4B/GyjoYCchKWKQ7V7au2G9NZSsQPkZdmXIBObRf+Vn3S7Kj14pHw0XAOSnCkI2DzedyFs0aqm7zTTovXYhshsiRt3lU9tsXULsrtpy0FMPn/GKm9pjpb/9suhBBz7G70dMZANr+5F/uIClM2b8y8ElbHV0meGyfnXAKqy/eRez7SDj7po38CskAJaVavgFQtWGsfWoKsYANhwzYblDrJIELHWw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Wei Chen <Wei.Chen@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>
  • Delivery-date: Wed, 29 Jun 2022 06:19:21 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 29.06.2022 08:08, Penny Zheng wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Wednesday, June 29, 2022 1:56 PM
>>
>> On 29.06.2022 05:12, Penny Zheng wrote:
>>>> From: Julien Grall <julien@xxxxxxx>
>>>> Sent: Monday, June 27, 2022 6:19 PM
>>>>
>>>> On 27/06/2022 11:03, Penny Zheng wrote:
>>>>>> -----Original Message-----
>>>>> put_static_pages, that is, adding pages to the reserved list, is
>>>>> only for freeing static pages on runtime. In static page
>>>>> initialization stage, I also use free_statimem_pages, and in which
>>>>> stage, I think the domain has not been constructed at all. So I
>>>>> prefer the freeing of staticmem pages is split into two parts:
>>>>> free_staticmem_pages and put_static_pages
>>>>
>>>> AFAIU, all the pages would have to be allocated via
>>>> acquire_domstatic_pages(). This call requires the domain to be
>>>> allocated and setup for handling memory.
>>>>
>>>> Therefore, I think the split is unnecessary. This would also have the
>>>> advantage to remove one loop. Admittly, this is not important when
>>>> the order 0, but it would become a problem for larger order (you may
>>>> have to pull the struct page_info multiple time in the cache).
>>>>
>>>
>>> How about this:
>>> I create a new func free_domstatic_page, and it will be like:
>>> "
>>> static void free_domstatic_page(struct domain *d, struct page_info
>>> *page) {
>>>     unsigned int i;
>>>     bool need_scrub;
>>>
>>>     /* NB. May recursively lock from relinquish_memory(). */
>>>     spin_lock_recursive(&d->page_alloc_lock);
>>>
>>>     arch_free_heap_page(d, page);
>>>
>>>     /*
>>>      * 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. We do
>>>      * scrub regardless if option scrub_domheap is set.
>>>      */
>>>     need_scrub = d->is_dying || scrub_debug || opt_scrub_domheap;
>>>
>>>     free_staticmem_pages(page, 1, need_scrub);
>>>
>>>     /* Add page on the resv_page_list *after* it has been freed. */
>>>     put_static_page(d, page);
>>>
>>>     drop_dom_ref = !domain_adjust_tot_pages(d, -1);
>>>
>>>     spin_unlock_recursive(&d->page_alloc_lock);
>>>
>>>     if ( drop_dom_ref )
>>>         put_domain(d);
>>> }
>>> "
>>>
>>> In free_domheap_pages, we just call free_domstatic_page:
>>>
>>> "
>>> @@ -2430,6 +2430,9 @@ void free_domheap_pages(struct page_info *pg,
>>> unsigned int order)
>>>
>>>      ASSERT_ALLOC_CONTEXT();
>>>
>>> +    if ( unlikely(pg->count_info & PGC_static) )
>>> +        return free_domstatic_page(d, pg);
>>> +
>>>      if ( unlikely(is_xen_heap_page(pg)) )
>>>      {
>>>          /* NB. May recursively lock from relinquish_memory(). */ @@
>>> -2673,6 +2676,38 @@ void free_staticmem_pages(struct page_info *pg,
>>> unsigned long nr_mfns, "
>>>
>>> Then the split could be avoided and we could save the loop as much as
>> possible.
>>> Any suggestion?
>>
>> Looks reasonable at the first glance (will need to see it in proper context 
>> for a
>> final opinion), provided e.g. Xen heap pages can never be static.
> 
> If you don't prefer let free_domheap_pages to call free_domstatic_page, then, 
> maybe
> the if-array should happen at put_page
> "
> @@ -1622,6 +1622,8 @@ void put_page(struct page_info *page)
> 
>      if ( unlikely((nx & PGC_count_mask) == 0) )
>      {
> +        if ( unlikely(page->count_info & PGC_static) )
> +            free_domstatic_page(page);
>          free_domheap_page(page);
>      }
>  }
> "
> Wdyt now?

Personally I'd prefer this variant, but we'll have to see what Julien or
the other Arm maintainers think.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.