RE: [PATCH 1/6] Drivers: hv: vmbus: Implement multi-channel support

2013-05-16 Thread KY Srinivasan


 -Original Message-
 From: Greg KH [mailto:gre...@linuxfoundation.org]
 Sent: Thursday, May 16, 2013 12:02 AM
 To: KY Srinivasan
 Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
 oher...@suse.com; jbottom...@parallels.com; h...@infradead.org; linux-
 s...@vger.kernel.org; a...@canonical.com; jasow...@redhat.com
 Subject: Re: [PATCH 1/6] Drivers: hv: vmbus: Implement multi-channel support
 
 On Wed, May 15, 2013 at 03:02:29PM -0700, K. Y. Srinivasan wrote:
  +/*
  + * Retrieve the (sub) channel on which to send an outgoing request.
  + * When a primary channel has multiple sub-channels, we choose a
  + * channel whose VCPU binding is closest to the VCPU on which
  + * this call is being made.
  + */
  +struct vmbus_channel *get_outgoing_channel(struct vmbus_channel
 *primary)
 
 That's a _very_ vague global symbol name you are adding to the kernel.
 Same goes for the other functions you are adding here, please fix that,
 and make them have the vmbus_ prefix, like everything else in this
 patch.


Will do.

K. Y


--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/6] Drivers: hv: vmbus: Implement multi-channel support

2013-05-15 Thread K. Y. Srinivasan
Starting with Win8, the host supports multiple sub-channels for a given
device. As in the past, the initial channel offer specifies the device and
is associated with both the type and the instance GUIDs. For performance
critical devices, the host may support multiple sub-channels. The sub-channels
share the same type and instance GUID as the primary channel. The number of
sub-channels offerrred to the guest depends on the number of virtual CPUs
assigned to the guest. The guest can request the creation of these sub-channels
and once created and opened, the guest can distribute the traffic across all
the channels (the primary and the sub-channels). A request sent on a sub-channel
will have the response delivered on the same sub-channel.

At channel (sub-channel) creation we bind the channel interrupt to a CPU and
with this sub-channel support we will be able to spread the interrupt load
of a given device across all available CPUs.

Signed-off-by: K. Y. Srinivasan k...@microsoft.com
Reviewed-by: Haiyang Zhang haiya...@microsoft.com
---
 drivers/hv/channel.c  |   41 ++--
 drivers/hv/channel_mgmt.c |  117 +++--
 include/linux/hyperv.h|   62 +++-
 3 files changed, 211 insertions(+), 9 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 0b122f8..c219e9f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -216,6 +216,9 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 
send_ringbuffer_size,
list_del(open_info-msglistentry);
spin_unlock_irqrestore(vmbus_connection.channelmsg_lock, flags);
 
+   if (err == 0)
+   newchannel-state = CHANNEL_OPENED_STATE;
+
kfree(open_info);
return err;
 
@@ -500,15 +503,14 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, 
u32 gpadl_handle)
 }
 EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl);
 
-/*
- * vmbus_close - Close the specified channel
- */
-void vmbus_close(struct vmbus_channel *channel)
+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;
@@ -538,6 +540,37 @@ void vmbus_close(struct vmbus_channel *channel)
 
 
 }
+
+/*
+ * vmbus_close - Close the specified channel
+ */
+void vmbus_close(struct vmbus_channel *channel)
+{
+   struct list_head *cur, *tmp;
+   struct vmbus_channel *cur_channel;
+
+   if (channel-primary_channel != NULL) {
+   /*
+* We will only close sub-channels when
+* the primary is closed.
+*/
+   return;
+   }
+   /*
+* Close all the sub-channels first and then close the
+* primary channel.
+*/
+   list_for_each_safe(cur, tmp, channel-sc_list) {
+   cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
+   if (cur_channel-state != CHANNEL_OPENED_STATE)
+   continue;
+   vmbus_close_internal(cur_channel);
+   }
+   /*
+* Now close the primary.
+*/
+   vmbus_close_internal(channel);
+}
 EXPORT_SYMBOL_GPL(vmbus_close);
 
 /**
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 21ef689..834dff3 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -115,6 +115,9 @@ static struct vmbus_channel *alloc_channel(void)
return NULL;
 
spin_lock_init(channel-inbound_lock);
+   spin_lock_init(channel-sc_lock);
+
+   INIT_LIST_HEAD(channel-sc_list);
 
channel-controlwq = create_workqueue(hv_vmbus_ctl);
if (!channel-controlwq) {
@@ -166,6 +169,7 @@ static void vmbus_process_rescind_offer(struct work_struct 
*work)
 struct vmbus_channel,
 work);
unsigned long flags;
+   struct vmbus_channel *primary_channel;
struct vmbus_channel_relid_released msg;
 
vmbus_device_unregister(channel-device_obj);
@@ -174,9 +178,16 @@ 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));
 
-   spin_lock_irqsave(vmbus_connection.channel_lock, flags);
-   list_del(channel-listentry);
-   spin_unlock_irqrestore(vmbus_connection.channel_lock, flags);
+   if (channel-primary_channel == NULL) {
+   spin_lock_irqsave(vmbus_connection.channel_lock, flags);
+   list_del(channel-listentry);
+   spin_unlock_irqrestore(vmbus_connection.channel_lock, flags);
+   } else 

Re: [PATCH 1/6] Drivers: hv: vmbus: Implement multi-channel support

2013-05-15 Thread Greg KH
On Wed, May 15, 2013 at 03:02:29PM -0700, K. Y. Srinivasan wrote:
 +/*
 + * Retrieve the (sub) channel on which to send an outgoing request.
 + * When a primary channel has multiple sub-channels, we choose a
 + * channel whose VCPU binding is closest to the VCPU on which
 + * this call is being made.
 + */
 +struct vmbus_channel *get_outgoing_channel(struct vmbus_channel *primary)

That's a _very_ vague global symbol name you are adding to the kernel.
Same goes for the other functions you are adding here, please fix that,
and make them have the vmbus_ prefix, like everything else in this
patch.

So, sorry, no ack here.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html