Re: [PATCH net 2/4] hv_netvsc: reset vf_inject on VF removal

Yuval Mintz [off-list ref] writes:

quoted

+static void netvsc_inject_enable(struct net_device_context
+*net_device_ctx) {
+	net_device_ctx->vf_inject = true;
+}
+
+static void netvsc_inject_disable(struct net_device_context
+*net_device_ctx) {
+	net_device_ctx->vf_inject = false;
+
+	/* Wait for currently active users to drain out. */
+	while (atomic_read(&net_device_ctx->vf_use_cnt) != 0)
+		udelay(50);
+}

That was already the behavior before, but are you certain you
want to unconditionally block without any possible timeout?

Yes, this is OK. After PATCH4 of this series there is only one place
which takes the vf_use_cnt (netvsc_recv_callback()) and it is an
interrupt handler, there are no sleepable operations there.

-- 
  Vitaly

On Thu, 11 Aug 2016 14:09:53 +0200
Vitaly Kuznetsov [off-list ref] wrote:

Yuval Mintz [off-list ref] writes:

quoted

quoted

+static void netvsc_inject_enable(struct net_device_context
+*net_device_ctx) {
+	net_device_ctx->vf_inject = true;
+}
+
+static void netvsc_inject_disable(struct net_device_context
+*net_device_ctx) {
+	net_device_ctx->vf_inject = false;
+
+	/* Wait for currently active users to drain out. */
+	while (atomic_read(&net_device_ctx->vf_use_cnt) != 0)
+		udelay(50);
+}  

That was already the behavior before, but are you certain you
want to unconditionally block without any possible timeout?  

Yes, this is OK. After PATCH4 of this series there is only one place
which takes the vf_use_cnt (netvsc_recv_callback()) and it is an
interrupt handler, there are no sleepable operations there.

Since network devices are protected by RCU, it looks like the refcount
is not necessary.  I think vf_inject flag and vf_use_cnt could just be replaced
by doing RCU on vf_netdev.

The callback is invoked from tasklet (softirq) context.