[PATCH 0/3] Drivers: hv: Fix CPU assignment for FC devices

[PATCH 0/3] Drivers: hv: Fix CPU assignment for FC devices

[K. Y. Srinivasan]

Currently all channels of a Fibre channel device are being bound to CPU 0.
Correct this by including Fibre Channel devices in the list of
performance critical devices. Also included is a patch to correct the
returned error code for util drivers as well as cleanup of the
vmbus signaling function.

K. Y. Srinivasan (2):
  Drivers: hv: vmbus: Treat Fibre Channel devices as performance
    critical
  Drivers: hv: vmbus: Cleanup vmbus_set_event()

Vitaly Kuznetsov (1):
  Drivers: hv: utils: fix hvt_op_poll() return value on transport
    destroy

 drivers/hv/channel_mgmt.c       |    3 +++
 drivers/hv/connection.c         |    4 ++--
 drivers/hv/hv.c                 |   16 ----------------
 drivers/hv/hv_utils_transport.c |    2 +-
 drivers/hv/hyperv_vmbus.h       |    4 +---
 5 files changed, 7 insertions(+), 22 deletions(-)

-- 
1.7.4.1

[PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport destroy

[K. Y. Srinivasan]

From: Vitaly Kuznetsov <vkuznets@redhat.com>

The return type of hvt_op_poll() is unsigned int and -EBADF is
inappropriate, poll functions return POLL* statuses.

Reported-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_utils_transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
index ee20b50..4f42c0e 100644
--- a/drivers/hv/hv_utils_transport.c
+++ b/drivers/hv/hv_utils_transport.c
@@ -109,7 +109,7 @@ static unsigned int hvt_op_poll(struct file *file, poll_table *wait)
 	poll_wait(file, &hvt->outmsg_q, wait);
 
 	if (hvt->mode == HVUTIL_TRANSPORT_DESTROY)
-		return -EBADF;
+		return POLLERR | POLLHUP;
 
 	if (hvt->outmsg_len > 0)
 		return POLLIN | POLLRDNORM;
-- 
1.7.4.1

[PATCH 2/3] Drivers: hv: vmbus: Treat Fibre Channel devices as performance critical

[K. Y. Srinivasan]

For performance critical devices, we distribute the incoming
channel interrupt load across available CPUs in the guest.
Include Fibre channel devices in the set of devices for which
we would distribute the interrupt load.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index d013171..1c1ad47 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -361,6 +361,7 @@ err_free_chan:
 enum {
 	IDE = 0,
 	SCSI,
+	FC,
 	NIC,
 	ND_NIC,
 	PCIE,
@@ -377,6 +378,8 @@ static const struct hv_vmbus_device_id hp_devs[] = {
 	{ HV_IDE_GUID, },
 	/* Storage - SCSI */
 	{ HV_SCSI_GUID, },
+	/* Storage - FC */
+	{ HV_SYNTHFC_GUID, },
 	/* Network */
 	{ HV_NIC_GUID, },
 	/* NetworkDirect Guest RDMA */
-- 
1.7.4.1

[PATCH 3/3] Drivers: hv: vmbus: Cleanup vmbus_set_event()

[K. Y. Srinivasan]

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/connection.c   |    4 ++--
 drivers/hv/hv.c           |   16 ----------------
 drivers/hv/hyperv_vmbus.h |    4 +---
 3 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 3dc5a9c..4a320e6 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -474,7 +474,7 @@ int vmbus_post_msg(void *buffer, size_t buflen)
 /*
  * vmbus_set_event - Send an event notification to the parent
  */
-int vmbus_set_event(struct vmbus_channel *channel)
+void vmbus_set_event(struct vmbus_channel *channel)
 {
 	u32 child_relid = channel->offermsg.child_relid;
 
@@ -485,5 +485,5 @@ int vmbus_set_event(struct vmbus_channel *channel)
 			(child_relid >> 5));
 	}
 
-	return hv_signal_event(channel->sig_event);
+	hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
 }
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 1db9556..2ed8e16 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -336,22 +336,6 @@ int hv_post_message(union hv_connection_id connection_id,
 	return status & 0xFFFF;
 }
 
-
-/*
- * hv_signal_event -
- * Signal an event on the specified connection using the hypervisor event IPC.
- *
- * This involves a hypercall.
- */
-int hv_signal_event(void *con_id)
-{
-	u64 status;
-
-	status = hv_do_hypercall(HVCALL_SIGNAL_EVENT, con_id, NULL);
-
-	return status & 0xFFFF;
-}
-
 static int hv_ce_set_next_event(unsigned long delta,
 				struct clock_event_device *evt)
 {
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0411b7b..70540d5 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -587,8 +587,6 @@ extern int hv_post_message(union hv_connection_id connection_id,
 			 enum hv_message_type message_type,
 			 void *payload, size_t payload_size);
 
-extern int hv_signal_event(void *con_id);
-
 extern int hv_synic_alloc(void);
 
 extern void hv_synic_free(void);
@@ -736,7 +734,7 @@ void vmbus_disconnect(void);
 
 int vmbus_post_msg(void *buffer, size_t buflen);
 
-int vmbus_set_event(struct vmbus_channel *channel);
+void vmbus_set_event(struct vmbus_channel *channel);
 
 void vmbus_on_event(unsigned long data);
 
-- 
1.7.4.1

Re: [PATCH 3/3] Drivers: hv: vmbus: Cleanup vmbus_set_event()

[Greg KH]

On Tue, Dec 15, 2015 at 04:27:28PM -0800, K. Y. Srinivasan wrote:
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

No changelog text?  Sorry, I can't take this :(

You know better...

RE: [PATCH 3/3] Drivers: hv: vmbus: Cleanup vmbus_set_event()

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, December 21, 2015 1:10 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 3/3] Drivers: hv: vmbus: Cleanup vmbus_set_event()
> 
> On Tue, Dec 15, 2015 at 04:27:28PM -0800, K. Y. Srinivasan wrote:
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> 
> No changelog text?  Sorry, I can't take this :(
> 
> You know better...
Sorry about that; I will resubmit shortly.

Thanks,

K. Y

RE: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport destroy

[Dexuan Cui]

> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On Behalf
> Of K. Y. Srinivasan
> Sent: Wednesday, December 16, 2015 8:27
> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> vkuznets@redhat.com; jasowang@redhat.com
> Subject: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport
> destroy
> 
> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> The return type of hvt_op_poll() is unsigned int and -EBADF is
> inappropriate, poll functions return POLL* statuses.
> 
> Reported-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  drivers/hv/hv_utils_transport.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
> index ee20b50..4f42c0e 100644
> --- a/drivers/hv/hv_utils_transport.c
> +++ b/drivers/hv/hv_utils_transport.c
> @@ -109,7 +109,7 @@ static unsigned int hvt_op_poll(struct file *file,
> poll_table *wait)
>  	poll_wait(file, &hvt->outmsg_q, wait);
> 
>  	if (hvt->mode == HVUTIL_TRANSPORT_DESTROY)
> -		return -EBADF;
> +		return POLLERR | POLLHUP;
> 
>  	if (hvt->outmsg_len > 0)
>  		return POLLIN | POLLRDNORM;
> --

Hi Vitaly,
The daemon only polls on POLLIN.
I'm not sure returning "POLLERR | POLLHUP" here can wake up the daemon or not.

Thanks,
-- Dexuan

Re: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport destroy

[Vitaly Kuznetsov]

Dexuan Cui <decui@microsoft.com> writes:

>> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On Behalf
>> Of K. Y. Srinivasan
>> Sent: Wednesday, December 16, 2015 8:27
>> To: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
>> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
>> vkuznets@redhat.com; jasowang@redhat.com
>> Subject: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport
>> destroy
>> 
>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
>> 
>> The return type of hvt_op_poll() is unsigned int and -EBADF is
>> inappropriate, poll functions return POLL* statuses.
>> 
>> Reported-by: Dexuan Cui <decui@microsoft.com>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
>> ---
>>  drivers/hv/hv_utils_transport.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/drivers/hv/hv_utils_transport.c b/drivers/hv/hv_utils_transport.c
>> index ee20b50..4f42c0e 100644
>> --- a/drivers/hv/hv_utils_transport.c
>> +++ b/drivers/hv/hv_utils_transport.c
>> @@ -109,7 +109,7 @@ static unsigned int hvt_op_poll(struct file *file,
>> poll_table *wait)
>>  	poll_wait(file, &hvt->outmsg_q, wait);
>> 
>>  	if (hvt->mode == HVUTIL_TRANSPORT_DESTROY)
>> -		return -EBADF;
>> +		return POLLERR | POLLHUP;
>> 
>>  	if (hvt->outmsg_len > 0)
>>  		return POLLIN | POLLRDNORM;
>> --
>
> Hi Vitaly,
> The daemon only polls on POLLIN.
> I'm not sure returning "POLLERR | POLLHUP" here can wake up the daemon or not.
>

I tested this patch with hv_kvp_daemon which does poll() and it
works: we wake up all pollers from hvutil_transport_destroy(). Here
we just need to return proper value. Actually, the return value doesn't
really matter -- we do read() after poll() and get -EBADF there. But
let's be consistent.

-- 
  Vitaly

RE: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport destroy

[Dexuan Cui]

> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> ...
> >> @@ -109,7 +109,7 @@ static unsigned int hvt_op_poll(struct file *file,
> >> poll_table *wait)
> >>  	poll_wait(file, &hvt->outmsg_q, wait);
> >>
> >>  	if (hvt->mode == HVUTIL_TRANSPORT_DESTROY)
> >> -		return -EBADF;
> >> +		return POLLERR | POLLHUP;
> >>
> >>  	if (hvt->outmsg_len > 0)
> >>  		return POLLIN | POLLRDNORM;
> >> --
> >
> > Hi Vitaly,
> > The daemon only polls on POLLIN.
> > I'm not sure returning "POLLERR | POLLHUP" here can wake up the daemon or
> not.
> >
> 
> I tested this patch with hv_kvp_daemon which does poll() and it
> works: we wake up all pollers from hvutil_transport_destroy(). Here
> we just need to return proper value. Actually, the return value doesn't
> really matter -- we do read() after poll() and get -EBADF there. But
> let's be consistent.
> 
>   Vitaly

Thanks Vitaly for the explanation!

I checked do_pollfd() and can confirm it's OK.

Thanks,
-- Dexuan