Messages in this thread | | | From | Haiyang Zhang <> | Subject | RE: [PATCH net-next, 1/3] Drivers: hv: vmbus: Add a dev_num variable based on channel offer sequence | Date | Sun, 29 Dec 2019 01:06:35 +0000 |
| |
> -----Original Message----- > From: Stephen Hemminger <stephen@networkplumber.org> > Sent: Saturday, December 28, 2019 7:20 PM > To: Haiyang Zhang <haiyangz@microsoft.com> > Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; > KY Srinivasan <kys@microsoft.com>; Stephen Hemminger > <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets > <vkuznets@redhat.com>; davem@davemloft.net; linux-kernel@vger.kernel.org > Subject: Re: [PATCH net-next, 1/3] Drivers: hv: vmbus: Add a dev_num variable > based on channel offer sequence > > On Sat, 28 Dec 2019 15:46:31 -0800 > Haiyang Zhang <haiyangz@microsoft.com> wrote: > > > + > > +next: > > + 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; > > + } > > + } > > + > > + if (found) { > > + i++; > > + goto next; > > + } > > + > > Overall, keeping track of dev_num is a good solution. > > I prefer not having a loop coded with goto's. Why not > a nested loop. Sure, I can use a nested loop.
> Also, there already is a search of the channel > list in vmbus_process_offer() so why is another lookup needed? vmbus_process_offer() looks for the if_instance and if_type matches to determine if this is a subchannel vs primary channel. The loop terminates at different condition from hv_set_devnum(). So I didn't re-use the existing loop.
And this kind of search happens only during channel offering, and doesn't impact performance much.
Thanks, - Haiyang
| |