From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Dexuan Cui <decui@microsoft.com>
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, "K. Y. Srinivasan" <kys@microsoft.com>
Subject: Re: [PATCH V2 10/12] Drivers: hv: vmbus: channge vmbus_connection.channel_lock to mutex
Date: Thu, 12 Nov 2015 11:41:22 +0100 [thread overview]
Message-ID: <87vb97ijot.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <1447295184-9051-10-git-send-email-kys@microsoft.com> (K. Y. Srinivasan's message of "Wed, 11 Nov 2015 18:26:22 -0800")
"K. Y. Srinivasan" <kys@microsoft.com> writes:
> From: Dexuan Cui <decui@microsoft.com>
>
> spinlock is unnecessary here.
> mutex is enough.
Hm, mutex is usually required when we need to sleep and a spinlock is
enough otherwise :-)
Or are you trying to say we don't need to disable interrupts here? In
that can maybe we can just switch to spin_lock()/spin_unlock()?
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/hv/channel_mgmt.c | 12 ++++++------
> drivers/hv/connection.c | 7 +++----
> drivers/hv/hyperv_vmbus.h | 2 +-
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 9c9da3a..d013171 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -206,9 +206,9 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
> }
>
> if (channel->primary_channel == NULL) {
> - spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
> + mutex_lock(&vmbus_connection.channel_mutex);
> list_del(&channel->listentry);
> - spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
> + mutex_unlock(&vmbus_connection.channel_mutex);
>
> primary_channel = channel;
> } else {
> @@ -253,7 +253,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
> unsigned long flags;
>
> /* Make sure this is a new offer */
> - spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
> + mutex_lock(&vmbus_connection.channel_mutex);
>
> list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
> if (!uuid_le_cmp(channel->offermsg.offer.if_type,
> @@ -269,7 +269,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
> list_add_tail(&newchannel->listentry,
> &vmbus_connection.chn_list);
>
> - spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
> + mutex_unlock(&vmbus_connection.channel_mutex);
>
> if (!fnew) {
> /*
> @@ -341,9 +341,9 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
> err_deq_chan:
> vmbus_release_relid(newchannel->offermsg.child_relid);
>
> - spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
> + mutex_lock(&vmbus_connection.channel_mutex);
> list_del(&newchannel->listentry);
> - spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
> + mutex_unlock(&vmbus_connection.channel_mutex);
>
> if (newchannel->target_cpu != get_cpu()) {
> put_cpu();
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 4fc2e88..521f48e 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -146,7 +146,7 @@ int vmbus_connect(void)
> spin_lock_init(&vmbus_connection.channelmsg_lock);
>
> INIT_LIST_HEAD(&vmbus_connection.chn_list);
> - spin_lock_init(&vmbus_connection.channel_lock);
> + mutex_init(&vmbus_connection.channel_mutex);
>
> /*
> * Setup the vmbus event connection for channel interrupt
> @@ -282,11 +282,10 @@ struct vmbus_channel *relid2channel(u32 relid)
> {
> struct vmbus_channel *channel;
> struct vmbus_channel *found_channel = NULL;
> - unsigned long flags;
> struct list_head *cur, *tmp;
> struct vmbus_channel *cur_sc;
>
> - spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
> + mutex_lock(&vmbus_connection.channel_mutex);
> list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
> if (channel->offermsg.child_relid == relid) {
> found_channel = channel;
> @@ -305,7 +304,7 @@ struct vmbus_channel *relid2channel(u32 relid)
> }
> }
> }
> - spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
> + mutex_unlock(&vmbus_connection.channel_mutex);
>
> return found_channel;
> }
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 64950d8..0af6dce 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -688,7 +688,7 @@ struct vmbus_connection {
>
> /* List of channels */
> struct list_head chn_list;
> - spinlock_t channel_lock;
> + struct mutex channel_mutex;
>
> struct workqueue_struct *work_queue;
> };
--
Vitaly
next prev parent reply other threads:[~2015-11-12 10:41 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-12 2:25 [PATCH V2 00/12] Drivers: hv: vmbus: Miscellaneous fixes and cleanup K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 01/12] Drivers: hv: vss: run only on supported host versions K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 02/12] Drivers: hv: vmbus: Use uuid_le type consistently K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 03/12] Drivers: hv: vmbus: Use uuid_le_cmp() for comparing GUIDs K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 04/12] Drivers: hv: vmbus: Get rid of the unused macro K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 05/12] Drivers: hv: vmbus: Get rid of the unused irq variable K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 06/12] Drivers: hv: vmbus: serialize process_chn_event() and vmbus_close_internal() K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 07/12] Drivers: hv: vmbus: do sanity check of channel state in vmbus_close_internal() K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 08/12] Drivers: hv: vmbus: fix rescind-offer handling for device without a driver K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 09/12] Drivers: hv: vmbus: release relid on error in vmbus_process_offer() K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH V2 10/12] Drivers: hv: vmbus: channge vmbus_connection.channel_lock to mutex K. Y. Srinivasan
2015-11-12 10:41 ` Vitaly Kuznetsov [this message]
2015-11-12 13:10 ` Dexuan Cui
2015-11-12 2:26 ` [PATCH V2 11/12] drivers:hv: Allow for MMIO claims that span ACPI _CRS records K. Y. Srinivasan
2015-11-12 2:26 ` [PATCH 12/12] tools/hv: Use include/uapi with __EXPORTED_HEADERS__ K. Y. Srinivasan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87vb97ijot.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=apw@canonical.com \
--cc=decui@microsoft.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=jasowang@redhat.com \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=olaf@aepfle.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.