Andrea Parri <parri.and...@gmail.com> writes:
> The technique used to get the next VMBus version seems increasisly
> clumsy as the number of VMBus versions increases. Performance is
> not a concern since this is only done once during system boot; it's
> just that we'll end up with more lines of code than is really needed.
>
> As an alternative, introduce a table with the version numbers listed
> in order (from the most recent to the oldest). vmbus_connect() loops
> through the versions listed in the table until it gets an accepted
> connection or gets to the end of the table (invalid version).
>
> Suggested-by: Michael Kelley <mikel...@microsoft.com>
> Signed-off-by: Andrea Parri <parri.and...@gmail.com>
> ---
> drivers/hv/connection.c | 51 +++++++++++++++--------------------------
> include/linux/hyperv.h | 2 --
> 2 files changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 6e4c015783ffc..90a32c9d79403 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -40,29 +40,19 @@ EXPORT_SYMBOL_GPL(vmbus_connection);
> __u32 vmbus_proto_version;
> EXPORT_SYMBOL_GPL(vmbus_proto_version);
>
> -static __u32 vmbus_get_next_version(__u32 current_version)
> -{
> - switch (current_version) {
> - case (VERSION_WIN7):
> - return VERSION_WS2008;
> -
> - case (VERSION_WIN8):
> - return VERSION_WIN7;
> -
> - case (VERSION_WIN8_1):
> - return VERSION_WIN8;
> -
> - case (VERSION_WIN10):
> - return VERSION_WIN8_1;
> -
> - case (VERSION_WIN10_V5):
> - return VERSION_WIN10;
> -
> - case (VERSION_WS2008):
> - default:
> - return VERSION_INVAL;
> - }
> -}
> +/*
> + * Table of VMBus versions listed from newest to oldest; the table
> + * must terminate with VERSION_INVAL.
> + */
> +__u32 vmbus_versions[] = {
> + VERSION_WIN10_V5,
> + VERSION_WIN10,
> + VERSION_WIN8_1,
> + VERSION_WIN8,
> + VERSION_WIN7,
> + VERSION_WS2008,
> + VERSION_INVAL
> +};
>
> int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32
> version)
> {
> @@ -169,8 +159,8 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo
> *msginfo, u32 version)
> */
> int vmbus_connect(void)
> {
> - int ret = 0;
> struct vmbus_channel_msginfo *msginfo = NULL;
> + int i, ret = 0;
> __u32 version;
>
> /* Initialize the vmbus connection */
> @@ -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.
>
> - 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",
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index b4a017093b697..7073f1eb3618c 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -194,8 +194,6 @@ static inline u32 hv_get_avail_to_write_percent(
>
> #define VERSION_INVAL -1
>
> -#define VERSION_CURRENT VERSION_WIN10_V5
> -
> /* Make maximum size of pipe payload of 16K */
> #define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
--
Vitaly
