* [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path @ 2014-04-09 1:45 K. Y. Srinivasan 2014-04-09 1:45 ` [PATCH 1/2] Drivers: hv: Eliminate the channel spinlock in the " K. Y. Srinivasan 2014-04-23 17:54 ` [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path KY Srinivasan 0 siblings, 2 replies; 4+ messages in thread From: K. Y. Srinivasan @ 2014-04-09 1:45 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan Currently we map the channel handle to the channel under the protection of a spin lock. Additionally, we dispatch the channel callback function under the protection of the channel inbound lock (another spin lock). In some recent nework performance runs, the time spent acquiring and releasing these locks were identified as potential bottlenecks. This patch-set gets rid of these locks by leveraging the interrupt bindings that we support starting with win8. K. Y. Srinivasan (2): Drivers: hv: Eliminate the channel spinlock in the callback path Drivers: hv: vmbus: Implement per-CPU mapping of relid to channel drivers/hv/channel.c | 16 ++++++++++--- drivers/hv/channel_mgmt.c | 52 ++++++++++++++++++++++++++++++++++++++++---- drivers/hv/connection.c | 35 +++++++++++++++++++++++------- drivers/hv/hv.c | 2 + drivers/hv/hyperv_vmbus.h | 5 ++++ include/linux/hyperv.h | 7 ++++++ 6 files changed, 100 insertions(+), 17 deletions(-) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] Drivers: hv: Eliminate the channel spinlock in the callback path 2014-04-09 1:45 [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path K. Y. Srinivasan @ 2014-04-09 1:45 ` K. Y. Srinivasan 2014-04-09 1:45 ` [PATCH 2/2] Drivers: hv: vmbus: Implement per-CPU mapping of relid to channel K. Y. Srinivasan 2014-04-23 17:54 ` [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path KY Srinivasan 1 sibling, 1 reply; 4+ messages in thread From: K. Y. Srinivasan @ 2014-04-09 1:45 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan By ensuring that we set the callback handler to NULL in the channel close path on the same CPU that the channel is bound to, we can eliminate this lock acquisition and release in a performance critical path. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com> --- drivers/hv/channel.c | 16 ++++++++++++---- drivers/hv/channel_mgmt.c | 11 +++++++---- drivers/hv/connection.c | 11 ++++------- include/linux/hyperv.h | 2 ++ 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 602ca86..740edec 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -471,18 +471,26 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) } EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl); +static void reset_channel_cb(void *arg) +{ + struct vmbus_channel *channel = arg; + + channel->onchannel_callback = NULL; +} + static void vmbus_close_internal(struct vmbus_channel *channel) { struct vmbus_channel_close_channel *msg; int ret; - unsigned long flags; channel->state = CHANNEL_OPEN_STATE; channel->sc_creation_callback = NULL; /* Stop callback and cancel the timer asap */ - spin_lock_irqsave(&channel->inbound_lock, flags); - channel->onchannel_callback = NULL; - spin_unlock_irqrestore(&channel->inbound_lock, flags); + if (channel->target_cpu != smp_processor_id()) + smp_call_function_single(channel->target_cpu, reset_channel_cb, + channel, true); + else + reset_channel_cb(channel); /* Send a closing message */ diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index fa92046..6f7fdd9 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -365,7 +365,7 @@ static u32 next_vp; * performance critical channels (IDE, SCSI and Network) will be uniformly * distributed across all available CPUs. */ -static u32 get_vp_index(uuid_le *type_guid) +static void init_vp_index(struct vmbus_channel *channel, uuid_le *type_guid) { u32 cur_cpu; int i; @@ -387,10 +387,13 @@ static u32 get_vp_index(uuid_le *type_guid) * Also if the channel is not a performance critical * channel, bind it to cpu 0. */ - return 0; + channel->target_cpu = 0; + channel->target_vp = 0; + return; } cur_cpu = (++next_vp % max_cpus); - return hv_context.vp_index[cur_cpu]; + channel->target_cpu = cur_cpu; + channel->target_vp = hv_context.vp_index[cur_cpu]; } /* @@ -438,7 +441,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr) offer->connection_id; } - newchannel->target_vp = get_vp_index(&offer->offer.if_type); + init_vp_index(newchannel, &offer->offer.if_type); memcpy(&newchannel->offermsg, offer, sizeof(struct vmbus_channel_offer_channel)); diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index f2d7bf9..d484bad 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -274,7 +274,6 @@ struct vmbus_channel *relid2channel(u32 relid) static void process_chn_event(u32 relid) { struct vmbus_channel *channel; - unsigned long flags; void *arg; bool read_state; u32 bytes_to_read; @@ -293,13 +292,12 @@ static void process_chn_event(u32 relid) /* * A channel once created is persistent even when there * is no driver handling the device. An unloading driver - * sets the onchannel_callback to NULL under the - * protection of the channel inbound_lock. Thus, checking - * and invoking the driver specific callback takes care of - * orderly unloading of the driver. + * sets the onchannel_callback to NULL on the same CPU + * as where this interrupt is handled (in an interrupt context). + * Thus, checking and invoking the driver specific callback takes + * care of orderly unloading of the driver. */ - spin_lock_irqsave(&channel->inbound_lock, flags); if (channel->onchannel_callback != NULL) { arg = channel->channel_callback_context; read_state = channel->batched_reading; @@ -324,7 +322,6 @@ static void process_chn_event(u32 relid) pr_err("no channel callback for relid - %u\n", relid); } - spin_unlock_irqrestore(&channel->inbound_lock, flags); } /* diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index ab7359f..8b41570 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -694,6 +694,8 @@ struct vmbus_channel { * preserve the earlier behavior. */ u32 target_vp; + /* The corresponding CPUID in the guest */ + u32 target_cpu; /* * Support for sub-channels. For high performance devices, * it will be useful to have multiple sub-channels to support -- 1.7.4.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] Drivers: hv: vmbus: Implement per-CPU mapping of relid to channel 2014-04-09 1:45 ` [PATCH 1/2] Drivers: hv: Eliminate the channel spinlock in the " K. Y. Srinivasan @ 2014-04-09 1:45 ` K. Y. Srinivasan 0 siblings, 0 replies; 4+ messages in thread From: K. Y. Srinivasan @ 2014-04-09 1:45 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan Currently the mapping of the relID to channel is done under the protection of a single spin lock. Starting with ws2012, each channel is bound to a specific VCPU in the guest. Use this binding to eliminate the spin lock by setting up per-cpu state for mapping relId to the channel. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com> --- drivers/hv/channel_mgmt.c | 41 ++++++++++++++++++++++++++++++++++++++++- drivers/hv/connection.c | 24 +++++++++++++++++++++++- drivers/hv/hv.c | 2 ++ drivers/hv/hyperv_vmbus.h | 5 +++++ include/linux/hyperv.h | 5 +++++ 5 files changed, 75 insertions(+), 2 deletions(-) diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 6f7fdd9..6c8b032c 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -149,6 +149,7 @@ static struct vmbus_channel *alloc_channel(void) spin_lock_init(&channel->sc_lock); INIT_LIST_HEAD(&channel->sc_list); + INIT_LIST_HEAD(&channel->percpu_list); channel->controlwq = create_workqueue("hv_vmbus_ctl"); if (!channel->controlwq) { @@ -188,7 +189,20 @@ static void free_channel(struct vmbus_channel *channel) queue_work(vmbus_connection.work_queue, &channel->work); } +static void percpu_channel_enq(void *arg) +{ + struct vmbus_channel *channel = arg; + int cpu = smp_processor_id(); + + list_add_tail(&channel->percpu_list, &hv_context.percpu_list[cpu]); +} +static void percpu_channel_deq(void *arg) +{ + struct vmbus_channel *channel = arg; + + list_del(&channel->percpu_list); +} /* * vmbus_process_rescind_offer - @@ -210,6 +224,12 @@ static void vmbus_process_rescind_offer(struct work_struct *work) msg.header.msgtype = CHANNELMSG_RELID_RELEASED; vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released)); + if (channel->target_cpu != smp_processor_id()) + smp_call_function_single(channel->target_cpu, + percpu_channel_deq, channel, true); + else + percpu_channel_deq(channel); + if (channel->primary_channel == NULL) { spin_lock_irqsave(&vmbus_connection.channel_lock, flags); list_del(&channel->listentry); @@ -245,6 +265,7 @@ static void vmbus_process_offer(struct work_struct *work) work); struct vmbus_channel *channel; bool fnew = true; + bool enq = false; int ret; unsigned long flags; @@ -264,12 +285,22 @@ static void vmbus_process_offer(struct work_struct *work) } } - if (fnew) + if (fnew) { list_add_tail(&newchannel->listentry, &vmbus_connection.chn_list); + enq = true; + } spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags); + if (enq) { + if (newchannel->target_cpu != smp_processor_id()) + smp_call_function_single(newchannel->target_cpu, + percpu_channel_enq, + newchannel, true); + else + percpu_channel_enq(newchannel); + } if (!fnew) { /* * Check to see if this is a sub-channel. @@ -282,6 +313,14 @@ static void vmbus_process_offer(struct work_struct *work) spin_lock_irqsave(&channel->sc_lock, flags); list_add_tail(&newchannel->sc_list, &channel->sc_list); spin_unlock_irqrestore(&channel->sc_lock, flags); + + if (newchannel->target_cpu != smp_processor_id()) + smp_call_function_single(newchannel->target_cpu, + percpu_channel_enq, + newchannel, true); + else + percpu_channel_enq(newchannel); + newchannel->state = CHANNEL_OPEN_STATE; if (channel->sc_creation_callback != NULL) channel->sc_creation_callback(newchannel); diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index d484bad..3589236 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -231,6 +231,28 @@ cleanup: return ret; } +/* + * Map the given relid to the corresponding channel based on the + * per-cpu list of channels that have been affinitized to this CPU. + * This will be used in the channel callback path as we can do this + * mapping in a lock-free fashion. + */ +static struct vmbus_channel *pcpu_relid2channel(u32 relid) +{ + struct vmbus_channel *channel; + struct vmbus_channel *found_channel = NULL; + int cpu = smp_processor_id(); + struct list_head *pcpu_head = &hv_context.percpu_list[cpu]; + + list_for_each_entry(channel, pcpu_head, percpu_list) { + if (channel->offermsg.child_relid == relid) { + found_channel = channel; + break; + } + } + + return found_channel; +} /* * relid2channel - Get the channel object given its @@ -282,7 +304,7 @@ static void process_chn_event(u32 relid) * Find the channel based on this relid and invokes the * channel callback to process the event */ - channel = relid2channel(relid); + channel = pcpu_relid2channel(relid); if (!channel) { pr_err("channel not found for relid - %u\n", relid); diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index bcb4950..edfc848 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -383,6 +383,8 @@ void hv_synic_init(void *arg) */ rdmsrl(HV_X64_MSR_VP_INDEX, vp_index); hv_context.vp_index[cpu] = (u32)vp_index; + + INIT_LIST_HEAD(&hv_context.percpu_list[cpu]); return; } diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 860134d..18d1a84 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -510,6 +510,11 @@ struct hv_context { * basis. */ struct tasklet_struct *event_dpc[NR_CPUS]; + /* + * To optimize the mapping of relid to channel, maintain + * per-cpu list of the channels based on their CPU affinity. + */ + struct list_head percpu_list[NR_CPUS]; }; extern struct hv_context hv_context; diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 8b41570..ebd0441 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -732,6 +732,11 @@ struct vmbus_channel { * Support per-channel state for use by vmbus drivers. */ void *per_channel_state; + /* + * To support per-cpu lookup mapping of relid to channel, + * link up channels based on their CPU affinity. + */ + struct list_head percpu_list; }; static inline void set_channel_read_state(struct vmbus_channel *c, bool state) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path 2014-04-09 1:45 [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path K. Y. Srinivasan 2014-04-09 1:45 ` [PATCH 1/2] Drivers: hv: Eliminate the channel spinlock in the " K. Y. Srinivasan @ 2014-04-23 17:54 ` KY Srinivasan 1 sibling, 0 replies; 4+ messages in thread From: KY Srinivasan @ 2014-04-23 17:54 UTC (permalink / raw) To: KY Srinivasan, gregkh, linux-kernel, devel, olaf, apw, jasowang > -----Original Message----- > From: K. Y. Srinivasan [mailto:kys@microsoft.com] > Sent: Tuesday, April 8, 2014 6:45 PM > To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; > jasowang@redhat.com > Cc: KY Srinivasan > Subject: [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path > > Currently we map the channel handle to the channel under the protection of > a spin lock. Additionally, we dispatch the channel callback function under the > protection of the channel inbound lock (another spin lock). In some recent > nework performance runs, the time spent acquiring and releasing these locks > were identified as potential bottlenecks. This patch-set gets rid of these locks > by leveraging the interrupt bindings that we support starting with win8. > > K. Y. Srinivasan (2): > Drivers: hv: Eliminate the channel spinlock in the callback path > Drivers: hv: vmbus: Implement per-CPU mapping of relid to channel > > drivers/hv/channel.c | 16 ++++++++++--- > drivers/hv/channel_mgmt.c | 52 > ++++++++++++++++++++++++++++++++++++++++---- > drivers/hv/connection.c | 35 +++++++++++++++++++++++------- > drivers/hv/hv.c | 2 + > drivers/hv/hyperv_vmbus.h | 5 ++++ > include/linux/hyperv.h | 7 ++++++ > 6 files changed, 100 insertions(+), 17 deletions(-) > > -- > 1.7.4.1 Greg, Should I resend this patch set. K. Y ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-04-23 17:54 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-09 1:45 [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path K. Y. Srinivasan 2014-04-09 1:45 ` [PATCH 1/2] Drivers: hv: Eliminate the channel spinlock in the " K. Y. Srinivasan 2014-04-09 1:45 ` [PATCH 2/2] Drivers: hv: vmbus: Implement per-CPU mapping of relid to channel K. Y. Srinivasan 2014-04-23 17:54 ` [PATCH 0/2] Eliminate spin locks in the vmbus channel callback path KY Srinivasan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome