-----Original Message-----
From: Michael Kelley <redacted>
Sent: Monday, December 30, 2019 8:35 PM
To: Haiyang Zhang <haiyangz@microsoft.com>; sashal@kernel.org; linux-
hyperv@vger.kernel.org; netdev@vger.kernel.org
Cc: Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan
[off-list ref]; Stephen Hemminger [off-list ref];
olaf@aepfle.de; vkuznets [off-list ref]; davem@davemloft.net;
linux-kernel@vger.kernel.org
Subject: RE: [PATCH V2,net-next, 1/3] Drivers: hv: vmbus: Add a dev_num
variable based on channel offer sequence

From: Haiyang Zhang <haiyangz@microsoft.com> Sent: Monday, December 30,
2019 12:14 PM

quoted

This number is set to the first available number, starting from zero,
when a vmbus device's primary channel is offered.

Let's use "VMBus" as the capitalization in text.

Sure I will.

quoted

It will be used for stable naming when Async probing is used.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
Changes
V2:
	Use nest loops in hv_set_devnum, instead of goto.

 drivers/hv/channel_mgmt.c | 38

++++++++++++++++++++++++++++++++++++--

quoted

 include/linux/hyperv.h    |  6 ++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 8eb1675..00fa2db 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c

@@ -315,6 +315,8 @@ static struct vmbus_channel *alloc_channel(void)
 	if (!channel)
 		return NULL;

+	channel->dev_num = HV_DEV_NUM_INVALID;
+
 	spin_lock_init(&channel->lock);
 	init_completion(&channel->rescind_event);

@@ -541,6 +543,36 @@ static void vmbus_add_channel_work(struct

work_struct *work)

quoted

 }

 /*
+ * Get the first available device number of its type, then
+ * record it in the channel structure.
+ */
+static void hv_set_devnum(struct vmbus_channel *newchannel)
+{
+	struct vmbus_channel *channel;
+	int i = -1;
+	bool found;
+
+	BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
+
+	do {
+		i++;
+		found = false;
+
+		list_for_each_entry(channel, &vmbus_connection.chn_list,
+				    listentry) {
+			if (i == channel->dev_num &&
+			    guid_equal(&channel->offermsg.offer.if_type,
+				       &newchannel->offermsg.offer.if_type)) {
+				found = true;
+				break;
+			}
+		}
+	} while (found);
+
+	newchannel->dev_num = i;
+}
+

It took me a little while to figure out what the above algorithm is doing.
Perhaps it would help to rename the "found" variable to "in_use", and add
this comment before the start of the "do" loop:

Iterate through each possible device number starting at zero.  If the device
number is already in use for a device of this type, try the next device number
until finding one that is not in use.   This approach selects the smallest
device number that is not in use, and so reuses any numbers that are freed
by devices that have been removed.

I will rename the variable, and add the code comments.
Thanks,

- Haiyang