On Tue, Oct 08, 2019 at 10:41:42PM +0000, Dexuan Cui wrote: > > From: Vitaly Kuznetsov <vkuzn...@redhat.com> > > Sent: Tuesday, October 8, 2019 6:00 AM > > ... > > > Looking at the uses of VERSION_INVAL, I find one remaining occurrence > > > of this macro in vmbus_bus_resume(), which does: > > > > > > if (vmbus_proto_version == VERSION_INVAL || > > > vmbus_proto_version == 0) { > > > ... > > > } > > > > > > TBH I'm looking at vmbus_bus_resume() and vmbus_bus_suspend() for the > > > first time and I'm not sure about how to change such check yet... any > > > suggestions? > > > > Hm, I don't think vmbus_proto_version can ever become == VERSION_INVAL > > if we rewrite the code the way you suggest, right? So you'll reduce this > > to 'if (!vmbus_proto_version)' meaning we haven't negotiated any version > > (yet). > > Yeah, Vitaly is correct. The check may be a little paranoid as I believe > "vmbus_proto_version" must be a negotiated value in vmbus_bus_resume() > and vmbus_bus_suspend(). I added the check just in case. > > > > Mmh, I see that vmbus_bus_resume() and vmbus_bus_suspend() can access > > > vmbus_connection.conn_state: can such accesses race with a concurrent > > > vmbus_connect()? > > > > Let's summon Dexuan who's the author! :-) > > There should not be an issue: > > vmbus_connect() is called in the early subsys_initcall(hv_acpi_init). > > vmbus_bus_suspend() is called late in the PM code after the kernel boots up, > e.g. > in the hibernation function hibernation_snapshot() -> dpm_suspend(). > > vmbus_bus_resume() is also called later in > late_initcall_sync(software_resume). > > In the hibernatin process, vmbus_bus_suspend()/resume() can also be called a > few times, and vmbus_bus_resume() calls vmbus_negotiate_version(). As I > checked, there is no issue, either.
Thank you both for these remarks. So, I'll proceed with the removal of VERSION_INVAL in v2 of this series. Thanks, Andrea