RE: [PATCH V2 10/12] Drivers: hv: vmbus: channge vmbus_connection.channel_lock to mutex
From: Dexuan Cui
Date: Thu Nov 12 2015 - 08:11:25 EST
> 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
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()
(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
Here I use mutex (rather than spinlock) because the critical section
can sleep, due to vmbus_device_unregister().
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/