[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 5/7] mm: make MEMF_no_refcount pages safe to assign
On 28.01.2020 18:01, Durrant, Paul wrote: >> From: Jan Beulich <jbeulich@xxxxxxxx> >> Sent: 28 January 2020 15:23 >> >> On 24.01.2020 16:31, Paul Durrant wrote: >>> Currently it is unsafe to assign a domheap page allocated with >>> MEMF_no_refcount to a domain because the domain't 'tot_pages' will not >>> be incremented, but will be decrement when the page is freed (since >>> free_domheap_pages() has no way of telling that the increment was >> skipped). >>> >>> This patch allocates a new 'count_info' bit for a PGC_no_refcount flag >>> which is then used to mark domheap pages allocated with >> MEMF_no_refcount. >>> This then allows free_domheap_pages() to skip decrementing tot_pages >> when >>> appropriate and hence makes the pages safe to assign. >>> >>> NOTE: The patch sets MEMF_no_refcount directly in alloc_domheap_pages() >>> rather than in assign_pages() because the latter is called with >>> MEMF_no_refcount by memory_exchange() as an optimization, to avoid >>> too many calls to domain_adjust_tot_pages() (which acquires and >>> releases the global 'heap_lock'). >> >> I don't think there were any optimization thoughts with this. The >> MEMF_no_refcount use is because otherwise for a domain with >> tot_pages == max_pages the assignment would fail. >> > > That would not be the case if the calls to steal_page() further up didn't > pass MEMF_no_refcount (which would be the correct thing to do if not > passing it to assign_pages(). No, that's not an option either: steal_page() would otherwise decrement ->tot_pages, allowing the domain to allocate new memory on another vCPU. This would again result in the exchange failing for no reason. (And no, I don't think a guest should be required to serialize e.g. ballooning operations with exchanges.) >>> --- a/xen/common/page_alloc.c >>> +++ b/xen/common/page_alloc.c >>> @@ -460,6 +460,9 @@ unsigned long domain_adjust_tot_pages(struct domain >> *d, long pages) >>> { >>> long dom_before, dom_after, dom_claimed, sys_before, sys_after; >>> >>> + if ( !pages ) >>> + goto out; >> >> Unrelated change? Are there, in fact, any callers passing in 0? >> Oh, further down you add one which may do so, but then perhaps >> better to make the caller not call here (as is done e.g. in >> memory_exchange())? > > I think it's preferable for domain_adjust_tot_pages() to handle zero > gracefully. That's an option, but imo would then better be a separate change (to also drop present guards of calls to the function). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |