[PATCH v6 03/23] xen: arm: update p2m_set_allocation() prototype
Oleksii Kurochko <[email protected]>
| Newsgroups | org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <2ebda180488310a6f862242cb173ffc9b106756e.1784559209.git.oleksii.kurochko@gmail.com> |
p2m_set_allocation() uses a bool *preempted out-argument that overloads two meanings. When non-NULL, the value written back (true) duplicates information already carried by the -ERESTART return code — pure redundancy, which the caller-side ASSERT(preempted == (rc == -ERESTART)) only documents. Separately, a NULL pointer is an implicit calling convention meaning "preemption is not permitted in this context". Replace the pointer with a plain bool can_preempt that explicitly controls whether the preemption check runs, making the NULL-to-suppress convention type-safe and self-documenting, and rely on the -ERESTART return code alone to report that preemption occurred. Since p2m_set_allocation() is called by the common dom0less build code, move its declaration from the ARM-specific asm/p2m.h to xen/p2m-common.h. Reported-by: Jan Beulich <[email protected]> Signed-off-by: Oleksii Kurochko <[email protected]> Reviewed-by: Michal Orzel <[email protected]> --- Changes in v6: - Nothing changed. Only rebase. --- Changes in v5: - Add Reviewed-by: Michal Orzel <[email protected]> --- Changes in v4: - Reword commit message: a NULL pointer was a calling convention meaning "preemption not permitted", not pure redundancy. - Annotate the explicit can_preempt arguments at the call sites with /* can_preempt */ comments for readability. - Move the function's doc comment to the prototype in xen/p2m-common.h (dropping the duplicate above the Arm and RISC-V definitions) and clarify that -ERESTART is only returned when can_preempt is true. - Add __must_check to the prototype, since the return code is now the only preemption-status indicator. --- Changes in v3: - Nothing changed. Only rebase. --- Changes in v2: - new patch --- --- xen/arch/arm/include/asm/p2m.h | 1 - xen/arch/arm/mmu/p2m.c | 24 ++++++------------------ xen/arch/riscv/include/asm/paging.h | 2 +- xen/arch/riscv/p2m.c | 9 ++------- xen/arch/riscv/paging.c | 7 ++----- xen/common/device-tree/dom0less-build.c | 2 +- xen/include/xen/p2m-common.h | 8 ++++++++ 7 files changed, 20 insertions(+), 33 deletions(-) diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h index 4a4913716bdd..737da60dcf58 100644 --- a/xen/arch/arm/include/asm/p2m.h +++ b/xen/arch/arm/include/asm/p2m.h @@ -238,7 +238,6 @@ void p2m_restore_state(struct vcpu *n); /* Print debugging/statistial info about a domain's p2m */ void p2m_dump_info(struct domain *d); -int p2m_set_allocation(struct domain *d, unsigned long pages, bool *preempted); int p2m_teardown_allocation(struct domain *d); static inline void p2m_write_lock(struct p2m_domain *p2m) diff --git a/xen/arch/arm/mmu/p2m.c b/xen/arch/arm/mmu/p2m.c index 51abf3504fcf..2cf35d8a3709 100644 --- a/xen/arch/arm/mmu/p2m.c +++ b/xen/arch/arm/mmu/p2m.c @@ -65,12 +65,7 @@ int arch_get_paging_mempool_size(struct domain *d, uint64_t *size) return 0; } -/* - * Set the pool of pages to the required number of pages. - * Returns 0 for success, non-zero for failure. - * Call with d->arch.paging.lock held. - */ -int p2m_set_allocation(struct domain *d, unsigned long pages, bool *preempted) +int p2m_set_allocation(struct domain *d, unsigned long pages, bool can_preempt) { struct page_info *pg; @@ -112,11 +107,8 @@ int p2m_set_allocation(struct domain *d, unsigned long pages, bool *preempted) break; /* Check to see if we need to yield and try again */ - if ( preempted && general_preempt_check() ) - { - *preempted = true; + if ( can_preempt && general_preempt_check() ) return -ERESTART; - } } return 0; @@ -125,7 +117,6 @@ int p2m_set_allocation(struct domain *d, unsigned long pages, bool *preempted) int arch_set_paging_mempool_size(struct domain *d, uint64_t size) { unsigned long pages = size >> PAGE_SHIFT; - bool preempted = false; int rc; if ( (size & ~PAGE_MASK) || /* Non page-sized request? */ @@ -133,27 +124,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, /* can_preempt */ true); spin_unlock(&d->arch.paging.lock); - ASSERT(preempted == (rc == -ERESTART)); - return rc; } int p2m_teardown_allocation(struct domain *d) { int ret = 0; - bool preempted = false; spin_lock(&d->arch.paging.lock); if ( d->arch.paging.p2m_total_pages != 0 ) { - ret = p2m_set_allocation(d, 0, &preempted); - if ( preempted ) + ret = p2m_set_allocation(d, 0, /* can_preempt */ true); + if ( ret == -ERESTART ) { spin_unlock(&d->arch.paging.lock); - return -ERESTART; + return ret; } ASSERT(d->arch.paging.p2m_total_pages == 0); } diff --git a/xen/arch/riscv/include/asm/paging.h b/xen/arch/riscv/include/asm/paging.h index e487c89a4ccd..103384723dc5 100644 --- a/xen/arch/riscv/include/asm/paging.h +++ b/xen/arch/riscv/include/asm/paging.h @@ -9,7 +9,7 @@ struct page_info; int paging_domain_init(struct domain *d); int paging_freelist_adjust(struct domain *d, unsigned long pages, - bool *preempted); + bool can_preempt); int paging_ret_to_domheap(struct domain *d, unsigned int nr_pages); int paging_refill_from_domheap(struct domain *d, unsigned int nr_pages); diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c index 703b9f4d2540..566266e3e78f 100644 --- a/xen/arch/riscv/p2m.c +++ b/xen/arch/riscv/p2m.c @@ -428,17 +428,12 @@ int p2m_init(struct domain *d, const struct xen_domctl_createdomain *config) return 0; } -/* - * Set the pool of pages to the required number of pages. - * Returns 0 for success, non-zero for failure. - * Call with d->arch.paging.lock held. - */ -int p2m_set_allocation(struct domain *d, unsigned long pages, bool *preempted) +int p2m_set_allocation(struct domain *d, unsigned long pages, bool can_preempt) { struct p2m_domain *p2m = p2m_get_hostp2m(d); int rc; - if ( (rc = paging_freelist_adjust(d, pages, preempted)) ) + if ( (rc = paging_freelist_adjust(d, pages, can_preempt)) ) return rc; /* diff --git a/xen/arch/riscv/paging.c b/xen/arch/riscv/paging.c index 76a203edbb0c..35f572689a7c 100644 --- a/xen/arch/riscv/paging.c +++ b/xen/arch/riscv/paging.c @@ -47,7 +47,7 @@ static int _paging_add_to_freelist(struct domain *d) } int paging_freelist_adjust(struct domain *d, unsigned long pages, - bool *preempted) + bool can_preempt) { ASSERT(spin_is_locked(&d->arch.paging.lock)); @@ -66,11 +66,8 @@ int paging_freelist_adjust(struct domain *d, unsigned long pages, return rc; /* Check to see if we need to yield and try again */ - if ( preempted && general_preempt_check() ) - { - *preempted = true; + if ( can_preempt && general_preempt_check() ) return -ERESTART; - } } return 0; diff --git a/xen/common/device-tree/dom0less-build.c b/xen/common/device-tree/dom0less-build.c index 9513c1c83766..fcbeb8adbd73 100644 --- a/xen/common/device-tree/dom0less-build.c +++ b/xen/common/device-tree/dom0less-build.c @@ -760,7 +760,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, /* can_preempt */ false); spin_unlock(&d->arch.paging.lock); return rc; diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h index f0bd9a6b9896..0eb061991283 100644 --- a/xen/include/xen/p2m-common.h +++ b/xen/include/xen/p2m-common.h @@ -43,5 +43,13 @@ int __must_check check_get_page_from_gfn(struct domain *d, gfn_t gfn, bool readonly, p2m_type_t *p2mt_p, struct page_info **page_p); +/* + * Set the pool of pages to the required number of pages. + * Returns 0 for success, -ERESTART if preempted (only when can_preempt is + * true), or a negative error code on failure. + * Call with d->arch.paging.lock held. + */ +int __must_check p2m_set_allocation(struct domain *d, unsigned long pages, + bool can_preempt); #endif /* _XEN_P2M_COMMON_H */ -- 2.54.0