2018-03-23 00:18:16

by Long Li

[permalink] [raw]
Subject: [PATCH 1/3] Vmbus: Add function to report available ring buffer to write in total ring size percentage

From: Long Li <[email protected]>

Netvsc has a similar function to calculate how much ring buffer in
percentage is available to write. This function is useful for storvsc and
other vmbus devices.

Define a similar function in vmbus to be used by storvsc.

Signed-off-by: Long Li <[email protected]>
---
drivers/hv/ring_buffer.c | 2 ++
include/linux/hyperv.h | 12 ++++++++++++
2 files changed, 14 insertions(+)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 8699bb969e7e..3c836c099a8f 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -227,6 +227,8 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
ring_info->ring_buffer->feature_bits.value = 1;

ring_info->ring_size = page_cnt << PAGE_SHIFT;
+ ring_info->ring_size_div10_reciprocal =
+ reciprocal_value(ring_info->ring_size / 10);
ring_info->ring_datasize = ring_info->ring_size -
sizeof(struct hv_ring_buffer);

diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 2048f3c3b68a..eb7204851089 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -36,6 +36,7 @@
#include <linux/device.h>
#include <linux/mod_devicetable.h>
#include <linux/interrupt.h>
+#include <linux/reciprocal_div.h>

#define MAX_PAGE_BUFFER_COUNT 32
#define MAX_MULTIPAGE_BUFFER_COUNT 32 /* 128K */
@@ -121,6 +122,7 @@ struct hv_ring_buffer {
struct hv_ring_buffer_info {
struct hv_ring_buffer *ring_buffer;
u32 ring_size; /* Include the shared header */
+ struct reciprocal_value ring_size_div10_reciprocal;
spinlock_t ring_lock;

u32 ring_datasize; /* < ring_size */
@@ -155,6 +157,16 @@ static inline u32 hv_get_bytes_to_write(const struct hv_ring_buffer_info *rbi)
return write;
}

+static inline u32 hv_get_avail_to_write_percent(
+ const struct hv_ring_buffer_info *rbi)
+{
+ u32 avail_write = hv_get_bytes_to_write(rbi);
+
+ return reciprocal_divide(
+ (avail_write << 3) + (avail_write << 1),
+ rbi->ring_size_div10_reciprocal);
+}
+
/*
* VMBUS version is 32 bit entity broken up into
* two 16 bit quantities: major_number. minor_number.
--
2.14.1



2018-03-23 00:18:27

by Long Li

[permalink] [raw]
Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage

From: Long Li <[email protected]>

In Vmbus, we have defined a function to calculate available ring buffer
percentage to write.

Use that function and remove duplicate netvsc code.

Signed-off-by: Long Li <[email protected]>
---
drivers/net/hyperv/netvsc.c | 17 +++--------------
drivers/net/hyperv/netvsc_drv.c | 3 ---
2 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 0265d703eb03..8af0069e4d8c 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -31,7 +31,6 @@
#include <linux/vmalloc.h>
#include <linux/rtnetlink.h>
#include <linux/prefetch.h>
-#include <linux/reciprocal_div.h>

#include <asm/sync_bitops.h>

@@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device *device)
#define RING_AVAIL_PERCENT_HIWATER 20
#define RING_AVAIL_PERCENT_LOWATER 10

-/*
- * Get the percentage of available bytes to write in the ring.
- * The return value is in range from 0 to 100.
- */
-static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info *ring_info)
-{
- u32 avail_write = hv_get_bytes_to_write(ring_info);
-
- return reciprocal_divide(avail_write * 100, netvsc_ring_reciprocal);
-}
-
static inline void netvsc_free_send_slot(struct netvsc_device *net_device,
u32 index)
{
@@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct netvsc_device *net_device,
wake_up(&net_device->wait_drain);

if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
- (hv_ringbuf_avail_percent(&channel->outbound) > RING_AVAIL_PERCENT_HIWATER ||
+ (hv_get_avail_to_write_percent(&channel->outbound) >
+ RING_AVAIL_PERCENT_HIWATER ||
queue_sends < 1)) {
netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
ndev_ctx->eth_stats.wake_queue++;
@@ -757,7 +746,7 @@ static inline int netvsc_send_pkt(
struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet->q_idx);
u64 req_id;
int ret;
- u32 ring_avail = hv_ringbuf_avail_percent(&out_channel->outbound);
+ u32 ring_avail = hv_get_avail_to_write_percent(&out_channel->outbound);

nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
if (skb)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index faea0be18924..b0b1c2fd2b7b 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -35,7 +35,6 @@
#include <linux/slab.h>
#include <linux/rtnetlink.h>
#include <linux/netpoll.h>
-#include <linux/reciprocal_div.h>

#include <net/arp.h>
#include <net/route.h>
@@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128;
module_param(ring_size, uint, S_IRUGO);
MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
unsigned int netvsc_ring_bytes __ro_after_init;
-struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;

static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
NETIF_MSG_LINK | NETIF_MSG_IFUP |
@@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void)
ring_size);
}
netvsc_ring_bytes = ring_size * PAGE_SIZE;
- netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes);

ret = vmbus_driver_register(&netvsc_drv);
if (ret)
--
2.14.1


2018-03-23 00:18:35

by Long Li

[permalink] [raw]
Subject: [PATCH 3/3] Storvsc: Select channel based on available percentage of ring buffer to write

From: Long Li <[email protected]>

This is a best effort for estimating on how busy the ring buffer is for
that channel, based on available buffer to write in percentage. It is still
possible that at the time of actual ring buffer write, the space may not be
available due to other processes may be writing at the time.

Selecting a channel based on how full it is can reduce the possibility that
a ring buffer write will fail, and avoid the situation a channel is over
busy.

Now it's possible that storvsc can use a smaller ring buffer size
(e.g. 40k bytes) to take advantage of cache locality.

Signed-off-by: Long Li <[email protected]>
---
drivers/scsi/storvsc_drv.c | 62 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index a2ec0bc9e9fa..96681c4f75cb 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -395,6 +395,12 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");

module_param(storvsc_vcpus_per_sub_channel, int, S_IRUGO);
MODULE_PARM_DESC(storvsc_vcpus_per_sub_channel, "Ratio of VCPUs to subchannels");
+
+static int ring_avail_percent_lowater = 10;
+module_param(ring_avail_percent_lowater, int, S_IRUGO);
+MODULE_PARM_DESC(ring_avail_percent_lowater,
+ "Select a channel if available ring size > this in percent");
+
/*
* Timeout in seconds for all devices managed by this driver.
*/
@@ -1285,9 +1291,9 @@ static int storvsc_do_io(struct hv_device *device,
{
struct storvsc_device *stor_device;
struct vstor_packet *vstor_packet;
- struct vmbus_channel *outgoing_channel;
+ struct vmbus_channel *outgoing_channel, *channel;
int ret = 0;
- struct cpumask alloced_mask;
+ struct cpumask alloced_mask, other_numa_mask;
int tgt_cpu;

vstor_packet = &request->vstor_packet;
@@ -1301,22 +1307,53 @@ static int storvsc_do_io(struct hv_device *device,
/*
* Select an an appropriate channel to send the request out.
*/
-
if (stor_device->stor_chns[q_num] != NULL) {
outgoing_channel = stor_device->stor_chns[q_num];
- if (outgoing_channel->target_cpu == smp_processor_id()) {
+ if (outgoing_channel->target_cpu == q_num) {
/*
* Ideally, we want to pick a different channel if
* available on the same NUMA node.
*/
cpumask_and(&alloced_mask, &stor_device->alloced_cpus,
cpumask_of_node(cpu_to_node(q_num)));
- for_each_cpu_wrap(tgt_cpu, &alloced_mask,
- outgoing_channel->target_cpu + 1) {
- if (tgt_cpu != outgoing_channel->target_cpu) {
- outgoing_channel =
- stor_device->stor_chns[tgt_cpu];
- break;
+
+ for_each_cpu_wrap(tgt_cpu, &alloced_mask, q_num + 1) {
+ if (tgt_cpu == q_num)
+ continue;
+ channel = stor_device->stor_chns[tgt_cpu];
+ if (hv_get_avail_to_write_percent(
+ &channel->outbound)
+ > ring_avail_percent_lowater) {
+ outgoing_channel = channel;
+ goto found_channel;
+ }
+ }
+
+ /*
+ * All the othe channels on the same NUMA node are
+ * busy. Try to use the channel with the current CPU
+ */
+ if (hv_get_avail_to_write_percent(
+ &outgoing_channel->outbound)
+ > ring_avail_percent_lowater)
+ goto found_channel;
+
+ /*
+ * If we reach here, all the channels on the current
+ * NUMA node are busy. Try to find a channel in
+ * other NUMA nodes
+ */
+ cpumask_andnot(&other_numa_mask,
+ &stor_device->alloced_cpus,
+ cpumask_of_node(cpu_to_node(q_num)));
+
+ for_each_cpu(tgt_cpu, &other_numa_mask) {
+ channel = stor_device->stor_chns[tgt_cpu];
+ if (hv_get_avail_to_write_percent(
+ &channel->outbound)
+ > ring_avail_percent_lowater) {
+ outgoing_channel = channel;
+ goto found_channel;
}
}
}
@@ -1324,7 +1361,7 @@ static int storvsc_do_io(struct hv_device *device,
outgoing_channel = get_og_chn(stor_device, q_num);
}

-
+found_channel:
vstor_packet->flags |= REQUEST_COMPLETION_FLAG;

vstor_packet->vm_srb.length = (sizeof(struct vmscsi_request) -
@@ -1733,7 +1770,8 @@ static int storvsc_probe(struct hv_device *device,
}

scsi_driver.can_queue = (max_outstanding_req_per_channel *
- (max_sub_channels + 1));
+ (max_sub_channels + 1)) *
+ (100 - ring_avail_percent_lowater) / 100;

host = scsi_host_alloc(&scsi_driver,
sizeof(struct hv_host_device));
--
2.14.1


2018-03-23 15:02:14

by Haiyang Zhang

[permalink] [raw]
Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage



> -----Original Message-----
> From: Long Li <[email protected]>
> Sent: Thursday, March 22, 2018 8:16 PM
> To: KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; Stephen Hemminger <[email protected]>;
> James E . J . Bottomley <[email protected]>; Martin K . Petersen
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> Cc: Long Li <[email protected]>
> Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer
> percentage
>
> From: Long Li <[email protected]>
>
> In Vmbus, we have defined a function to calculate available ring buffer
> percentage to write.
>
> Use that function and remove duplicate netvsc code.
>
> Signed-off-by: Long Li <[email protected]>
> ---
> drivers/net/hyperv/netvsc.c | 17 +++--------------
> drivers/net/hyperv/netvsc_drv.c | 3 ---
> 2 files changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index
> 0265d703eb03..8af0069e4d8c 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -31,7 +31,6 @@
> #include <linux/vmalloc.h>
> #include <linux/rtnetlink.h>
> #include <linux/prefetch.h>
> -#include <linux/reciprocal_div.h>
>
> #include <asm/sync_bitops.h>
>
> @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device *device)
> #define RING_AVAIL_PERCENT_HIWATER 20 #define
> RING_AVAIL_PERCENT_LOWATER 10
>
> -/*
> - * Get the percentage of available bytes to write in the ring.
> - * The return value is in range from 0 to 100.
> - */
> -static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info
> *ring_info) -{
> - u32 avail_write = hv_get_bytes_to_write(ring_info);
> -
> - return reciprocal_divide(avail_write * 100, netvsc_ring_reciprocal);
> -}
> -
> static inline void netvsc_free_send_slot(struct netvsc_device *net_device,
> u32 index)
> {
> @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct
> netvsc_device *net_device,
> wake_up(&net_device->wait_drain);
>
> if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx)) &&
> - (hv_ringbuf_avail_percent(&channel->outbound) >
> RING_AVAIL_PERCENT_HIWATER ||
> + (hv_get_avail_to_write_percent(&channel->outbound) >
> + RING_AVAIL_PERCENT_HIWATER ||
> queue_sends < 1)) {
> netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
> ndev_ctx->eth_stats.wake_queue++;
> @@ -757,7 +746,7 @@ static inline int netvsc_send_pkt(
> struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet->q_idx);
> u64 req_id;
> int ret;
> - u32 ring_avail = hv_ringbuf_avail_percent(&out_channel->outbound);
> + u32 ring_avail =
> +hv_get_avail_to_write_percent(&out_channel->outbound);
>
> nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> if (skb)
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index faea0be18924..b0b1c2fd2b7b 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -35,7 +35,6 @@
> #include <linux/slab.h>
> #include <linux/rtnetlink.h>
> #include <linux/netpoll.h>
> -#include <linux/reciprocal_div.h>
>
> #include <net/arp.h>
> #include <net/route.h>
> @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128;
> module_param(ring_size, uint, S_IRUGO); MODULE_PARM_DESC(ring_size,
> "Ring buffer size (# of pages)"); unsigned int netvsc_ring_bytes __ro_after_init;
> -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;
>
> static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
> NETIF_MSG_LINK | NETIF_MSG_IFUP |
> @@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void)
> ring_size);
> }
> netvsc_ring_bytes = ring_size * PAGE_SIZE;
> - netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes);
>
> ret = vmbus_driver_register(&netvsc_drv);
> if (ret)
> --


Please also remove netvsc_ring_reciprocal from hyperv_net.h
Thanks.

Reviewed-by: Haiyang Zhang <[email protected]>

2018-03-23 15:12:42

by Haiyang Zhang

[permalink] [raw]
Subject: RE: [PATCH 1/3] Vmbus: Add function to report available ring buffer to write in total ring size percentage



> -----Original Message-----
> From: Long Li <[email protected]>
> Sent: Thursday, March 22, 2018 8:16 PM
> To: KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; Stephen Hemminger <[email protected]>;
> James E . J . Bottomley <[email protected]>; Martin K . Petersen
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> Cc: Long Li <[email protected]>
> Subject: [PATCH 1/3] Vmbus: Add function to report available ring buffer to
> write in total ring size percentage
>
> From: Long Li <[email protected]>
>
> Netvsc has a similar function to calculate how much ring buffer in percentage
> is available to write. This function is useful for storvsc and other vmbus devices.
>
> Define a similar function in vmbus to be used by storvsc.
>
> Signed-off-by: Long Li <[email protected]>
> ---

Reviewed-by: Haiyang Zhang <[email protected]>


2018-03-23 15:13:47

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage



> -----Original Message-----
> From: Haiyang Zhang
> Sent: Friday, March 23, 2018 8:01 AM
> To: Long Li <[email protected]>; KY Srinivasan
> <[email protected]>; Stephen Hemminger <[email protected]>;
> James E . J . Bottomley <[email protected]>; Martin K . Petersen
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]
> Cc: Long Li <[email protected]>
> Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring
> buffer percentage
>
>
>
> > -----Original Message-----
> > From: Long Li <[email protected]>
> > Sent: Thursday, March 22, 2018 8:16 PM
> > To: KY Srinivasan <[email protected]>; Haiyang Zhang
> > <[email protected]>; Stephen Hemminger
> <[email protected]>;
> > James E . J . Bottomley <[email protected]>; Martin K . Petersen
> > <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected]
> > Cc: Long Li <[email protected]>
> > Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring
> buffer
> > percentage
> >
> > From: Long Li <[email protected]>
> >
> > In Vmbus, we have defined a function to calculate available ring buffer
> > percentage to write.
> >
> > Use that function and remove duplicate netvsc code.
> >
> > Signed-off-by: Long Li <[email protected]>
> > ---
> > drivers/net/hyperv/netvsc.c | 17 +++--------------
> > drivers/net/hyperv/netvsc_drv.c | 3 ---
> > 2 files changed, 3 insertions(+), 17 deletions(-)

Why is the patch being sent to the scsi list and not to the network mailing list and
Dave Miller.

K. Y
> >
> > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index
> > 0265d703eb03..8af0069e4d8c 100644
> > --- a/drivers/net/hyperv/netvsc.c
> > +++ b/drivers/net/hyperv/netvsc.c
> > @@ -31,7 +31,6 @@
> > #include <linux/vmalloc.h>
> > #include <linux/rtnetlink.h>
> > #include <linux/prefetch.h>
> > -#include <linux/reciprocal_div.h>
> >
> > #include <asm/sync_bitops.h>
> >
> > @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device
> *device)
> > #define RING_AVAIL_PERCENT_HIWATER 20 #define
> > RING_AVAIL_PERCENT_LOWATER 10
> >
> > -/*
> > - * Get the percentage of available bytes to write in the ring.
> > - * The return value is in range from 0 to 100.
> > - */
> > -static u32 hv_ringbuf_avail_percent(const struct hv_ring_buffer_info
> > *ring_info) -{
> > - u32 avail_write = hv_get_bytes_to_write(ring_info);
> > -
> > - return reciprocal_divide(avail_write * 100, netvsc_ring_reciprocal);
> > -}
> > -
> > static inline void netvsc_free_send_slot(struct netvsc_device
> *net_device,
> > u32 index)
> > {
> > @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct
> > netvsc_device *net_device,
> > wake_up(&net_device->wait_drain);
> >
> > if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx))
> &&
> > - (hv_ringbuf_avail_percent(&channel->outbound) >
> > RING_AVAIL_PERCENT_HIWATER ||
> > + (hv_get_avail_to_write_percent(&channel->outbound) >
> > + RING_AVAIL_PERCENT_HIWATER ||
> > queue_sends < 1)) {
> > netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
> > ndev_ctx->eth_stats.wake_queue++;
> > @@ -757,7 +746,7 @@ static inline int netvsc_send_pkt(
> > struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet-
> >q_idx);
> > u64 req_id;
> > int ret;
> > - u32 ring_avail = hv_ringbuf_avail_percent(&out_channel-
> >outbound);
> > + u32 ring_avail =
> > +hv_get_avail_to_write_percent(&out_channel->outbound);
> >
> > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> > if (skb)
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index faea0be18924..b0b1c2fd2b7b 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -35,7 +35,6 @@
> > #include <linux/slab.h>
> > #include <linux/rtnetlink.h>
> > #include <linux/netpoll.h>
> > -#include <linux/reciprocal_div.h>
> >
> > #include <net/arp.h>
> > #include <net/route.h>
> > @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init = 128;
> > module_param(ring_size, uint, S_IRUGO);
> MODULE_PARM_DESC(ring_size,
> > "Ring buffer size (# of pages)"); unsigned int netvsc_ring_bytes
> __ro_after_init;
> > -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;
> >
> > static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
> > NETIF_MSG_LINK | NETIF_MSG_IFUP |
> > @@ -2186,7 +2184,6 @@ static int __init netvsc_drv_init(void)
> > ring_size);
> > }
> > netvsc_ring_bytes = ring_size * PAGE_SIZE;
> > - netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes);
> >
> > ret = vmbus_driver_register(&netvsc_drv);
> > if (ret)
> > --
>
>
> Please also remove netvsc_ring_reciprocal from hyperv_net.h
> Thanks.
>
> Reviewed-by: Haiyang Zhang <[email protected]>

2018-03-23 17:43:21

by Long Li

[permalink] [raw]
Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring buffer percentage

> Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate ring
> buffer percentage
>
>
>
> > -----Original Message-----
> > From: Haiyang Zhang
> > Sent: Friday, March 23, 2018 8:01 AM
> > To: Long Li <[email protected]>; KY Srinivasan
> > <[email protected]>; Stephen Hemminger <[email protected]>;
> James
> > E . J . Bottomley <[email protected]>; Martin K . Petersen
> > <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected]
> > Cc: Long Li <[email protected]>
> > Subject: RE: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate
> > ring buffer percentage
> >
> >
> >
> > > -----Original Message-----
> > > From: Long Li <[email protected]>
> > > Sent: Thursday, March 22, 2018 8:16 PM
> > > To: KY Srinivasan <[email protected]>; Haiyang Zhang
> > > <[email protected]>; Stephen Hemminger
> > <[email protected]>;
> > > James E . J . Bottomley <[email protected]>; Martin K . Petersen
> > > <[email protected]>; [email protected]; linux-
> > > [email protected]; [email protected]
> > > Cc: Long Li <[email protected]>
> > > Subject: [PATCH 2/3] Netvsc: Use the vmbus functiton to calculate
> > > ring
> > buffer
> > > percentage
> > >
> > > From: Long Li <[email protected]>
> > >
> > > In Vmbus, we have defined a function to calculate available ring
> > > buffer percentage to write.
> > >
> > > Use that function and remove duplicate netvsc code.
> > >
> > > Signed-off-by: Long Li <[email protected]>
> > > ---
> > > drivers/net/hyperv/netvsc.c | 17 +++--------------
> > > drivers/net/hyperv/netvsc_drv.c | 3 ---
> > > 2 files changed, 3 insertions(+), 17 deletions(-)
>
> Why is the patch being sent to the scsi list and not to the network mailing list
> and Dave Miller.

I will re-send the patch.

>
> K. Y
> > >
> > > diff --git a/drivers/net/hyperv/netvsc.c
> > > b/drivers/net/hyperv/netvsc.c
> > index
> > > 0265d703eb03..8af0069e4d8c 100644
> > > --- a/drivers/net/hyperv/netvsc.c
> > > +++ b/drivers/net/hyperv/netvsc.c
> > > @@ -31,7 +31,6 @@
> > > #include <linux/vmalloc.h>
> > > #include <linux/rtnetlink.h>
> > > #include <linux/prefetch.h>
> > > -#include <linux/reciprocal_div.h>
> > >
> > > #include <asm/sync_bitops.h>
> > >
> > > @@ -590,17 +589,6 @@ void netvsc_device_remove(struct hv_device
> > *device)
> > > #define RING_AVAIL_PERCENT_HIWATER 20 #define
> > > RING_AVAIL_PERCENT_LOWATER 10
> > >
> > > -/*
> > > - * Get the percentage of available bytes to write in the ring.
> > > - * The return value is in range from 0 to 100.
> > > - */
> > > -static u32 hv_ringbuf_avail_percent(const struct
> > > hv_ring_buffer_info
> > > *ring_info) -{
> > > - u32 avail_write = hv_get_bytes_to_write(ring_info);
> > > -
> > > - return reciprocal_divide(avail_write * 100, netvsc_ring_reciprocal);
> > > -}
> > > -
> > > static inline void netvsc_free_send_slot(struct netvsc_device
> > *net_device,
> > > u32 index)
> > > {
> > > @@ -649,7 +637,8 @@ static void netvsc_send_tx_complete(struct
> > > netvsc_device *net_device,
> > > wake_up(&net_device->wait_drain);
> > >
> > > if (netif_tx_queue_stopped(netdev_get_tx_queue(ndev, q_idx))
> > &&
> > > - (hv_ringbuf_avail_percent(&channel->outbound) >
> > > RING_AVAIL_PERCENT_HIWATER ||
> > > + (hv_get_avail_to_write_percent(&channel->outbound) >
> > > + RING_AVAIL_PERCENT_HIWATER ||
> > > queue_sends < 1)) {
> > > netif_tx_wake_queue(netdev_get_tx_queue(ndev, q_idx));
> > > ndev_ctx->eth_stats.wake_queue++; @@ -757,7 +746,7 @@
> static
> > >inline int netvsc_send_pkt(
> > > struct netdev_queue *txq = netdev_get_tx_queue(ndev, packet-
> > >q_idx);
> > > u64 req_id;
> > > int ret;
> > > - u32 ring_avail = hv_ringbuf_avail_percent(&out_channel-
> > >outbound);
> > > + u32 ring_avail =
> > > +hv_get_avail_to_write_percent(&out_channel->outbound);
> > >
> > > nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> > > if (skb)
> > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c
> > > index faea0be18924..b0b1c2fd2b7b 100644
> > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > @@ -35,7 +35,6 @@
> > > #include <linux/slab.h>
> > > #include <linux/rtnetlink.h>
> > > #include <linux/netpoll.h>
> > > -#include <linux/reciprocal_div.h>
> > >
> > > #include <net/arp.h>
> > > #include <net/route.h>
> > > @@ -55,7 +54,6 @@ static unsigned int ring_size __ro_after_init =
> > > 128; module_param(ring_size, uint, S_IRUGO);
> > MODULE_PARM_DESC(ring_size,
> > > "Ring buffer size (# of pages)"); unsigned int netvsc_ring_bytes
> > __ro_after_init;
> > > -struct reciprocal_value netvsc_ring_reciprocal __ro_after_init;
> > >
> > > static const u32 default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE |
> > > NETIF_MSG_LINK | NETIF_MSG_IFUP | @@ -
> 2186,7 +2184,6 @@ static
> > > int __init netvsc_drv_init(void)
> > > ring_size);
> > > }
> > > netvsc_ring_bytes = ring_size * PAGE_SIZE;
> > > - netvsc_ring_reciprocal = reciprocal_value(netvsc_ring_bytes);
> > >
> > > ret = vmbus_driver_register(&netvsc_drv);
> > > if (ret)
> > > --
> >
> >
> > Please also remove netvsc_ring_reciprocal from hyperv_net.h Thanks.

I will fix it and re-send the patch.

> >
> > Reviewed-by: Haiyang Zhang <[email protected]>

2018-03-24 05:56:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 3/3] Storvsc: Select channel based on available percentage of ring buffer to write

Hi Long,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc6]
[cannot apply to linus/master net-next/master net/master next-20180323]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Long-Li/Vmbus-Add-function-to-report-available-ring-buffer-to-write-in-total-ring-size-percentage/20180324-124431
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All warnings (new ones prefixed by >>):

drivers//scsi/storvsc_drv.c: In function 'storvsc_do_io':
>> drivers//scsi/storvsc_drv.c:1402:1: warning: the frame size of 2064 bytes is larger than 2048 bytes [-Wframe-larger-than=]
}
^

vim +1402 drivers//scsi/storvsc_drv.c

d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1287
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1288
c1b3d067 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1289 static int storvsc_do_io(struct hv_device *device,
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1290 struct storvsc_cmd_request *request, u16 q_num)
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1291 {
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1292 struct storvsc_device *stor_device;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1293 struct vstor_packet *vstor_packet;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1294 struct vmbus_channel *outgoing_channel, *channel;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1295 int ret = 0;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1296 struct cpumask alloced_mask, other_numa_mask;
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1297 int tgt_cpu;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1298
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1299 vstor_packet = &request->vstor_packet;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1300 stor_device = get_out_stor_device(device);
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1301
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1302 if (!stor_device)
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1303 return -ENODEV;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1304
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1305
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1306 request->device = device;
6f94d5de drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1307 /*
6f94d5de drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1308 * Select an an appropriate channel to send the request out.
6f94d5de drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1309 */
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1310 if (stor_device->stor_chns[q_num] != NULL) {
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1311 outgoing_channel = stor_device->stor_chns[q_num];
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1312 if (outgoing_channel->target_cpu == q_num) {
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1313 /*
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1314 * Ideally, we want to pick a different channel if
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1315 * available on the same NUMA node.
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1316 */
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1317 cpumask_and(&alloced_mask, &stor_device->alloced_cpus,
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1318 cpumask_of_node(cpu_to_node(q_num)));
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1319
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1320 for_each_cpu_wrap(tgt_cpu, &alloced_mask, q_num + 1) {
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1321 if (tgt_cpu == q_num)
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1322 continue;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1323 channel = stor_device->stor_chns[tgt_cpu];
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1324 if (hv_get_avail_to_write_percent(
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1325 &channel->outbound)
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1326 > ring_avail_percent_lowater) {
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1327 outgoing_channel = channel;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1328 goto found_channel;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1329 }
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1330 }
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1331
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1332 /*
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1333 * All the othe channels on the same NUMA node are
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1334 * busy. Try to use the channel with the current CPU
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1335 */
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1336 if (hv_get_avail_to_write_percent(
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1337 &outgoing_channel->outbound)
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1338 > ring_avail_percent_lowater)
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1339 goto found_channel;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1340
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1341 /*
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1342 * If we reach here, all the channels on the current
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1343 * NUMA node are busy. Try to find a channel in
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1344 * other NUMA nodes
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1345 */
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1346 cpumask_andnot(&other_numa_mask,
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1347 &stor_device->alloced_cpus,
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1348 cpumask_of_node(cpu_to_node(q_num)));
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1349
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1350 for_each_cpu(tgt_cpu, &other_numa_mask) {
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1351 channel = stor_device->stor_chns[tgt_cpu];
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1352 if (hv_get_avail_to_write_percent(
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1353 &channel->outbound)
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1354 > ring_avail_percent_lowater) {
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1355 outgoing_channel = channel;
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1356 goto found_channel;
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1357 }
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1358 }
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1359 }
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1360 } else {
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1361 outgoing_channel = get_og_chn(stor_device, q_num);
d86adf48 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2016-12-14 1362 }
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1363
e75adf20 drivers/scsi/storvsc_drv.c Long Li 2018-03-22 1364 found_channel:
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1365 vstor_packet->flags |= REQUEST_COMPLETION_FLAG;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1366
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1367 vstor_packet->vm_srb.length = (sizeof(struct vmscsi_request) -
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1368 vmscsi_size_delta);
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1369
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1370
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1371 vstor_packet->vm_srb.sense_info_length = sense_buffer_size;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1372
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1373
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1374 vstor_packet->vm_srb.data_transfer_length =
be0cf6ca drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2015-03-27 1375 request->payload->range.len;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1376
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1377 vstor_packet->operation = VSTOR_OPERATION_EXECUTE_SRB;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1378
be0cf6ca drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2015-03-27 1379 if (request->payload->range.len) {
be0cf6ca drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2015-03-27 1380
be0cf6ca drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2015-03-27 1381 ret = vmbus_sendpacket_mpb_desc(outgoing_channel,
be0cf6ca drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2015-03-27 1382 request->payload, request->payload_sz,
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1383 vstor_packet,
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1384 (sizeof(struct vstor_packet) -
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1385 vmscsi_size_delta),
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1386 (unsigned long)request);
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1387 } else {
0147dabc drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2015-03-27 1388 ret = vmbus_sendpacket(outgoing_channel, vstor_packet,
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1389 (sizeof(struct vstor_packet) -
8b612fa2 drivers/scsi/storvsc_drv.c K. Y. Srinivasan 2013-06-04 1390 vmscsi_size_delta),
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1391 (unsigned long)request,
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1392 VM_PKT_DATA_INBAND,
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1393 VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1394 }
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1395
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1396 if (ret != 0)
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1397 return ret;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1398
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1399 atomic_inc(&stor_device->num_outstanding_req);
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1400
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1401 return ret;
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 @1402 }
8dcf37d4 drivers/staging/hv/storvsc_drv.c K. Y. Srinivasan 2011-08-27 1403

:::::: The code at line 1402 was first introduced by commit
:::::: 8dcf37d446f103d55b96beddb100db6a0ad8d0ba Staging: hv: storvsc: Include storvsc.c in storvsc_drv.c

:::::: TO: K. Y. Srinivasan <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (13.16 kB)
.config.gz (39.82 kB)
Download all attachments