> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] > Sent: Thursday, November 12, 2015 18:41 > To: Dexuan Cui <decui@xxxxxxxxxxxxx> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; apw@xxxxxxxxxxxxx; > jasowang@xxxxxxxxxx; KY Srinivasan <kys@xxxxxxxxxxxxx> > Subject: Re: [PATCH V2 10/12] Drivers: hv: vmbus: channge > vmbus_connection.channel_lock to mutex > > "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx> writes: > > > From: Dexuan Cui <decui@xxxxxxxxxxxxx> > > > > spinlock is unnecessary here. > > mutex is enough. > > Hm, mutex is usually required when we need to sleep and a spinlock is > enough otherwise :-) Sorry, I should have written a better changelog. :-) > Or are you trying to say we don't need to disable interrupts here? In Yes. Here we try to protect vmbus_connection.chn_list and the related channel pointer (see relid2channel()) from being updated in parallel. The parallel paths, e.g., vmbus_process_offer() and vmbus_onoffer_rescind(), are in process context and no irq context is involved, so we don't need disable interrupts at all. > that can maybe we can just switch to spin_lock()/spin_unlock()? Switching to mutex actually makes preparation for a later patch (which will be posted to LKML once this pachset is accepted): Drivers: hv: vmbus: add an API vmbus_hvsock_device_unregister() https://github.com/dcui/linux/commit/185afe8394a9bdae2be11ee1ea2a38d05e373025 (on branch decui/vmsock_1020) For a vmsock socket connection, the host and the guest can be closing the connection at the same time. When the host tries to close the connection, a rescind offer is received in the VM. When the guest tries to close the connection, a new vmbus API vmbus_hvsock_device_unregister(channel) is invoked, so vmbus_hvsock_device_unregister() -> vmbus_device_unregister() is invoked and this can be running in parallel with vmbus_onoffer_rescind() -> vmbus_device_unregister(). The issue of "vmbus_onoffer_rescind () -> relid2channel()" is: it returns a channel pointer without the spinlock held, so actually there is no real protection for the channel pointer. So IMO we need to serialize vmbus_onoffer_rescind() and vmbus_hvsock_device_unregister(). Here I use mutex (rather than spinlock) because the critical section can sleep, due to vmbus_device_unregister(). Thanks, -- Dexuan _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
