> From: David Miller > Sent: Thursday, July 30, 2015 6:28 > > From: Dexuan Cui <de...@microsoft.com> > > Date: Tue, 28 Jul 2015 05:35:30 -0700 > > > > In the SMP guest case, when the per-channel callback hvsock_events() is > > running on virtual CPU A, if the guest tries to close the connection on > > virtual CPU B: we invoke vmbus_close() -> vmbus_close_internal(), > > then we can have trouble: on B, vmbus_close_internal() will send IPI > > reset_channel_cb() to A, trying to set channel->onchannel_callbackto NULL; > > on A, if the IPI handler happens between > > "if (channel->onchannel_callback != NULL)" and invoking > > channel->onchannel_callback, we'll invoke a function pointer of NULL. > > > > This is why the patch is necessary. > > > Sorry, I do not accept that you must use conditional locking and/or > IRQ disabling. > > Boil it down to what is necessary for the least common denominator, > and use that unconditionally.
Hi David, Thanks for the comment! I agree with you it's not clean to use conditional IRQ disabling. Here I didn't use unconditionally IRQ disabling because the Hyper-V netvsc and storvsc driver's vmbus event callbacks (i.e. netvsc_channel_cb() and storvsc_on_channel_callback()) may take relatively long time (e.g., netvsc can operate at a speed of 10Gb) and I think it's bad to disable IRQ for long time when the callbacks are running in a tasklet context, e.g., the Hyper-V timer can be affected: see vmbus_isr() -> hv_process_timer_expiration(). To resolve the race condition between vmbus_close_internal() and process_chn_event() in SMP case, now I propose a new method: we can serialize the 2 paths by adding tasklet_disable(hv_context.event_dpc[channel->target_cpu]) and tasklet_enable(...) in vmbus_close_internal(). In this way, we need the least change and we can drop this patch. Please let me know your opinion. Thanks, -- 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/
