On 1/23/20 3:32 PM, Jan Beulich wrote: > On 23.01.2020 15:03, Paul Durrant wrote: >> vmx_alloc_vlapic_mapping() currently contains some very odd looking code >> that allocates a MEMF_no_owner domheap page and then shares with the guest >> as if it were a xenheap page. This then requires vmx_free_vlapic_mapping() >> to call a special function in the mm code: free_shared_domheap_page(). >> >> By using a 'normal' domheap page (i.e. by not passing MEMF_no_owner to >> alloc_domheap_page()), the odd looking code in vmx_alloc_vlapic_mapping() >> can simply use get_page_and_type() to set up a writable mapping before >> insertion in the P2M and vmx_free_vlapic_mapping() can simply release the >> page using put_page_alloc_ref() followed by put_page_and_type(). This >> then allows free_shared_domheap_page() to be purged. >> >> There is, however, some fall-out from this simplification: >> >> - alloc_domheap_page() will now call assign_pages() and run into the fact >> that 'max_pages' is not set until some time after domain_create(). To >> avoid an allocation failure, domain_create() is modified to set >> max_pages to an initial value, sufficient to cover any domheap >> allocations required to complete domain creation. The value will be >> set to the 'real' max_pages when the tool-stack later performs the >> XEN_DOMCTL_max_mem operation, thus allowing the rest of the domain's >> memory to be allocated. > > I continue to disagree with this approach, and I don't think I've > heard back on the alternative suggestion of using MEMF_no_refcount > instead of MEMF_no_owner. As said before, the page (and any other > ones up to DOMAIN_INIT_PAGES, which I find rather fragile to be > set to 1) will now get accounted against the amount allowed for > the domain to allocate. > > It also looks to me as if this will break e.g. > p2m_pod_set_mem_target(), which at the very top has > > /* P == B: Nothing to do (unless the guest is being created). */ > populated = d->tot_pages - p2m->pod.count; > if ( populated > 0 && p2m->pod.entry_count == 0 ) > goto out; > > This code assumes that during domain creation all pages recorded > in ->tot_pages have also been recorded in ->pod.count. > > There may be other uses of ->tot_pages which this change conflicts > with.
This basically boils down to the "memory accounting swamp", where various random features need to allocate domain pages to function. >> - Because the domheap page is no longer a pseudo-xenheap page, the >> reference counting will prevent the domain from being destroyed. Thus >> the call to vmx_free_vlapic_mapping() is moved from the >> domain_destroy() method into the domain_relinquish_resources() method. >> Whilst in the area, make the domain_destroy() method an optional >> alternative_vcall() (since it will no longer peform any function in VMX >> and is stubbed in SVM anyway). > > All of this would, afaict, become irrelevant when using MEMF_no_refcount. > > There looks to be one issue (which I think we have been discussing > before): By not bumping ->tot_pages, its decrementing upon freeing > the page will be a problem. I can see two possible solutions to this: > - Introduce a flag indicating there should also be no accounting > upon freeing of the page. This seems like a reasonable approach to look into. -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
