|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH v3] xen/domain: make shutdown state explicit
From: Mykola Kvach <mykola_kvach@xxxxxxxx> The shutdown flow currently uses is_shutting_down and is_shut_down to represent the domain shutdown lifecycle. The two flags are not mutually exclusive: after shutdown completion is_shutting_down remains set until domain_resume() clears both flags. Replace the two booleans with an enum domain_shutdown_state. Keep domain_shutting_down() as the direct replacement for the old is_shutting_down flag: it is true once shutdown has been initiated and remains true after completion, until domain_resume(). Add domain_shutdown_completed() for users that need the final completed state. This makes the state transition explicit while avoiding a semantic split between "in progress" and "completed" at call sites where the old code only cared that shutdown had started and had not yet been reset by domain_resume(). Suggested-by: Jan Beulich <jbeulich@xxxxxxxx> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx> --- Changes in v3: - Keep domain_shutting_down() as a direct replacement for is_shutting_down: true from shutdown start until domain_resume(), including after shutdown completion. - Drop domain_in_shutdown_state(). - Make old is_shutting_down conversions mechanical again; use domain_shutdown_completed() only for old is_shut_down users. - Keep domain_resume() returning void and leave callers unchanged. - Drop resume-state validation and the related invalid-resume diagnostic, including the cached shutdown_state/shutdown_code values. - Reword the commit message to avoid describing domain_shutting_down() as only the transient shutdown phase. Changes in v2: - Drop the shutdown reason restrictions from domain_resume(), so the validation remains compatible with the existing soft-reset flow. - Use clearer helper naming for the three shutdown-state predicates. - Document in the commit message how old is_shutting_down/is_shut_down users map to the new shutdown-state helpers. - Fix label indentation noted during review. Link to discussion: https://patchew.org/Xen/cover.1756392094.git.mykola._5Fkvach@xxxxxxxx/bb53d9911b00879c7b25f5258d0e3e48005671f9.1756392094.git.mykola._5Fkvach@xxxxxxxx/#a64cff9f-df5f-467b-a944-74e803c64ab9@xxxxxxxx --- xen/arch/x86/hvm/viridian/time.c | 2 +- xen/arch/x86/mm.c | 2 +- xen/arch/x86/mm/hap/hap.c | 2 +- xen/arch/x86/mm/shadow/common.c | 5 +++-- xen/arch/x86/mm/shadow/multi.c | 12 +++++++----- xen/common/domain.c | 20 ++++++++++---------- xen/common/domctl.c | 2 +- xen/common/sched/core.c | 2 +- xen/drivers/passthrough/iommu.c | 8 ++++---- xen/drivers/passthrough/pci.c | 2 +- xen/include/xen/sched.h | 23 +++++++++++++++++++---- 11 files changed, 49 insertions(+), 31 deletions(-) diff --git a/xen/arch/x86/hvm/viridian/time.c b/xen/arch/x86/hvm/viridian/time.c index 9311858d63..cf57af263a 100644 --- a/xen/arch/x86/hvm/viridian/time.c +++ b/xen/arch/x86/hvm/viridian/time.c @@ -102,7 +102,7 @@ static void time_ref_count_thaw(const struct domain *d) struct viridian_domain *vd = d->arch.hvm.viridian; struct viridian_time_ref_count *trc = &vd->time_ref_count; - if ( d->is_shutting_down || + if ( domain_shutting_down(d) || test_and_set_bit(_TRC_running, &trc->flags) ) return; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index a158379e77..c8ce166d6e 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -1219,7 +1219,7 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain *l1e_owner) */ #if _PAGE_GNTTAB if ( (l1e_get_flags(l1e) & _PAGE_GNTTAB) && - !l1e_owner->is_shutting_down && !l1e_owner->is_dying ) + !domain_shutting_down(l1e_owner) && !l1e_owner->is_dying ) { gprintk(XENLOG_WARNING, "Attempt to implicitly unmap %pd's grant PTE %" PRIpte "\n", diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c index 5ccb80bda5..0ede4181a0 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -414,7 +414,7 @@ static mfn_t hap_make_monitor_table(struct vcpu *v) oom: if ( !d->is_dying && - (!d->is_shutting_down || d->shutdown_code != SHUTDOWN_crash) ) + (!domain_shutting_down(d) || d->shutdown_code != SHUTDOWN_crash) ) { printk(XENLOG_G_ERR "%pd: out of memory building monitor pagetable\n", d); diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c index ed698fa90b..c956b4840b 100644 --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -164,7 +164,7 @@ void shadow_promote(struct domain *d, mfn_t gmfn, unsigned int type) /* We should never try to promote a gmfn that has writeable mappings */ ASSERT((page->u.inuse.type_info & PGT_type_mask) != PGT_writable_page || (page->u.inuse.type_info & PGT_count_mask) == 0 - || d->is_shutting_down); + || domain_shutting_down(d)); /* Is the page already shadowed? */ if ( !test_and_set_bit(_PGC_shadowed_pt, &page->count_info) ) @@ -442,7 +442,8 @@ bool shadow_prealloc(struct domain *d, unsigned int type, unsigned int count) count += paging_logdirty_levels(); ret = _shadow_prealloc(d, count); - if ( !ret && (!d->is_shutting_down || d->shutdown_code != SHUTDOWN_crash) ) + if ( !ret && (!domain_shutting_down(d) || + d->shutdown_code != SHUTDOWN_crash) ) /* * Failing to allocate memory required for shadow usage can only result in * a domain crash, do it here rather that relying on every caller to do it. diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c index 2990cee869..632a83a78e 100644 --- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -2370,7 +2370,8 @@ static int cf_check sh_page_fault( * already used for some special purpose (ioreq pages, or granted pages). * If that happens we'll have killed the guest already but it's still not * safe to propagate entries out of the guest PT so get out now. */ - if ( unlikely(d->is_shutting_down && d->shutdown_code == SHUTDOWN_crash) ) + if ( unlikely(domain_shutting_down(d) && + d->shutdown_code == SHUTDOWN_crash) ) { SHADOW_PRINTK("guest is shutting down\n"); goto propagate; @@ -2480,7 +2481,7 @@ static int cf_check sh_page_fault( #if GUEST_PAGING_LEVELS == 3 sh_update_cr3(v, false); #else - ASSERT(d->is_shutting_down); + ASSERT(domain_shutting_down(d)); sh_trace_va(TRC_SHADOW_DOMF_DYING, va); #endif paging_unlock(d); @@ -2494,7 +2495,8 @@ static int cf_check sh_page_fault( && ft == ft_demand_write ) sh_unsync(v, gmfn); - if ( unlikely(d->is_shutting_down && d->shutdown_code == SHUTDOWN_crash) ) + if ( unlikely(domain_shutting_down(d) && + d->shutdown_code == SHUTDOWN_crash) ) { /* We might end up with a crashed domain here if * sh_remove_shadows() in a previous sh_resync() call has @@ -3265,7 +3267,7 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool noflush) sh_make_shadow); if ( unlikely(pagetable_is_null(v->arch.paging.shadow.shadow_table[0])) ) { - ASSERT(d->is_dying || d->is_shutting_down); + ASSERT(d->is_dying || domain_shutting_down(d)); return old_entry; } if ( !paging_mode_external(d) && !is_pv_32bit_domain(d) ) @@ -3332,7 +3334,7 @@ static pagetable_t cf_check sh_update_cr3(struct vcpu *v, bool noflush) ASSERT(pagetable_is_null(old_entry)); if ( unlikely(pagetable_is_null(v->arch.paging.shadow.shadow_table[0])) ) { - ASSERT(d->is_dying || d->is_shutting_down); + ASSERT(d->is_dying || domain_shutting_down(d)); return old_entry; } #else diff --git a/xen/common/domain.c b/xen/common/domain.c index bb9e210c28..473b7f8e3f 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -191,7 +191,7 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, const struct domain *d) { info->state = XEN_DOMCTL_GETDOMSTATE_STATE_EXIST; - if ( d->is_shut_down ) + if ( domain_shutdown_completed(d) ) info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_SHUTDOWN; if ( d->is_dying == DOMDYING_dying ) info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; @@ -282,14 +282,14 @@ static void __domain_finalise_shutdown(struct domain *d) BUG_ON(!spin_is_locked(&d->shutdown_lock)); - if ( d->is_shut_down ) + if ( domain_shutdown_completed(d) ) return; for_each_vcpu ( d, v ) if ( !v->paused_for_shutdown ) return; - d->is_shut_down = 1; + d->shutdown_state = DOMSHUTDOWN_complete; domain_changed_state(d); if ( (d->shutdown_code == SHUTDOWN_suspend) && d->suspend_evtchn ) evtchn_send(d, d->suspend_evtchn); @@ -303,7 +303,7 @@ static void vcpu_check_shutdown(struct vcpu *v) spin_lock(&d->shutdown_lock); - if ( d->is_shutting_down ) + if ( domain_shutting_down(d) ) { if ( !v->paused_for_shutdown ) vcpu_pause_nosync(v); @@ -1356,7 +1356,7 @@ int domain_kill(struct domain *d) void __domain_crash(struct domain *d) { - if ( d->is_shutting_down ) + if ( domain_shutting_down(d) ) { /* Print nothing: the domain is already shutting down. */ } @@ -1394,13 +1394,13 @@ int domain_shutdown(struct domain *d, u8 reason) if ( is_hardware_domain(d) ) hwdom_shutdown(reason); - if ( d->is_shutting_down ) + if ( domain_shutting_down(d) ) { spin_unlock(&d->shutdown_lock); return 0; } - d->is_shutting_down = 1; + d->shutdown_state = DOMSHUTDOWN_in_progress; smp_mb(); /* set shutdown status /then/ check for per-cpu deferrals */ @@ -1435,7 +1435,7 @@ void domain_resume(struct domain *d) spin_lock(&d->shutdown_lock); - d->is_shutting_down = d->is_shut_down = 0; + d->shutdown_state = DOMSHUTDOWN_none; arch_domain_resume(d); @@ -1460,7 +1460,7 @@ int vcpu_start_shutdown_deferral(struct vcpu *v) v->defer_shutdown = 1; smp_mb(); /* set deferral status /then/ check for shutdown */ - if ( unlikely(v->domain->is_shutting_down) ) + if ( unlikely(domain_shutting_down(v->domain)) ) vcpu_check_shutdown(v); return v->defer_shutdown; @@ -1470,7 +1470,7 @@ void vcpu_end_shutdown_deferral(struct vcpu *v) { v->defer_shutdown = 0; smp_mb(); /* clear deferral status /then/ check for shutdown */ - if ( unlikely(v->domain->is_shutting_down) ) + if ( unlikely(domain_shutting_down(v->domain)) ) vcpu_check_shutdown(v); } diff --git a/xen/common/domctl.c b/xen/common/domctl.c index b969f5ada6..5594fc72d8 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -86,7 +86,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info) info->flags = (info->nr_online_vcpus ? flags : 0) | ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying : 0) | - (d->is_shut_down ? XEN_DOMINF_shutdown : 0) | + (domain_shutdown_completed(d) ? XEN_DOMINF_shutdown : 0) | (d->controller_pause_count > 0 ? XEN_DOMINF_paused : 0) | (d->debugger_attached ? XEN_DOMINF_debugged : 0) | (is_xenstore_domain(d) ? XEN_DOMINF_xs_domain : 0) | diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index 8e2b75bc35..9ac00e4c0d 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -1539,7 +1539,7 @@ static void cf_check domain_watchdog_timeout(void *data) BUILD_BUG_ON(alignof(*d) < PAGE_SIZE); - if ( d->is_shutting_down || d->is_dying ) + if ( domain_shutting_down(d) || d->is_dying ) return; printk("Watchdog timer %u fired for %pd\n", id, d); diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index c9425d6971..4b22ecfe65 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -355,7 +355,7 @@ long iommu_map(struct domain *d, dfn_t dfn0, mfn_t mfn0, if ( likely(!rc) ) continue; - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutting_down(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU mapping dfn %"PRI_dfn" to mfn %"PRI_mfn" failed: %d\n", d->domain_id, dfn_x(dfn), mfn_x(mfn), rc); @@ -427,7 +427,7 @@ long iommu_unmap(struct domain *d, dfn_t dfn0, unsigned long page_count, if ( likely(!err) ) continue; - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutting_down(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU unmapping dfn %"PRI_dfn" failed: %d\n", d->domain_id, dfn_x(dfn), err); @@ -492,7 +492,7 @@ int iommu_iotlb_flush(struct domain *d, dfn_t dfn, unsigned long page_count, flush_flags); if ( unlikely(rc) ) { - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutting_down(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU IOTLB flush failed: %d, dfn %"PRI_dfn", page count %lu flags %x\n", d->domain_id, rc, dfn_x(dfn), page_count, flush_flags); @@ -517,7 +517,7 @@ int iommu_iotlb_flush_all(struct domain *d, unsigned int flush_flags) flush_flags | IOMMU_FLUSHF_all); if ( unlikely(rc) ) { - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutting_down(d) && printk_ratelimit() ) printk(XENLOG_ERR "d%d: IOMMU IOTLB flush all failed: %d\n", d->domain_id, rc); diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c index 464bb0fee4..5dbee61900 100644 --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -1746,7 +1746,7 @@ void iommu_dev_iotlb_flush_timeout(struct domain *d, struct pci_dev *pdev) pdev->broken = true; - if ( !d->is_shutting_down && printk_ratelimit() ) + if ( !domain_shutting_down(d) && printk_ratelimit() ) printk(XENLOG_ERR "dom%d: ATS device %pp flush failed\n", d->domain_id, &pdev->sbdf); if ( !is_hardware_domain(d) ) diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 00db1da12f..22a0deb31f 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -222,7 +222,7 @@ struct vcpu bool force_context_switch; /* Require shutdown to be deferred for some asynchronous operation? */ bool defer_shutdown; - /* VCPU is paused following shutdown request (d->is_shutting_down)? */ + /* VCPU is paused following a domain shutdown request? */ bool paused_for_shutdown; /* VCPU need affinity restored */ uint8_t affinity_broken; @@ -382,6 +382,12 @@ struct domain_console { char buf[256]; }; +enum domain_shutdown_state { + DOMSHUTDOWN_none, + DOMSHUTDOWN_in_progress, + DOMSHUTDOWN_complete, +}; + struct domain { domid_t domain_id; @@ -552,10 +558,9 @@ struct domain struct rangeset *iomem_caps; struct rangeset *irq_caps; - /* Guest has shut down (inc. reason code)? */ + /* Guest shutdown state and associated reason code. */ spinlock_t shutdown_lock; - bool is_shutting_down; /* in process of shutting down? */ - bool is_shut_down; /* fully shut down? */ + enum domain_shutdown_state shutdown_state; #define SHUTDOWN_CODE_INVALID ~0u unsigned int shutdown_code; @@ -674,6 +679,16 @@ struct domain unsigned int pending_scrub_index; } __aligned(PAGE_SIZE); +static inline bool domain_shutting_down(const struct domain *d) +{ + return d->shutdown_state != DOMSHUTDOWN_none; +} + +static inline bool domain_shutdown_completed(const struct domain *d) +{ + return d->shutdown_state == DOMSHUTDOWN_complete; +} + static inline struct page_list_head *page_to_list( struct domain *d, const struct page_info *pg) { -- 2.43.0
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |