> From: K. Y. Srinivasan > Sent: Friday, July 17, 2015 3:17 > Subject: [PATCH net-next 1/1] hv_netvsc: Wait for sub-channels to be processed > during probe > diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h > ... > @@ -1116,6 +1127,9 @@ int rndis_filter_device_add(struct hv_device *dev, > num_possible_rss_qs = cpumask_weight(node_cpu_mask); > net_device->num_chn = min(num_possible_rss_qs, num_rss_qs); > > + num_rss_qs = net_device->num_chn - 1; > + net_device->num_sc_offered = num_rss_qs; > + > if (net_device->num_chn == 1) > goto out; > > @@ -1157,11 +1171,22 @@ int rndis_filter_device_add(struct hv_device *dev, > > ret = rndis_filter_set_rss_param(rndis_device, net_device->num_chn); > > + /* > + * Wait for the host to send us the sub-channel offers. > + */ > + spin_lock_irqsave(&net_device->sc_lock, flags); > + sc_delta = net_device->num_chn - 1 - num_rss_qs; > + net_device->num_sc_offered -= sc_delta;
Hi KY, IMO here the "-= " should be "+="? I think sc_delta is usually <= 0, meaning the host may allocate less subchannels than we expect. With "-=", net_device->num_sc_offered can become bigger -- this doesn't seem correct. Why not use "net_device->num_sc_offered = net_device->num_chn - 1;" directly? At this point, net_device->num_chn has been the number of the actual channels. > + spin_unlock_irqrestore(&net_device->sc_lock, flags); > + > + if (net_device->num_sc_offered != 0) > + wait_for_completion(&net_device->channel_init_wait); BTW, I also tested the patch and I can confirm the panic I saw disappeared with the patch. -- Dexuan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
