> > @@ -244,21 +234,18 @@ int vmbus_connect(void) > > * version. > > */ > > > > - version = VERSION_CURRENT; > > + for (i = 0; ; i++) { > > + version = vmbus_versions[i]; > > + if (version == VERSION_INVAL) > > + goto cleanup; > > If you use e.g. ARRAY_SIZE() you can get rid of VERSION_INVAL - and make > this code look more natural. Thank you for pointing this out, Vitaly. IIUC, you're suggesting that I do: for (i = 0; i < ARRAY_SIZE(vmbus_versions); i++) { version = vmbus_versions[i]; ret = vmbus_negotiate_version(msginfo, version); if (ret == -ETIMEDOUT) goto cleanup; if (vmbus_connection.conn_state == CONNECTED) break; } if (vmbus_connection.conn_state != CONNECTED) break; and that I remove VERSION_INVAL from vmbus_versions, yes? 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? 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()? Thanks, Andrea > > > > - do { > > ret = vmbus_negotiate_version(msginfo, version); > > if (ret == -ETIMEDOUT) > > goto cleanup; > > > > if (vmbus_connection.conn_state == CONNECTED) > > break; > > - > > - version = vmbus_get_next_version(version); > > - } while (version != VERSION_INVAL); > > - > > - if (version == VERSION_INVAL) > > - goto cleanup; > > + } > > > > vmbus_proto_version = version; > > pr_info("Vmbus version:%d.%d\n",