Re: [PATCH v3 02/23] xen: arm: update p2m_set_allocation() prototype

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/06/2026 12:17 pm, Oleksii Kurochko wrote:
> p2m_set_allocation() signals preemption redundantly: via a bool *preempted
> out-argument (set to true) and via -ERESTART return code, both at the same
> time. This led to the caller-side ASSERT(preempted == (rc == -ERESTART))
> added solely to document their agreement.

Well no, it's not redundant.  A NULL pointer is used to signal that
preemption is not permitted/available in the current context.

You notice this below, but it does invalidate the description given in
this paragraph.

> diff --git a/xen/arch/arm/mmu/p2m.c b/xen/arch/arm/mmu/p2m.c
> index 51abf3504fcf..e5c6be7c0890 100644
> --- a/xen/arch/arm/mmu/p2m.c
> +++ b/xen/arch/arm/mmu/p2m.c
> @@ -133,27 +130,24 @@ int arch_set_paging_mempool_size(struct domain *d, 
> uint64_t size)
>          return -EINVAL;
>  
>      spin_lock(&d->arch.paging.lock);
> -    rc = p2m_set_allocation(d, pages, &preempted);
> +    rc = p2m_set_allocation(d, pages, true);
>      spin_unlock(&d->arch.paging.lock);
> diff --git a/xen/common/device-tree/dom0less-build.c 
> b/xen/common/device-tree/dom0less-build.c
> index eacfd93087ae..c3818ffed45f 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -747,7 +747,7 @@ static int __init domain_p2m_set_allocation(struct domain 
> *d, uint64_t mem,
>                  domain_p2m_pages(mem, d->max_vcpus);
>  
>      spin_lock(&d->arch.paging.lock);
> -    rc = p2m_set_allocation(d, p2m_pages, NULL);
> +    rc = p2m_set_allocation(d, p2m_pages, false);
>      spin_unlock(&d->arch.paging.lock);

Passing booleans like this makes the code unnecessarily hard to follow.

At least use ", /* can_preempt */ true);" so the context is available
directly to the reader.

~Andrew