[PATCH V3 0/2] limit xen vnic max queues number to online cpus number

[PATCH V3 0/2] limit xen vnic max queues number to online cpus number

[Joe Jin]

Currently xen vnic allowed to create lots of queues by set module param
max_queues(both netback and netfront), when queues number larger than
cpus number, it does not help for performance but need more cpu time.

This patchset limit netback and netfront max queues number to online
cpus number.

Joe Jin (2):
  xen-netback: limit xen vif max queues number to online cpus
  xen-netfront: limit vnic max_queues number to online cpus

---
Changes in v3:
  - Corrected the range info [1-CPUs].

Changes in v2:
  - Replace param_get_int to param_get_uint.
  - Use '%u' for unsigned int when print.
  - Replace EINVAL to ERANGE when out of range [0-CPUs].
  - Reset the perm of max_queues on sysfs to 0644.

 drivers/net/xen-netback/netback.c |   28 ++++++++++++++++++++++------
 drivers/net/xen-netfront.c        |   28 ++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 12 deletions(-)

[PATCH V3 1/2] xen-netback: limit xen vif max queues number to online cpus

[Joe Jin]

Should not allocate xen vif queues number more than online cpus.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/net/xen-netback/netback.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index ec98d43..021dcb0 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -68,7 +68,9 @@ unsigned int rx_stall_timeout_msecs = 60000;
 module_param(rx_stall_timeout_msecs, uint, 0444);
 
 unsigned int xenvif_max_queues;
-module_param_named(max_queues, xenvif_max_queues, uint, 0644);
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp);
+module_param_call(max_queues, xennet_set_max_queues, param_get_uint,
+		  &xenvif_max_queues, 0644);
 MODULE_PARM_DESC(max_queues,
 		 "Maximum number of queues per virtual interface");
 
@@ -107,6 +109,20 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif_queue *queue
 					     u16      size,
 					     u16      flags);
 
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp)
+{
+	unsigned int cpus = num_online_cpus();
+	unsigned int max_queues = simple_strtoul(val, NULL, 10);
+
+	if (max_queues == 0 || max_queues > cpus) {
+		pr_info("max_queues %u is out of range [1 - %u]!\n", 
+			max_queues, cpus);
+		return -ERANGE;
+	}
+
+	return param_set_uint(val, kp);
+}
+
 static inline unsigned long idx_to_pfn(struct xenvif_queue *queue,
 				       u16 idx)
 {
@@ -2110,15 +2126,15 @@ int xenvif_dealloc_kthread(void *data)
 static int __init netback_init(void)
 {
 	int rc = 0;
+	unsigned int cpus = num_online_cpus();
 
 	if (!xen_domain())
 		return -ENODEV;
 
-	/* Allow as many queues as there are CPUs if user has not
-	 * specified a value.
-	 */
-	if (xenvif_max_queues == 0)
-		xenvif_max_queues = num_online_cpus();
+	/* Allow at most as many queues as CPUs. */
+	if (xenvif_max_queues == 0 || xenvif_max_queues > cpus)
+		xenvif_max_queues = cpus;
+	pr_info("vif max_queues: %u\n", xenvif_max_queues);
 
 	if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) {
 		pr_info("fatal_skb_slots too small (%d), bump it to XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n",
-- 
1.7.1

[PATCH V3 1/2] xen-netback: limit xen vif max queues number to online cpus

[Joe Jin]

Should not allocate xen vif queues number more than online cpus.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/net/xen-netback/netback.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index ec98d43..021dcb0 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -68,7 +68,9 @@ unsigned int rx_stall_timeout_msecs = 60000;
 module_param(rx_stall_timeout_msecs, uint, 0444);
 
 unsigned int xenvif_max_queues;
-module_param_named(max_queues, xenvif_max_queues, uint, 0644);
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp);
+module_param_call(max_queues, xennet_set_max_queues, param_get_uint,
+		  &xenvif_max_queues, 0644);
 MODULE_PARM_DESC(max_queues,
 		 "Maximum number of queues per virtual interface");
 
@@ -107,6 +109,20 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif_queue *queue
 					     u16      size,
 					     u16      flags);
 
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp)
+{
+	unsigned int cpus = num_online_cpus();
+	unsigned int max_queues = simple_strtoul(val, NULL, 10);
+
+	if (max_queues == 0 || max_queues > cpus) {
+		pr_info("max_queues %u is out of range [1 - %u]!\n", 
+			max_queues, cpus);
+		return -ERANGE;
+	}
+
+	return param_set_uint(val, kp);
+}
+
 static inline unsigned long idx_to_pfn(struct xenvif_queue *queue,
 				       u16 idx)
 {
@@ -2110,15 +2126,15 @@ int xenvif_dealloc_kthread(void *data)
 static int __init netback_init(void)
 {
 	int rc = 0;
+	unsigned int cpus = num_online_cpus();
 
 	if (!xen_domain())
 		return -ENODEV;
 
-	/* Allow as many queues as there are CPUs if user has not
-	 * specified a value.
-	 */
-	if (xenvif_max_queues == 0)
-		xenvif_max_queues = num_online_cpus();
+	/* Allow at most as many queues as CPUs. */
+	if (xenvif_max_queues == 0 || xenvif_max_queues > cpus)
+		xenvif_max_queues = cpus;
+	pr_info("vif max_queues: %u\n", xenvif_max_queues);
 
 	if (fatal_skb_slots < XEN_NETBK_LEGACY_SLOTS_MAX) {
 		pr_info("fatal_skb_slots too small (%d), bump it to XEN_NETBK_LEGACY_SLOTS_MAX (%d)\n",
-- 
1.7.1

[PATCH V3 2/2] xen-netfront: limit vnic max_queues number to online cpus

[Joe Jin]

Should not allocate vnic queues number more than online cpus.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/net/xen-netfront.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f821a97..5cc2f4a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -58,7 +58,9 @@
 
 /* Module parameters */
 static unsigned int xennet_max_queues;
-module_param_named(max_queues, xennet_max_queues, uint, 0644);
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp);
+module_param_call(max_queues, xennet_set_max_queues, param_get_uint,
+		  &xennet_max_queues, 0644);
 MODULE_PARM_DESC(max_queues,
 		 "Maximum number of queues per virtual interface");
 
@@ -164,6 +166,19 @@ struct netfront_rx_info {
 	struct xen_netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX - 1];
 };
 
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp)
+{
+	unsigned int cpus = num_online_cpus();
+	unsigned int max_queues = simple_strtoul(val, NULL, 10);
+
+	if (max_queues == 0 || max_queues > cpus) {
+		pr_err("max_queues %u is out of range [1 - %u]!\n", 
+		       max_queues, cpus);
+		return -ERANGE;
+	}
+	return param_set_uint(val, kp);
+}
+
 static void skb_entry_set_link(union skb_entry *list, unsigned short id)
 {
 	list->link = id;
@@ -2126,6 +2141,8 @@ static struct xenbus_driver netfront_driver = {
 
 static int __init netif_init(void)
 {
+	unsigned int cpus = num_online_cpus();
+
 	if (!xen_domain())
 		return -ENODEV;
 
@@ -2134,11 +2151,10 @@ static int __init netif_init(void)
 
 	pr_info("Initialising Xen virtual ethernet driver\n");
 
-	/* Allow as many queues as there are CPUs if user has not
-	 * specified a value.
-	 */
-	if (xennet_max_queues == 0)
-		xennet_max_queues = num_online_cpus();
+	/* Allow at most as many queues as there are CPUs. */
+	if (xennet_max_queues == 0 || xennet_max_queues > cpus)
+		xennet_max_queues = cpus;
+	pr_info("max_queues: %d\n", xennet_max_queues);
 
 	return xenbus_register_frontend(&netfront_driver);
 }
-- 
1.7.1

[PATCH V3 2/2] xen-netfront: limit vnic max_queues number to online cpus

[Joe Jin]

Should not allocate vnic queues number more than online cpus.

Signed-off-by: Joe Jin <joe.jin@oracle.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/net/xen-netfront.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f821a97..5cc2f4a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -58,7 +58,9 @@
 
 /* Module parameters */
 static unsigned int xennet_max_queues;
-module_param_named(max_queues, xennet_max_queues, uint, 0644);
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp);
+module_param_call(max_queues, xennet_set_max_queues, param_get_uint,
+		  &xennet_max_queues, 0644);
 MODULE_PARM_DESC(max_queues,
 		 "Maximum number of queues per virtual interface");
 
@@ -164,6 +166,19 @@ struct netfront_rx_info {
 	struct xen_netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX - 1];
 };
 
+static int xennet_set_max_queues(const char *val, struct kernel_param *kp)
+{
+	unsigned int cpus = num_online_cpus();
+	unsigned int max_queues = simple_strtoul(val, NULL, 10);
+
+	if (max_queues == 0 || max_queues > cpus) {
+		pr_err("max_queues %u is out of range [1 - %u]!\n", 
+		       max_queues, cpus);
+		return -ERANGE;
+	}
+	return param_set_uint(val, kp);
+}
+
 static void skb_entry_set_link(union skb_entry *list, unsigned short id)
 {
 	list->link = id;
@@ -2126,6 +2141,8 @@ static struct xenbus_driver netfront_driver = {
 
 static int __init netif_init(void)
 {
+	unsigned int cpus = num_online_cpus();
+
 	if (!xen_domain())
 		return -ENODEV;
 
@@ -2134,11 +2151,10 @@ static int __init netif_init(void)
 
 	pr_info("Initialising Xen virtual ethernet driver\n");
 
-	/* Allow as many queues as there are CPUs if user has not
-	 * specified a value.
-	 */
-	if (xennet_max_queues == 0)
-		xennet_max_queues = num_online_cpus();
+	/* Allow at most as many queues as there are CPUs. */
+	if (xennet_max_queues == 0 || xennet_max_queues > cpus)
+		xennet_max_queues = cpus;
+	pr_info("max_queues: %d\n", xennet_max_queues);
 
 	return xenbus_register_frontend(&netfront_driver);
 }
-- 
1.7.1

Re: [Xen-devel] [PATCH V3 0/2] limit xen vnic max queues number to online cpus number

[Andrew Cooper]

On 23/10/15 10:42, Joe Jin wrote:
> Currently xen vnic allowed to create lots of queues by set module param
> max_queues(both netback and netfront), when queues number larger than
> cpus number, it does not help for performance but need more cpu time.
>
> This patchset limit netback and netfront max queues number to online
> cpus number.
>
> Joe Jin (2):
>   xen-netback: limit xen vif max queues number to online cpus
>   xen-netfront: limit vnic max_queues number to online cpus

Please do not exclude the possibility for making crazy numbers of queues.

I can appreciate the want to prevent people shooting themselves in the
foot, but being able oversubscribe the number of queues is important for
performance testing.

~Andrew

Re: [PATCH V3 0/2] limit xen vnic max queues number to online cpus number

[Andrew Cooper]

On 23/10/15 10:42, Joe Jin wrote:
> Currently xen vnic allowed to create lots of queues by set module param
> max_queues(both netback and netfront), when queues number larger than
> cpus number, it does not help for performance but need more cpu time.
>
> This patchset limit netback and netfront max queues number to online
> cpus number.
>
> Joe Jin (2):
>   xen-netback: limit xen vif max queues number to online cpus
>   xen-netfront: limit vnic max_queues number to online cpus

Please do not exclude the possibility for making crazy numbers of queues.

I can appreciate the want to prevent people shooting themselves in the
foot, but being able oversubscribe the number of queues is important for
performance testing.

~Andrew

Re: [Xen-devel] [PATCH V3 1/2] xen-netback: limit xen vif max queues number to online cpus

[Andrew Cooper]

On 23/10/15 10:44, Joe Jin wrote:
> Should not allocate xen vif queues number more than online cpus.
>
> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/net/xen-netback/netback.c |   28 ++++++++++++++++++++++------
>  1 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index ec98d43..021dcb0 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -68,7 +68,9 @@ unsigned int rx_stall_timeout_msecs = 60000;
>  module_param(rx_stall_timeout_msecs, uint, 0444);
>  
>  unsigned int xenvif_max_queues;
> -module_param_named(max_queues, xenvif_max_queues, uint, 0644);
> +static int xennet_set_max_queues(const char *val, struct kernel_param *kp);
> +module_param_call(max_queues, xennet_set_max_queues, param_get_uint,
> +		  &xenvif_max_queues, 0644);
>  MODULE_PARM_DESC(max_queues,
>  		 "Maximum number of queues per virtual interface");
>  
> @@ -107,6 +109,20 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif_queue *queue
>  					     u16      size,
>  					     u16      flags);
>  
> +static int xennet_set_max_queues(const char *val, struct kernel_param *kp)
> +{
> +	unsigned int cpus = num_online_cpus();

This calculation is problematic in the scenario where a guest boots up,
then has more cpus hotplugged in, then wants to set up a sensible number
of queues.

~Andrew

Re: [PATCH V3 1/2] xen-netback: limit xen vif max queues number to online cpus

[Andrew Cooper]

On 23/10/15 10:44, Joe Jin wrote:
> Should not allocate xen vif queues number more than online cpus.
>
> Signed-off-by: Joe Jin <joe.jin@oracle.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  drivers/net/xen-netback/netback.c |   28 ++++++++++++++++++++++------
>  1 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index ec98d43..021dcb0 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -68,7 +68,9 @@ unsigned int rx_stall_timeout_msecs = 60000;
>  module_param(rx_stall_timeout_msecs, uint, 0444);
>  
>  unsigned int xenvif_max_queues;
> -module_param_named(max_queues, xenvif_max_queues, uint, 0644);
> +static int xennet_set_max_queues(const char *val, struct kernel_param *kp);
> +module_param_call(max_queues, xennet_set_max_queues, param_get_uint,
> +		  &xenvif_max_queues, 0644);
>  MODULE_PARM_DESC(max_queues,
>  		 "Maximum number of queues per virtual interface");
>  
> @@ -107,6 +109,20 @@ static struct xen_netif_rx_response *make_rx_response(struct xenvif_queue *queue
>  					     u16      size,
>  					     u16      flags);
>  
> +static int xennet_set_max_queues(const char *val, struct kernel_param *kp)
> +{
> +	unsigned int cpus = num_online_cpus();

This calculation is problematic in the scenario where a guest boots up,
then has more cpus hotplugged in, then wants to set up a sensible number
of queues.

~Andrew

Re: [PATCH V3 1/2] xen-netback: limit xen vif max queues number to online cpus

[Wei Liu]

On Fri, Oct 23, 2015 at 05:44:44PM +0800, Joe Jin wrote:
> Should not allocate xen vif queues number more than online cpus.

I think it's absolutely fine for administrators to override the value
should they choose to.

Wei.

Re: [PATCH V3 1/2] xen-netback: limit xen vif max queues number to online cpus

[Wei Liu]

On Fri, Oct 23, 2015 at 05:44:44PM +0800, Joe Jin wrote:
> Should not allocate xen vif queues number more than online cpus.

I think it's absolutely fine for administrators to override the value
should they choose to.

Wei.