Messages in this thread | | | Subject | Re: [PATCH V2 3/4] xen/unpopulated-alloc: Add mechanism to use Xen resource | From | Oleksandr <> | Date | Wed, 24 Nov 2021 11:33:29 +0200 |
| |
On 23.11.21 23:25, Stefano Stabellini wrote:
Hi Stefano
> On Tue, 23 Nov 2021, Oleksandr wrote: >>> Actually after reading your replies and explanation I changed opinion: I >>> think we do need the fallback because Linux cannot really assume that >>> it is running on "new Xen" so it definitely needs to keep working if >>> CONFIG_XEN_UNPOPULATED_ALLOC is enabled and the extended regions are not >>> advertised. >>> >>> I think we'll have to roll back some of the changes introduced by >>> 121f2faca2c0a. That's because even if CONFIG_XEN_UNPOPULATED_ALLOC is >>> enabled we cannot know if we can use unpopulated-alloc or whether we >>> have to use alloc_xenballooned_pages until we parse the /hypervisor node >>> in device tree at runtime. >> Exactly! >> >> >>> In short, we cannot switch between unpopulated-alloc and >>> alloc_xenballooned_pages at build time, we have to do it at runtime >>> (boot time). >> +1 >> >> >> I created a patch to partially revert 121f2faca2c0a "xen/balloon: rename >> alloc/free_xenballooned_pages". >> >> If there is no objections I will add it to V3 (which is almost ready, except >> the fallback bits). Could you please tell me what do you think? > > It makes sense to me. You can add my Reviewed-by.
Great, thank you!
> > >> From dc79bcd425358596d95e715a8bd8b81deaaeb703 Mon Sep 17 00:00:00 2001 >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> Date: Tue, 23 Nov 2021 18:14:41 +0200 >> Subject: [PATCH] xen/balloon: Bring alloc(free)_xenballooned_pages helpers >> back >> >> This patch rolls back some of the changes introduced by commit >> 121f2faca2c0a "xen/balloon: rename alloc/free_xenballooned_pages" >> in order to make possible to still allocate xenballooned pages >> if CONFIG_XEN_UNPOPULATED_ALLOC is enabled. >> >> On Arm the unpopulated pages will be allocated on top of extended >> regions provided by Xen via device-tree (the subsequent patches >> will add required bits to support unpopulated-alloc feature on Arm). >> The problem is that extended regions feature has been introduced >> into Xen quite recently (during 4.16 release cycle). So this >> effectively means that Linux must only use unpopulated-alloc on Arm >> if it is running on "new Xen" which advertises these regions. >> But, it will only be known after parsing the "hypervisor" node >> at boot time, so before doing that we cannot assume anything. >> >> In order to keep working if CONFIG_XEN_UNPOPULATED_ALLOC is enabled >> and the extended regions are not advertised (Linux is running on >> "old Xen", etc) we need the fallback to alloc_xenballooned_pages(). >> >> This way we wouldn't reduce the amount of memory usable (wasting >> RAM pages) for any of the external mappings anymore (and eliminate >> XSA-300) with "new Xen", but would be still functional ballooning >> out RAM pages with "old Xen". >> >> Also rename alloc(free)_xenballooned_pages to xen_alloc(free)_ballooned_pages. >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> --- >> drivers/xen/balloon.c | 20 +++++++++----------- >> include/xen/balloon.h | 3 +++ >> include/xen/xen.h | 6 ++++++ >> 3 files changed, 18 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c >> index ba2ea11..a2c4fc49 100644 >> --- a/drivers/xen/balloon.c >> +++ b/drivers/xen/balloon.c >> @@ -581,7 +581,6 @@ void balloon_set_new_target(unsigned long target) >> } >> EXPORT_SYMBOL_GPL(balloon_set_new_target); >> >> -#ifndef CONFIG_XEN_UNPOPULATED_ALLOC >> static int add_ballooned_pages(unsigned int nr_pages) >> { >> enum bp_state st; >> @@ -610,12 +609,12 @@ static int add_ballooned_pages(unsigned int nr_pages) >> } >> >> /** >> - * xen_alloc_unpopulated_pages - get pages that have been ballooned out >> + * xen_alloc_ballooned_pages - get pages that have been ballooned out >> * @nr_pages: Number of pages to get >> * @pages: pages returned >> * @return 0 on success, error otherwise >> */ >> -int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages) >> +int xen_alloc_ballooned_pages(unsigned int nr_pages, struct page **pages) >> { >> unsigned int pgno = 0; >> struct page *page; >> @@ -652,23 +651,23 @@ int xen_alloc_unpopulated_pages(unsigned int nr_pages, >> struct page **pages) >> return 0; >> out_undo: >> mutex_unlock(&balloon_mutex); >> - xen_free_unpopulated_pages(pgno, pages); >> + xen_free_ballooned_pages(pgno, pages); >> /* >> - * NB: free_xenballooned_pages will only subtract pgno pages, but since >> + * NB: xen_free_ballooned_pages will only subtract pgno pages, but since >> * target_unpopulated is incremented with nr_pages at the start we need >> * to remove the remaining ones also, or accounting will be screwed. >> */ >> balloon_stats.target_unpopulated -= nr_pages - pgno; >> return ret; >> } >> -EXPORT_SYMBOL(xen_alloc_unpopulated_pages); >> +EXPORT_SYMBOL(xen_alloc_ballooned_pages); >> >> /** >> - * xen_free_unpopulated_pages - return pages retrieved with >> get_ballooned_pages >> + * xen_free_ballooned_pages - return pages retrieved with get_ballooned_pages >> * @nr_pages: Number of pages >> * @pages: pages to return >> */ >> -void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages) >> +void xen_free_ballooned_pages(unsigned int nr_pages, struct page **pages) >> { >> unsigned int i; >> >> @@ -687,9 +686,9 @@ void xen_free_unpopulated_pages(unsigned int nr_pages, >> struct page **pages) >> >> mutex_unlock(&balloon_mutex); >> } >> -EXPORT_SYMBOL(xen_free_unpopulated_pages); >> +EXPORT_SYMBOL(xen_free_ballooned_pages); >> >> -#if defined(CONFIG_XEN_PV) >> +#if defined(CONFIG_XEN_PV) && !defined(CONFIG_XEN_UNPOPULATED_ALLOC) >> static void __init balloon_add_region(unsigned long start_pfn, >> unsigned long pages) >> { >> @@ -712,7 +711,6 @@ static void __init balloon_add_region(unsigned long >> start_pfn, >> balloon_stats.total_pages += extra_pfn_end - start_pfn; >> } >> #endif >> -#endif >> >> static int __init balloon_init(void) >> { >> diff --git a/include/xen/balloon.h b/include/xen/balloon.h >> index e93d4f0..f78a6cc 100644 >> --- a/include/xen/balloon.h >> +++ b/include/xen/balloon.h >> @@ -26,6 +26,9 @@ extern struct balloon_stats balloon_stats; >> >> void balloon_set_new_target(unsigned long target); >> >> +int xen_alloc_ballooned_pages(unsigned int nr_pages, struct page **pages); >> +void xen_free_ballooned_pages(unsigned int nr_pages, struct page **pages); >> + >> #ifdef CONFIG_XEN_BALLOON >> void xen_balloon_init(void); >> #else >> diff --git a/include/xen/xen.h b/include/xen/xen.h >> index 9f031b5..410e3e4 100644 >> --- a/include/xen/xen.h >> +++ b/include/xen/xen.h >> @@ -52,7 +52,13 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1, >> extern u64 xen_saved_max_mem_size; >> #endif >> >> +#ifdef CONFIG_XEN_UNPOPULATED_ALLOC >> int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages); >> void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages); >> +#else >> +#define xen_alloc_unpopulated_pages xen_alloc_ballooned_pages >> +#define xen_free_unpopulated_pages xen_free_ballooned_pages >> +#include <xen/balloon.h> >> +#endif >> >> #endif /* _XEN_XEN_H */
-- Regards,
Oleksandr Tyshchenko
| |