Skip to contentSkip to conversation

Drivers: hv: Fix CPU assignment for FC devices

PATCH In review K. Y. Srinivasan 4 participants 9 messages Dec 15, 2015 – Dec 21, 2015 Hyper-V/Azure core and drivers

lore
Share & tools
lore

K. Y. Srinivasan

author
#0 lore raw

cover Drivers: hv: Fix CPU assignment for FC devices

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(-)
Signature · 1 line
1.7.4.1
author
#1 lore raw

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

From: Vitaly Kuznetsov <[email protected]>

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

Reported-by Dexuan Cui <[email protected]>
Signed-off-by Vitaly Kuznetsov <[email protected]>
Signed-off-by K. Y. Srinivasan <[email protected]>
---
 drivers/hv/hv_utils_transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/hv/hv_utils_transport.c +1 -1
index ee20b50..4f42c0e 100644
@@ -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;
Signature · 1 line
1.7.4.1
author
#2 lore raw

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

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 <[email protected]>
---
 drivers/hv/channel_mgmt.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/hv/channel_mgmt.c +3 -0
index d013171..1c1ad47 100644
@@ -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 */
Signature · 1 line
1.7.4.1
author
#3 lore raw

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

In reply to K. Y. Srinivasan
Signed-off-by K. Y. Srinivasan <[email protected]>
---
 drivers/hv/connection.c   |    4 ++--
 drivers/hv/hv.c           |   16 ----------------
 drivers/hv/hyperv_vmbus.h |    4 +---
 3 files changed, 3 insertions(+), 21 deletions(-)
drivers/hv/connection.c +2 -2
index 3dc5a9c..4a320e6 100644
@@ -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);
 }
drivers/hv/hv.c +0 -16
index 1db9556..2ed8e16 100644
@@ -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)
 {
drivers/hv/hyperv_vmbus.h +1 -3
index 0411b7b..70540d5 100644
@@ -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);
Signature · 1 line
1.7.4.1
author
#5 lore raw

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

Quoted 11 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Monday, December 21, 2015 1:10 PM
To: KY Srinivasan <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 3/3] Drivers: hv: vmbus: Cleanup vmbus_set_event()

Quoted 7 lines
Quoted 2 lines · K. Y. Srinivasan
Signed-off-by K. Y. Srinivasan <[email protected]>

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

You know better...

Sorry about that; I will resubmit shortly.

Thanks,

K. Y

maintainer
#6 lore raw

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

In reply to K. Y. Srinivasan
Quoted 34 lines

From: devel [mailto:[email protected]] On Behalf Of K. Y. Srinivasan
Sent: Wednesday, December 16, 2015 8:27
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport destroy

From: Vitaly Kuznetsov <[email protected]>

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

Reported-by Dexuan Cui <[email protected]>
Signed-off-by Vitaly Kuznetsov <[email protected]>
Signed-off-by K. Y. Srinivasan <[email protected]>
---
 drivers/hv/hv_utils_transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/hv/hv_utils_transport.c +1 -1
index ee20b50..4f42c0e 100644
@@ -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)
Quoted 2 lines

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

#7 lore raw

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

Quoted 33 lines · Dexuan Cui
Quoted 33 lines

From: devel [mailto:[email protected]] On Behalf Of K. Y. Srinivasan
Sent: Wednesday, December 16, 2015 8:27
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 1/3] Drivers: hv: utils: fix hvt_op_poll() return value on transport destroy

From: Vitaly Kuznetsov <[email protected]>

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

Reported-by Dexuan Cui <[email protected]>
Signed-off-by Vitaly Kuznetsov <[email protected]>
Signed-off-by K. Y. Srinivasan <[email protected]>
---
 drivers/hv/hv_utils_transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/hv/hv_utils_transport.c +1 -1
index ee20b50..4f42c0e 100644
@@ -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;
Quoted 8 lines
Quoted 3 lines
drivers/hv/hv_utils_transport.c +0 -0
 	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.

Signature · 1 line
  Vitaly
maintainer
#8 lore raw

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

Quoted 20 lines

From: Vitaly Kuznetsov [mailto:[email protected]]
...

Quoted 8 lines
Quoted 8 lines
drivers/hv/hv_utils_transport.c +1 -1
@@ -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;
Quoted 7 lines
Quoted 3 lines
drivers/hv/hv_utils_transport.c +0 -0
 	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.

Quoted 1 line
Quoted 7 lines

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