|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/2] xen/mm: limit non-scrubbed allocations to a specific order
On 08.01.2026 18:55, Roger Pau Monne wrote:
> 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@xxxxxxxxxx>
> ---
> 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.
How about stealing a bit from the range above MEMOP_EXTENT_SHIFT to
indicate that state, with the actual page (and order plus scrub progress)
recorded in the target struct domain? Actually, maybe such an indicator
isn't needed at all: If the next invocation (continuation or not) finds
an in-progress allocation, it could simply use that rather than doing a
real allocation. (What to do if this isn't a continuation is less clear:
We could fail such requests [likely not an option unless we can reliably
tell original requests from continuations], or split the allocation if
the request is smaller, or free the allocation to then take the normal
path.) All of which of course only for "foreign" requests.
If the hypercall is never continued, we could refuse to unpause the
domain (with the allocation then freed normally when the domain gets
destroyed).
As another alternative, how about returning unscrubbed pages altogether
when it's during domain creation, requiring the tool stack to do the
scrubbing (potentially allowing it to skip some of it when pages are
fully initialized anyway, much like we do for Dom0 iirc)?
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -279,6 +279,18 @@ 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;
> + }
At least for order 0 requests there's no point in trying this. With the
current logic, actually for orders up to MAX_DIRTY_ORDER.
Further, from a general interface perspective, wouldn't we need to do the
same for at least XENMEM_increase_reservation?
> @@ -1115,7 +1139,16 @@ 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++;
> }
Yet an alternative consideration: When "cold" is true, couldn't we call
process_pending_softirqs() like you do here ( >= SYS_STATE_smp_boot then
of course), without any of the other changes? Of course that's worse
than a proper continuation, especially from the calling domain's pov.
> @@ -223,6 +224,14 @@ 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
Using CONFIG_DOMU_MAX_ORDER rather than the command line overridable
domu_max_order means people couldn't even restore original behavior.
Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |