Messages in this thread | | | From | Michael Kelley <> | Subject | RE: [PATCH v2 1/1] x86/hyperv: Initialize clockevents earlier in CPU onlining | Date | Mon, 18 Nov 2019 01:23:15 +0000 |
| |
From: Dexuan Cui <decui@microsoft.com> Sent: Thursday, November 14, 2019 10:49 PM > > From: Michael Kelley <mikelley@microsoft.com> > > Sent: Tuesday, November 12, 2019 5:12 PM > > ... > > @@ -190,14 +278,20 @@ void hv_stimer_free(void) > > void hv_stimer_global_cleanup(void) > > { > > int cpu; > > ... > > + /* > > + * hv_stime_legacy_cleanup() will stop the stimer if Direct > > + * Mode is not enabled, and fallback to the LAPIC timer. > > + */ > > + for_each_present_cpu(cpu) { > > + hv_stimer_legacy_cleanup(cpu); > > } > > + > > + /* > > + * If Direct Mode is enabled, the cpuhp teardown callback > > + * (hv_stimer_cleanup) will be run on all CPUs to stop the > > + * stimers. > > + */ > > hv_stimer_free(); > > } > > In the case of direct_mode_enabled == true: When hv_vmbus unloads, > vmbus_exit() -> hv_stimer_global_cleanup() -> hv_stimer_free() -> > cpuhp_remove_state() disables the direct mode timers. > > This does not look symmetric since hv_stimer_alloc() is called before > hv_vmbus loads, but I suppose this is not a real issue because hv_vmbus > should never unload in practice. :-)
Thanks for the review. Looking at this a bit more, ideally hv_stimer_global_cleanup() should go away entirely. vmbus_exit() should let the call to cpuhp_remove_state() clean up the stimer on each CPU via hv_synic_cleanup(). But vmbus_exit() shuts down the VMbus first, which is really the wrong order.
As you say, vmbus_exit() never gets called in practice, so it's not a current issue. But I'll look at fixing this in a future patch so that it makes logical sense.
Michael
> > Tested-by: Dexuan Cui <decui@microsoft.com> > Reviewed-by: Dexuan Cui <decui@microsoft.com>
| |