Hello, In XenServer we have seen the watchdog occasionally triggering during domain creation if 1GB pages are scrubbed in-place during physmap population. The following series attempt to mitigate this by limiting the in-place scrubbing during allocation to 2M pages, but it has some drawbacks, see the post-commit remarks in patch 2. I'm hopping someone might have a better idea, or we converge we can't do better than this for the time being. Thanks, Roger. Roger Pau Monne (2): xen/mm: add a NUMA node parameter to scrub_free_pages() xen/mm: limit non-scrubbed allocations to a specific order xen/arch/arm/domain.c | 2 +- xen/arch/x86/domain.c | 2 +- xen/common/memory.c | 12 +++++++++ xen/common/page_alloc.c | 54 +++++++++++++++++++++++++++++++++++++---- xen/include/xen/mm.h | 12 ++++++++- 5 files changed, 74 insertions(+), 8 deletions(-) -- 2.51.0 Such parameter allow requesting to scrub memory only from the specified node. If there's no memory to scrub from the requested node the function returns false. If the node is already being scrubbed from a different CPU the function returns true so the caller can differentiate whether there's still pending work to do. No functional change intended. Existing callers are switched to use the new interface, albeit they all pass NUMA_NO_NODE to keep the current behavior. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/arm/domain.c | 2 +- xen/arch/x86/domain.c | 2 +- xen/common/page_alloc.c | 17 ++++++++++++++--- xen/include/xen/mm.h | 3 ++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -XXX,XX +XXX,XX @@ static void noreturn idle_loop(void) * and then, after it is done, whether softirqs became pending * while we were scrubbing. */ - else if ( !softirq_pending(cpu) && !scrub_free_pages() && + else if ( !softirq_pending(cpu) && !scrub_free_pages(NUMA_NO_NODE) && !softirq_pending(cpu) ) do_idle(); diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -XXX,XX +XXX,XX @@ static void noreturn cf_check idle_loop(void) * and then, after it is done, whether softirqs became pending * while we were scrubbing. */ - else if ( !softirq_pending(cpu) && !scrub_free_pages() && + else if ( !softirq_pending(cpu) && !scrub_free_pages(NUMA_NO_NODE) && !softirq_pending(cpu) ) { if ( guest ) diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -XXX,XX +XXX,XX @@ static void cf_check scrub_continue(void *data) } } -bool scrub_free_pages(void) +bool scrub_free_pages(nodeid_t node) { struct page_info *pg; unsigned int zone; unsigned int cpu = smp_processor_id(); bool preempt = false; - nodeid_t node; unsigned int cnt = 0; - node = node_to_scrub(true); + if ( node != NUMA_NO_NODE ) + { + if ( !node_need_scrub[node] ) + /* Nothing to scrub. */ + return false; + + if ( node_test_and_set(node, node_scrubbing) ) + /* Another CPU is scrubbing it. */ + return true; + } + else + node = node_to_scrub(true); + if ( node == NUMA_NO_NODE ) return false; diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -XXX,XX +XXX,XX @@ #include <xen/compiler.h> #include <xen/mm-frame.h> #include <xen/mm-types.h> +#include <xen/numa.h> #include <xen/types.h> #include <xen/list.h> #include <xen/spinlock.h> @@ -XXX,XX +XXX,XX @@ void init_xenheap_pages(paddr_t ps, paddr_t pe); void xenheap_max_mfn(unsigned long mfn); void *alloc_xenheap_pages(unsigned int order, unsigned int memflags); void free_xenheap_pages(void *v, unsigned int order); -bool scrub_free_pages(void); +bool scrub_free_pages(nodeid_t node); #define alloc_xenheap_page() (alloc_xenheap_pages(0,0)) #define free_xenheap_page(v) (free_xenheap_pages(v,0)) -- 2.51.0 The current model of falling back to allocate unscrubbed pages and scrub them in place at allocation time risks triggering the watchdog: Watchdog timer detects that CPU55 is stuck! ----[ Xen-4.17.5-21 x86_64 debug=n Not tainted ]---- CPU: 55 RIP: e008:[<ffff82d040204c4a>] clear_page_sse2+0x1a/0x30 RFLAGS: 0000000000000202 CONTEXT: hypervisor (d0v12) [...] Xen call trace: [<ffff82d040204c4a>] R clear_page_sse2+0x1a/0x30 [<ffff82d04022a121>] S clear_domain_page+0x11/0x20 [<ffff82d04022c170>] S common/page_alloc.c#alloc_heap_pages+0x400/0x5a0 [<ffff82d04022d4a7>] S alloc_domheap_pages+0x67/0x180 [<ffff82d040226f9f>] S common/memory.c#populate_physmap+0x22f/0x3b0 [<ffff82d040228ec8>] S do_memory_op+0x728/0x1970 The maximum allocation order on x86 is limited to 18, that means allocating and scrubbing possibly 1G worth of memory in 4K chunks. Start by limiting dirty allocations to CONFIG_DOMU_MAX_ORDER, which is currently set to 2M chunks. However such limitation might cause fragmentation in HVM p2m population during domain creation. To prevent that introduce some extra logic in populate_physmap() that fallback to preemptive page-scrubbing if the requested allocation cannot be fulfilled and there's scrubbing work to do. This approach is less fair than the current one, but allows preemptive page scrubbing in the context of populate_physmap() to attempt to ensure unnecessary page-shattering. Fixes: 74d2e11ccfd2 ("mm: Scrub pages in alloc_heap_pages() if needed") Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- I'm not particularly happy with this approach, as it doesn't guarantee progress for the callers. IOW: a caller might do a lot of scrubbing, just to get it's pages stolen by a different concurrent thread doing allocations. However I'm not sure there's a better solution than resorting to 2M allocations if there's not enough free memory that is scrubbed. I'm having trouble seeing where we could temporary store page(s) allocated that need to be scrubbed before being assigned to the domain, in a way that can be used by continuations, and that would allow Xen to keep track of them in case the operation is never finished. IOW: we would need to account for cleanup of such temporary stash of pages in case the domain never completes the hypercall, or is destroyed midway. Otherwise we could add the option to switch back to scrubbing before returning the pages to the free pool, but that's also problematic: the current approach aim to scrub pages in the same NUMA node as the CPU that's doing the scrubbing. If we scrub in the context of the domain destruction hypercall there's no attempt to scrub pages in the local NUMA node. --- xen/common/memory.c | 12 ++++++++++++ xen/common/page_alloc.c | 37 +++++++++++++++++++++++++++++++++++-- xen/include/xen/mm.h | 9 +++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/xen/common/memory.c b/xen/common/memory.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -XXX,XX +XXX,XX @@ static void populate_physmap(struct memop_args *a) if ( unlikely(!page) ) { + nodeid_t node = MEMF_get_node(a->memflags); + + if ( memory_scrub_pending(node) || + (node != NUMA_NO_NODE && + !(a->memflags & MEMF_exact_node) && + memory_scrub_pending(node = NUMA_NO_NODE)) ) + { + scrub_free_pages(node); + a->preempted = 1; + goto out; + } + gdprintk(XENLOG_INFO, "Could not allocate order=%u extent: id=%d memflags=%#x (%u of %u)\n", a->extent_order, d->domain_id, a->memflags, diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -XXX,XX +XXX,XX @@ static heap_by_zone_and_order_t *_heap[MAX_NUMNODES]; static unsigned long node_need_scrub[MAX_NUMNODES]; +bool memory_scrub_pending(nodeid_t node) +{ + nodeid_t i; + + if ( node != NUMA_NO_NODE ) + return node_need_scrub[node]; + + for_each_online_node ( i ) + if ( node_need_scrub[i] ) + return true; + + return false; +} + static unsigned long *avail[MAX_NUMNODES]; static long total_avail_pages; @@ -XXX,XX +XXX,XX @@ static struct page_info *alloc_heap_pages( } pg = get_free_buddy(zone_lo, zone_hi, order, memflags, d); - /* Try getting a dirty buddy if we couldn't get a clean one. */ - if ( !pg && !(memflags & MEMF_no_scrub) ) + /* + * Try getting a dirty buddy if we couldn't get a clean one. Limit the + * fallback to orders equal or below MAX_DIRTY_ORDER, as otherwise the + * non-preemptive scrubbing could trigger the watchdog. + */ + if ( !pg && !(memflags & MEMF_no_scrub) && + /* + * Allow any order unscrubbed allocations during boot time, we + * compensate by processing softirqs in the scrubbing loop below once + * irqs are enabled. + */ + (order <= MAX_DIRTY_ORDER || system_state < SYS_STATE_active) ) pg = get_free_buddy(zone_lo, zone_hi, order, memflags | MEMF_no_scrub, d); if ( !pg ) @@ -XXX,XX +XXX,XX @@ static struct page_info *alloc_heap_pages( if ( test_and_clear_bit(_PGC_need_scrub, &pg[i].count_info) ) { if ( !(memflags & MEMF_no_scrub) ) + { scrub_one_page(&pg[i], cold); + /* + * Use SYS_STATE_smp_boot explicitly; ahead of that state + * interrupts are disabled. + */ + if ( system_state == SYS_STATE_smp_boot && + !(dirty_cnt & 0xff) ) + process_pending_softirqs(); + } dirty_cnt++; } diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -XXX,XX +XXX,XX @@ void xenheap_max_mfn(unsigned long mfn); void *alloc_xenheap_pages(unsigned int order, unsigned int memflags); void free_xenheap_pages(void *v, unsigned int order); bool scrub_free_pages(nodeid_t node); +bool memory_scrub_pending(nodeid_t node); #define alloc_xenheap_page() (alloc_xenheap_pages(0,0)) #define free_xenheap_page(v) (free_xenheap_pages(v,0)) @@ -XXX,XX +XXX,XX @@ struct npfec { #else #define MAX_ORDER 20 /* 2^20 contiguous pages */ #endif + +/* Max order when scrubbing pages at allocation time. */ +#ifdef CONFIG_DOMU_MAX_ORDER +# define MAX_DIRTY_ORDER CONFIG_DOMU_MAX_ORDER +#else +# define MAX_DIRTY_ORDER 9 +#endif + mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags); /* Private domain structs for DOMID_XEN, DOMID_IO, etc. */ -- 2.51.0 Hello, In XenServer we have seen the watchdog occasionally triggering during domain creation if 1GB pages are scrubbed in-place during physmap population. The following series attempt to mitigate this by adding preemption to page scrubbing in populate_physmap(). Also a new limit and command line option to signal the maximum allocation order when doing in-place scrubbing. This is set by default to CONFIG_PTDOM_MAX_ORDER. Thanks, Roger. Roger Pau Monne (3): xen/mm: enforce SCRUB_DEBUG checks for MEMF_no_scrub allocations xen/mm: allow deferred scrub of physmap populate allocated pages xen/mm: limit non-scrubbed allocations to a specific order docs/misc/xen-command-line.pandoc | 13 ++++ xen/common/domain.c | 30 +++++++++ xen/common/memory.c | 104 ++++++++++++++++++++++++++++-- xen/common/page_alloc.c | 30 +++++++-- xen/include/xen/mm.h | 14 ++++ xen/include/xen/sched.h | 5 ++ 6 files changed, 187 insertions(+), 9 deletions(-) -- 2.51.0 The logic in alloc_heap_pages() only checks for scrubbing pattern correctness when the caller doesn't pass MEMF_no_scrub in memflags. However already scrubbed pages can be checked for correctness, regardless of the caller having requested MEMF_no_scrub. Relax the checking around the check_one_page() call, to allow for calls with MEMF_no_scrub to also check the correctness of pages marked as already scrubbed when allocated. This widens the checking of scrubbing correctness, so it would also check the scrubbing correctness of MEMF_no_scrub allocations. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- After discussing with Jan I've deliberately omitted the tag: Fixes: 0c5f2f9cefac ("mm: Make sure pages are scrubbed") The intended approach might have been to ensure the caller of alloc_heap_pages() gets properly scrubbed pages, rather than asserting the internal state of free pages is as expected. --- xen/common/page_alloc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -XXX,XX +XXX,XX @@ static struct page_info *alloc_heap_pages( spin_unlock(&heap_lock); - if ( first_dirty != INVALID_DIRTY_IDX || - (scrub_debug && !(memflags & MEMF_no_scrub)) ) + if ( first_dirty != INVALID_DIRTY_IDX || scrub_debug ) { bool cold = d && d != current->domain; @@ -XXX,XX +XXX,XX @@ static struct page_info *alloc_heap_pages( dirty_cnt++; } - else if ( !(memflags & MEMF_no_scrub) ) + else check_one_page(&pg[i]); } -- 2.51.0 Physmap population has the need to use pages as big as possible to reduce p2m shattering. However that triggers issues when big enough pages are not yet scrubbed, and so scrubbing must be done at allocation time. On some scenarios with added contention the watchdog can trigger: Watchdog timer detects that CPU55 is stuck! ----[ Xen-4.17.5-21 x86_64 debug=n Not tainted ]---- CPU: 55 RIP: e008:[<ffff82d040204c4a>] clear_page_sse2+0x1a/0x30 RFLAGS: 0000000000000202 CONTEXT: hypervisor (d0v12) [...] Xen call trace: [<ffff82d040204c4a>] R clear_page_sse2+0x1a/0x30 [<ffff82d04022a121>] S clear_domain_page+0x11/0x20 [<ffff82d04022c170>] S common/page_alloc.c#alloc_heap_pages+0x400/0x5a0 [<ffff82d04022d4a7>] S alloc_domheap_pages+0x67/0x180 [<ffff82d040226f9f>] S common/memory.c#populate_physmap+0x22f/0x3b0 [<ffff82d040228ec8>] S do_memory_op+0x728/0x1970 Introduce a mechanism to preempt page scrubbing in populate_physmap(). It relies on stashing the dirty page in the domain struct temporarily to preempt to guest context, so the scrubbing can resume when the domain re-enters the hypercall. The added deferral mechanism will only be used for domain construction, and is designed to be used with a single threaded domain builder. If the toolstack makes concurrent calls to XENMEM_populate_physmap for the same target domain it will trash stashed pages, resulting in slow domain physmap population. Note a similar issue is present in increase reservation. However that hypercall is likely to only be used once the domain is already running and the known implementations use 4K pages. It will be deal with in a separate patch using a different approach, that will also take care of the allocation in populate_physmap() once the domain is running. Fixes: 74d2e11ccfd2 ("mm: Scrub pages in alloc_heap_pages() if needed") Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v3: - Introduce helper to free stashed pages. - Attempt to free stashed pages from domain_unpause_by_systemcontroller() also. - Free stashed page in get_stashed_allocation() if it doesn't match the requested parameters. Changes since v2: - Introduce FREE_DOMHEAP_PAGE{,S}(). - Remove j local counter. - Free page pending scrub in domain_kill() also. - Remove BUG_ON(). - Reorder get_stashed_allocation() flow. - s/dirty/unscrubbed/ in a printk message. Changes since v1: - New in this version, different approach than v1. --- xen/common/domain.c | 30 ++++++++++++ xen/common/memory.c | 101 +++++++++++++++++++++++++++++++++++++++- xen/common/page_alloc.c | 2 +- xen/include/xen/mm.h | 10 ++++ xen/include/xen/sched.h | 5 ++ 5 files changed, 146 insertions(+), 2 deletions(-) diff --git a/xen/common/domain.c b/xen/common/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -XXX,XX +XXX,XX @@ static int domain_teardown(struct domain *d) return 0; } +/* + * Called multiple times during domain destruction, to attempt to early free + * any stashed pages to be scrubbed. The call from _domain_destroy() is done + * when the toolstack can no longer stash any pages. + */ +static void domain_free_pending_scrub(struct domain *d) +{ + rspin_lock(&d->page_alloc_lock); + if ( d->pending_scrub ) + { + FREE_DOMHEAP_PAGES(d->pending_scrub, d->pending_scrub_order); + d->pending_scrub_order = 0; + d->pending_scrub_index = 0; + } + rspin_unlock(&d->page_alloc_lock); +} + /* * Destroy a domain once all references to it have been dropped. Used either * from the RCU path, or from the domain_create() error path before the domain @@ -XXX,XX +XXX,XX @@ static void _domain_destroy(struct domain *d) XVFREE(d->console); + domain_free_pending_scrub(d); + argo_destroy(d); rangeset_domain_destroy(d); @@ -XXX,XX +XXX,XX @@ int domain_kill(struct domain *d) rspin_barrier(&d->domain_lock); argo_destroy(d); vnuma_destroy(d->vnuma); + domain_free_pending_scrub(d); + rspin_unlock(&d->page_alloc_lock); domain_set_outstanding_pages(d, 0); /* fallthrough */ case DOMDYING_dying: @@ -XXX,XX +XXX,XX @@ int domain_unpause_by_systemcontroller(struct domain *d) */ if ( new == 0 && !d->creation_finished ) { + if ( d->pending_scrub ) + { + printk(XENLOG_ERR + "%pd: cannot be started with pending unscrubbed pages, destroying\n", + d); + domain_crash(d); + domain_free_pending_scrub(d); + return -EBUSY; + } d->creation_finished = true; arch_domain_creation_finished(d); } diff --git a/xen/common/memory.c b/xen/common/memory.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -XXX,XX +XXX,XX @@ static void increase_reservation(struct memop_args *a) a->nr_done = i; } +/* + * Temporary storage for a domain assigned page that's not been fully scrubbed. + * Stored pages must be domheap ones. + * + * The stashed page can be freed at any time by Xen, the caller must pass the + * order and NUMA node requirement to the fetch function to ensure the + * currently stashed page matches it's requirements. + */ +static void stash_allocation(struct domain *d, struct page_info *page, + unsigned int order, unsigned int scrub_index) +{ + rspin_lock(&d->page_alloc_lock); + + /* + * Drop the passed page in preference for the already stashed one. This + * interface is designed to be used for single-threaded domain creation. + */ + if ( d->pending_scrub ) + free_domheap_pages(page, order); + else + { + d->pending_scrub_index = scrub_index; + d->pending_scrub_order = order; + d->pending_scrub = page; + } + + rspin_unlock(&d->page_alloc_lock); +} + +static struct page_info *get_stashed_allocation(struct domain *d, + unsigned int order, + nodeid_t node, + unsigned int *scrub_index) +{ + struct page_info *page = NULL; + + rspin_lock(&d->page_alloc_lock); + + /* + * If there's a pending page to scrub check if it satisfies the current + * request. If it doesn't free it and return NULL. + */ + if ( d->pending_scrub && d->pending_scrub_order == order && + (node == NUMA_NO_NODE || node == page_to_nid(d->pending_scrub)) ) + { + page = d->pending_scrub; + *scrub_index = d->pending_scrub_index; + } + else + free_domheap_pages(d->pending_scrub, d->pending_scrub_order); + + /* + * The caller now owns the page or it has been freed, clear stashed + * information. Prevent concurrent usages of get_stashed_allocation() + * from returning the same page to different contexts. + */ + d->pending_scrub_index = 0; + d->pending_scrub_order = 0; + d->pending_scrub = NULL; + + rspin_unlock(&d->page_alloc_lock); + return page; +} + static void populate_physmap(struct memop_args *a) { struct page_info *page; @@ -XXX,XX +XXX,XX @@ static void populate_physmap(struct memop_args *a) } else { - page = alloc_domheap_pages(d, a->extent_order, a->memflags); + unsigned int scrub_start = 0; + nodeid_t node = + (a->memflags & MEMF_exact_node) ? MEMF_get_node(a->memflags) + : NUMA_NO_NODE; + + page = get_stashed_allocation(d, a->extent_order, node, + &scrub_start); + + if ( !page ) + page = alloc_domheap_pages(d, a->extent_order, + a->memflags | (d->creation_finished ? 0 + : MEMF_no_scrub)); if ( unlikely(!page) ) { @@ -XXX,XX +XXX,XX @@ static void populate_physmap(struct memop_args *a) goto out; } + if ( !d->creation_finished ) + { + unsigned int dirty_cnt = 0; + + /* Check if there's anything to scrub. */ + for ( j = scrub_start; j < (1U << a->extent_order); j++ ) + { + if ( !test_and_clear_bit(_PGC_need_scrub, + &page[j].count_info) ) + continue; + + scrub_one_page(&page[j], true); + + if ( (j + 1) != (1U << a->extent_order) && + !(++dirty_cnt & 0xff) && + hypercall_preempt_check() ) + { + a->preempted = 1; + stash_allocation(d, page, a->extent_order, ++j); + goto out; + } + } + } + if ( unlikely(a->memflags & MEMF_no_tlbflush) ) { for ( j = 0; j < (1U << a->extent_order); j++ ) diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -XXX,XX +XXX,XX @@ static void page_list_add_scrub(struct page_info *pg, unsigned int node, # define scrub_page_cold clear_page_cold #endif -static void scrub_one_page(const struct page_info *pg, bool cold) +void scrub_one_page(const struct page_info *pg, bool cold) { void *ptr; diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -XXX,XX +XXX,XX @@ unsigned long avail_node_heap_pages(unsigned int nodeid); #define alloc_domheap_page(d,f) (alloc_domheap_pages(d,0,f)) #define free_domheap_page(p) (free_domheap_pages(p,0)) +/* Free an allocation, and zero the pointer to it. */ +#define FREE_DOMHEAP_PAGES(p, o) do { \ + void *_ptr_ = (p); \ + (p) = NULL; \ + free_domheap_pages(_ptr_, o); \ +} while ( false ) +#define FREE_DOMHEAP_PAGE(p) FREE_DOMHEAP_PAGES(p, 0) + +void scrub_one_page(const struct page_info *pg, bool cold); + int online_page(mfn_t mfn, uint32_t *status); int offline_page(mfn_t mfn, int broken, uint32_t *status); int query_page_offline(mfn_t mfn, uint32_t *status); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -XXX,XX +XXX,XX @@ struct domain /* Pointer to console settings; NULL for system domains. */ struct domain_console *console; + + /* Pointer to allocated domheap page that possibly needs scrubbing. */ + struct page_info *pending_scrub; + unsigned int pending_scrub_order; + unsigned int pending_scrub_index; } __aligned(PAGE_SIZE); static inline struct page_list_head *page_to_list( -- 2.51.0 The current logic allows for up to 1G pages to be scrubbed in place, which can cause the watchdog to trigger in practice. Reduce the limit for in-place scrubbed allocations to a newly introduced define: CONFIG_DIRTY_MAX_ORDER. This currently defaults to CONFIG_PTDOM_MAX_ORDER on all architectures. Also introduce a command line option to set the value. Fixes: 74d2e11ccfd2 ("mm: Scrub pages in alloc_heap_pages() if needed") Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v3: - Note the order limitation is only post-boot. Changes since v2: - Move placement of the max-order-dirty option help. - Add note in memop-max-order about interactions. - Use CONFIG_PTDOM_MAX_ORDER as the default. Changes since v1: - Split from previous patch. - Introduce a command line option to set the limit. --- docs/misc/xen-command-line.pandoc | 13 +++++++++++++ xen/common/memory.c | 3 --- xen/common/page_alloc.c | 23 ++++++++++++++++++++++- xen/include/xen/mm.h | 4 ++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index XXXXXXX..XXXXXXX 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -XXX,XX +XXX,XX @@ presented as the number of bits needed to encode it. This must be at least one pending bit to be allocated. Defaults to 20 bits (to cover at most 1048576 interrupts). +### max-order-dirty +> `= <integer>` + +Specify the maximum allocation order allowed when scrubbing allocated pages +in-place. The allocation is non-preemptive, and hence the value must be keep +low enough to avoid hogging the CPU for too long. + +Defaults to `CONFIG_DIRTY_MAX_ORDER` or if unset to `CONFIG_PTDOM_MAX_ORDER`. +Note those are internal per-architecture defines not available from Kconfig. + ### mce (x86) > `= <boolean>` @@ -XXX,XX +XXX,XX @@ requests issued by the various kinds of domains (in this order: ordinary DomU, control domain, hardware domain, and - when supported by the platform - DomU with pass-through device assigned). +Note orders here can be further limited by the value in `max-order-dirty` for +allocations requesting pages to be scrubbed in-place. + ### mmcfg (x86) > `= <boolean>[,amd-fam10]` diff --git a/xen/common/memory.c b/xen/common/memory.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -XXX,XX +XXX,XX @@ struct memop_args { #ifndef CONFIG_CTLDOM_MAX_ORDER #define CONFIG_CTLDOM_MAX_ORDER CONFIG_PAGEALLOC_MAX_ORDER #endif -#ifndef CONFIG_PTDOM_MAX_ORDER -#define CONFIG_PTDOM_MAX_ORDER CONFIG_HWDOM_MAX_ORDER -#endif static unsigned int __read_mostly domu_max_order = CONFIG_DOMU_MAX_ORDER; static unsigned int __read_mostly ctldom_max_order = CONFIG_CTLDOM_MAX_ORDER; diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -XXX,XX +XXX,XX @@ static PAGE_LIST_HEAD(page_offlined_list); /* Broken page list, protected by heap_lock. */ static PAGE_LIST_HEAD(page_broken_list); +/* Maximum order allowed for post-boot allocations with MEMF_no_scrub. */ +#ifndef CONFIG_DIRTY_MAX_ORDER +# define CONFIG_DIRTY_MAX_ORDER CONFIG_PTDOM_MAX_ORDER +#endif +static unsigned int __ro_after_init dirty_max_order = CONFIG_DIRTY_MAX_ORDER; +integer_param("max-order-dirty", dirty_max_order); + /************************* * BOOT-TIME ALLOCATOR */ @@ -XXX,XX +XXX,XX @@ static struct page_info *alloc_heap_pages( pg = get_free_buddy(zone_lo, zone_hi, order, memflags, d); /* Try getting a dirty buddy if we couldn't get a clean one. */ - if ( !pg && !(memflags & MEMF_no_scrub) ) + if ( !pg && !(memflags & MEMF_no_scrub) && + /* + * Allow any order unscrubbed allocations during boot time, we + * compensate by processing softirqs in the scrubbing loop below once + * irqs are enabled. + */ + (order <= dirty_max_order || system_state < SYS_STATE_active) ) pg = get_free_buddy(zone_lo, zone_hi, order, memflags | MEMF_no_scrub, d); if ( !pg ) @@ -XXX,XX +XXX,XX @@ static struct page_info *alloc_heap_pages( scrub_one_page(&pg[i], cold); dirty_cnt++; + + /* + * Use SYS_STATE_smp_boot explicitly; ahead of that state + * interrupts are disabled. + */ + if ( system_state == SYS_STATE_smp_boot && + !(dirty_cnt & 0xff) ) + process_pending_softirqs(); } else check_one_page(&pg[i]); diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -XXX,XX +XXX,XX @@ struct npfec { #else #define MAX_ORDER 20 /* 2^20 contiguous pages */ #endif +#ifndef CONFIG_PTDOM_MAX_ORDER +# define CONFIG_PTDOM_MAX_ORDER CONFIG_HWDOM_MAX_ORDER +#endif + mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags); /* Private domain structs for DOMID_XEN, DOMID_IO, etc. */ -- 2.51.0