2020-06-17 16:50:32

by Andrea Parri

Subject: [PATCH 6/8] Drivers: hv: vmbus: Remove unnecessary channel->lock critical sections (sc_list updaters)

None of the readers/updaters of sc_list rely on channel->lock for
synchronization.

Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
---
drivers/hv/channel_mgmt.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 36dd8b6c544a4..92f8bb2077a94 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -400,8 +400,6 @@ static void vmbus_release_relid(u32 relid)

void hv_process_channel_removal(struct vmbus_channel *channel)
{
- unsigned long flags;
-
lockdep_assert_held(&vmbus_connection.channel_mutex);
BUG_ON(!channel->rescind);

@@ -422,14 +420,10 @@ void hv_process_channel_removal(struct vmbus_channel *channel)
if (channel->offermsg.child_relid != INVALID_RELID)
vmbus_channel_unmap_relid(channel);

- if (channel->primary_channel == NULL) {
+ if (channel->primary_channel == NULL)
list_del(&channel->listentry);
- } else {
- struct vmbus_channel *primary_channel = channel->primary_channel;
- spin_lock_irqsave(&primary_channel->lock, flags);
+ else
list_del(&channel->sc_list);
- spin_unlock_irqrestore(&primary_channel->lock, flags);
- }

/*
* If this is a "perf" channel, updates the hv_numa_map[] masks so that
@@ -470,7 +464,6 @@ static void vmbus_add_channel_work(struct work_struct *work)
struct vmbus_channel *newchannel =
container_of(work, struct vmbus_channel, add_channel_work);
struct vmbus_channel *primary_channel = newchannel->primary_channel;
- unsigned long flags;
int ret;

/*
@@ -531,13 +524,10 @@ static void vmbus_add_channel_work(struct work_struct *work)
*/
newchannel->probe_done = true;

- if (primary_channel == NULL) {
+ if (primary_channel == NULL)
list_del(&newchannel->listentry);
- } else {
- spin_lock_irqsave(&primary_channel->lock, flags);
+ else
list_del(&newchannel->sc_list);
- spin_unlock_irqrestore(&primary_channel->lock, flags);
- }

/* vmbus_process_offer() has mapped the channel. */
vmbus_channel_unmap_relid(newchannel);
@@ -557,7 +547,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
{
struct vmbus_channel *channel;
struct workqueue_struct *wq;
- unsigned long flags;
bool fnew = true;

/*
@@ -609,10 +598,10 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
}
}

- if (fnew)
+ if (fnew) {
list_add_tail(&newchannel->listentry,
&vmbus_connection.chn_list);
- else {
+ } else {
/*
* Check to see if this is a valid sub-channel.
*/
@@ -630,9 +619,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
* Process the sub-channel.
*/
newchannel->primary_channel = channel;
- spin_lock_irqsave(&channel->lock, flags);
list_add_tail(&newchannel->sc_list, &channel->sc_list);
- spin_unlock_irqrestore(&channel->lock, flags);
}

vmbus_channel_map_relid(newchannel);
--
2.25.1

2020-06-18 19:11:13

by Michael Kelley (LINUX)

Subject: RE: [PATCH 6/8] Drivers: hv: vmbus: Remove unnecessary channel->lock critical sections (sc_list updaters)

From: Andrea Parri (Microsoft) <[email protected]> Sent: Wednesday, June 17, 2020 9:47 AM
>
> None of the readers/updaters of sc_list rely on channel->lock for
> synchronization.
>
> Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
> ---
> drivers/hv/channel_mgmt.c | 25 ++++++-------------------
> 1 file changed, 6 insertions(+), 19 deletions(-)

Reviewed-by: Michael Kelley <[email protected]>