Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2014-04-07 Thread Igor Mammedov
On Thu, 27 Dec 2012 11:43:30 +0800
Wanlong Gao gaowanl...@cn.fujitsu.com wrote:

 On 12/27/2012 11:28 AM, Jason Wang wrote:
  On 12/26/2012 06:19 PM, Wanlong Gao wrote:
  On 12/26/2012 06:06 PM, Jason Wang wrote:
  On 12/26/2012 03:06 PM, Wanlong Gao wrote:
  Add a cpu notifier to virtio-net, so that we can reset the
  virtqueue affinity if the cpu hotplug happens. It improve
  the performance through enabling or disabling the virtqueue
  affinity after doing cpu hotplug.
  Hi Wanlong:
 
  Thanks for looking at this.
  Cc: Rusty Russell ru...@rustcorp.com.au
  Cc: Michael S. Tsirkin m...@redhat.com
  Cc: Jason Wang jasow...@redhat.com
  Cc: virtualization@lists.linux-foundation.org
  Cc: net...@vger.kernel.org
  Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
  ---
   drivers/net/virtio_net.c | 39 ++-
   1 file changed, 38 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
  index a6fcf15..9710cf4 100644
  --- a/drivers/net/virtio_net.c
  +++ b/drivers/net/virtio_net.c
  @@ -26,6 +26,7 @@
   #include linux/scatterlist.h
   #include linux/if_vlan.h
   #include linux/slab.h
  +#include linux/cpu.h
   
   static int napi_weight = 128;
   module_param(napi_weight, int, 0444);
  @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
   module_param(csum, bool, 0444);
   module_param(gso, bool, 0444);
   
  +static bool cpu_hotplug = false;
  +
   /* FIXME: MTU in config. */
   #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
   #define GOOD_COPY_LEN   128
  @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct 
  virtnet_info *vi, bool set)
   vi-affinity_hint_set = false;
   }
   
  +static int virtnet_cpu_callback(struct notifier_block *nfb,
  +   unsigned long action, void *hcpu)
  +{
  +switch(action) {
  +case CPU_ONLINE:
  +case CPU_ONLINE_FROZEN:
  +case CPU_DEAD:
  +case CPU_DEAD_FROZEN:
  +cpu_hotplug = true;
  +break;
  +default:
  +break;
  +}
  +return NOTIFY_OK;
  +}
  +
  +static struct notifier_block virtnet_cpu_notifier = {
  +.notifier_call = virtnet_cpu_callback,
  +};
  +
   static void virtnet_get_ringparam(struct net_device *dev,
   struct ethtool_ringparam *ring)
   {
  @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device 
  *dev, int new_mtu)
*/
   static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff 
  *skb)
   {
  -int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
  +int txq;
  +
  +if (unlikely(cpu_hotplug == true)) {
  +virtnet_set_affinity(netdev_priv(dev), true);
  +cpu_hotplug = false;
  +}
  +
  Why don't you just do this in callback?
  Callback can just give us a hcpu, can't get the virtnet_info from 
  callback. Am I missing something?
  
  Well, I think you can just embed the notifier block into virtnet_info,
  then use something like container_of in the callback to make the
  notifier per device. This also solve the concern of Eric.
 
 Yeah, thank you very much for your suggestion. I'll try it.
 
  btw. Does qemu/kvm support cpu-hotplug now?
  From http://www.linux-kvm.org/page/CPUHotPlug, I saw that qemu-kvm can 
  support hotplug
  but failed to merge to qemu.git, right?
  
  Not sure, I just try latest qemu, it even does not have a cpu_set command.
 
 Adding Igor to CC,
 
 As I know, hotplug support is cleaned from qemu, and Igor want to rework it 
 but not been completed?
 I'm not sure about that, Igor, could you send out your tech-preview-patches?
CPU hot-add is supported by upstream now, hot-remove is not supported yet,
besides of qemu work it would require quite a work on kvm side as well.

 
 Thanks,
 Wanlong Gao
 
  
  Thanks
 
  Thanks,
  Wanlong Gao
 
  
  
 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2013-01-03 Thread Eric Dumazet
On Wed, 2012-12-26 at 15:06 +0800, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.
 
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct virtnet_info 
 *vi, bool set)
   vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 +unsigned long action, void *hcpu)
 +{
 + switch(action) {
 + case CPU_ONLINE:
 + case CPU_ONLINE_FROZEN:
 + case CPU_DEAD:
 + case CPU_DEAD_FROZEN:
 + cpu_hotplug = true;
 + break;
 + default:
 + break;
 + }
 + return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 + .notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
   struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device *dev, 
 int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
  {
 - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 + int txq;
 +
 + if (unlikely(cpu_hotplug == true)) {
 + virtnet_set_affinity(netdev_priv(dev), true);
 + cpu_hotplug = false;
 + }
 +
 + txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 smp_processor_id();
  
   while (unlikely(txq = dev-real_num_tx_queues))
 @@ -1248,6 +1278,8 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
  {
   struct virtio_device *vdev = vi-vdev;
  
 + unregister_hotcpu_notifier(virtnet_cpu_notifier);
 +
   virtnet_set_affinity(vi, false);
  
   vdev-config-del_vqs(vdev);
 @@ -1372,6 +1404,11 @@ static int init_vqs(struct virtnet_info *vi)
   goto err_free;
  
   virtnet_set_affinity(vi, true);
 +
 + ret = register_hotcpu_notifier(virtnet_cpu_notifier);
 + if (ret)
 + goto err_free;
 +
   return 0;
  
  err_free:

It looks like this patch assumes virtio_net supports a single instance.

Try your patch with two instances, I am pretty sure it wont do very
well.

It seems to me you need something else than a single boolean.

A sequence number for example should be better...



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-27 Thread Michael S. Tsirkin
On Thu, Dec 27, 2012 at 11:34:16AM +0800, Jason Wang wrote:
 On 12/26/2012 06:46 PM, Michael S. Tsirkin wrote:
  On Wed, Dec 26, 2012 at 03:06:54PM +0800, Wanlong Gao wrote:
  Add a cpu notifier to virtio-net, so that we can reset the
  virtqueue affinity if the cpu hotplug happens. It improve
  the performance through enabling or disabling the virtqueue
  affinity after doing cpu hotplug.
 
  Cc: Rusty Russell ru...@rustcorp.com.au
  Cc: Michael S. Tsirkin m...@redhat.com
  Cc: Jason Wang jasow...@redhat.com
  Cc: virtualization@lists.linux-foundation.org
  Cc: net...@vger.kernel.org
  Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
  Thanks for looking into this.
  Some comments:
 
  1. Looks like the logic in
  virtnet_set_affinity (and in virtnet_select_queue)
  will not work very well when CPU IDs are not
  consequitive. This can happen with hot unplug.
 
  Maybe we should add a VQ allocator, and defining
  a per-cpu variable specifying the VQ instead
  of using CPU ID.
 
 Yes, and generate the affinity hint based on the mapping. Btw, what does
 VQ allocator means here?

Some logic to generate CPU to VQ mapping.

 
 
  2. The below code seems racy e.g. when CPU is added
  during device init.
 
  3. using a global cpu_hotplug seems inelegant.
  In any case we should document what is the
  meaning of this variable.
 
  ---
   drivers/net/virtio_net.c | 39 ++-
   1 file changed, 38 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
  index a6fcf15..9710cf4 100644
  --- a/drivers/net/virtio_net.c
  +++ b/drivers/net/virtio_net.c
  @@ -26,6 +26,7 @@
   #include linux/scatterlist.h
   #include linux/if_vlan.h
   #include linux/slab.h
  +#include linux/cpu.h
   
   static int napi_weight = 128;
   module_param(napi_weight, int, 0444);
  @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
   module_param(csum, bool, 0444);
   module_param(gso, bool, 0444);
   
  +static bool cpu_hotplug = false;
  +
   /* FIXME: MTU in config. */
   #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
   #define GOOD_COPY_LEN 128
  @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct 
  virtnet_info *vi, bool set)
 vi-affinity_hint_set = false;
   }
   
  +static int virtnet_cpu_callback(struct notifier_block *nfb,
  + unsigned long action, void *hcpu)
  +{
  +  switch(action) {
  +  case CPU_ONLINE:
  +  case CPU_ONLINE_FROZEN:
  +  case CPU_DEAD:
  +  case CPU_DEAD_FROZEN:
  +  cpu_hotplug = true;
  +  break;
  +  default:
  +  break;
  +  }
  +  return NOTIFY_OK;
  +}
  +
  +static struct notifier_block virtnet_cpu_notifier = {
  +  .notifier_call = virtnet_cpu_callback,
  +};
  +
   static void virtnet_get_ringparam(struct net_device *dev,
 struct ethtool_ringparam *ring)
   {
  @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device 
  *dev, int new_mtu)
*/
   static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff 
  *skb)
   {
  -  int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
  +  int txq;
  +
  +  if (unlikely(cpu_hotplug == true)) {
  +  virtnet_set_affinity(netdev_priv(dev), true);
  +  cpu_hotplug = false;
  +  }
  +
  +  txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
   smp_processor_id();
   
 while (unlikely(txq = dev-real_num_tx_queues))
  @@ -1248,6 +1278,8 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
   {
 struct virtio_device *vdev = vi-vdev;
   
  +  unregister_hotcpu_notifier(virtnet_cpu_notifier);
  +
 virtnet_set_affinity(vi, false);
   
 vdev-config-del_vqs(vdev);
  @@ -1372,6 +1404,11 @@ static int init_vqs(struct virtnet_info *vi)
 goto err_free;
   
 virtnet_set_affinity(vi, true);
  +
  +  ret = register_hotcpu_notifier(virtnet_cpu_notifier);
  +  if (ret)
  +  goto err_free;
  +
 return 0;
   
   err_free:
  -- 
  1.8.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-26 Thread Jason Wang
On 12/26/2012 03:06 PM, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.

Hi Wanlong:

Thanks for looking at this.
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct virtnet_info 
 *vi, bool set)
   vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 +unsigned long action, void *hcpu)
 +{
 + switch(action) {
 + case CPU_ONLINE:
 + case CPU_ONLINE_FROZEN:
 + case CPU_DEAD:
 + case CPU_DEAD_FROZEN:
 + cpu_hotplug = true;
 + break;
 + default:
 + break;
 + }
 + return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 + .notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
   struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device *dev, 
 int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
  {
 - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 + int txq;
 +
 + if (unlikely(cpu_hotplug == true)) {
 + virtnet_set_affinity(netdev_priv(dev), true);
 + cpu_hotplug = false;
 + }
 +

Why don't you just do this in callback?

btw. Does qemu/kvm support cpu-hotplug now?
 + txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 smp_processor_id();
  
   while (unlikely(txq = dev-real_num_tx_queues))
 @@ -1248,6 +1278,8 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
  {
   struct virtio_device *vdev = vi-vdev;
  
 + unregister_hotcpu_notifier(virtnet_cpu_notifier);
 +
   virtnet_set_affinity(vi, false);
  
   vdev-config-del_vqs(vdev);
 @@ -1372,6 +1404,11 @@ static int init_vqs(struct virtnet_info *vi)
   goto err_free;
  
   virtnet_set_affinity(vi, true);
 +
 + ret = register_hotcpu_notifier(virtnet_cpu_notifier);
 + if (ret)
 + goto err_free;
 +
   return 0;
  
  err_free:

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-26 Thread Wanlong Gao
On 12/26/2012 06:06 PM, Jason Wang wrote:
 On 12/26/2012 03:06 PM, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.
 
 Hi Wanlong:
 
 Thanks for looking at this.
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN   128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct virtnet_info 
 *vi, bool set)
  vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 +   unsigned long action, void *hcpu)
 +{
 +switch(action) {
 +case CPU_ONLINE:
 +case CPU_ONLINE_FROZEN:
 +case CPU_DEAD:
 +case CPU_DEAD_FROZEN:
 +cpu_hotplug = true;
 +break;
 +default:
 +break;
 +}
 +return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 +.notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
  struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device *dev, 
 int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
  {
 -int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 +int txq;
 +
 +if (unlikely(cpu_hotplug == true)) {
 +virtnet_set_affinity(netdev_priv(dev), true);
 +cpu_hotplug = false;
 +}
 +
 
 Why don't you just do this in callback?

Callback can just give us a hcpu, can't get the virtnet_info from callback. 
Am I missing something?

 
 btw. Does qemu/kvm support cpu-hotplug now?

From http://www.linux-kvm.org/page/CPUHotPlug, I saw that qemu-kvm can support 
hotplug
but failed to merge to qemu.git, right?

Thanks,
Wanlong Gao

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-26 Thread Michael S. Tsirkin
On Wed, Dec 26, 2012 at 03:06:54PM +0800, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.
 
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com

Thanks for looking into this.
Some comments:

1. Looks like the logic in
virtnet_set_affinity (and in virtnet_select_queue)
will not work very well when CPU IDs are not
consequitive. This can happen with hot unplug.

Maybe we should add a VQ allocator, and defining
a per-cpu variable specifying the VQ instead
of using CPU ID.


2. The below code seems racy e.g. when CPU is added
during device init.

3. using a global cpu_hotplug seems inelegant.
In any case we should document what is the
meaning of this variable.

 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct virtnet_info 
 *vi, bool set)
   vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 +unsigned long action, void *hcpu)
 +{
 + switch(action) {
 + case CPU_ONLINE:
 + case CPU_ONLINE_FROZEN:
 + case CPU_DEAD:
 + case CPU_DEAD_FROZEN:
 + cpu_hotplug = true;
 + break;
 + default:
 + break;
 + }
 + return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 + .notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
   struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device *dev, 
 int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
  {
 - int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 + int txq;
 +
 + if (unlikely(cpu_hotplug == true)) {
 + virtnet_set_affinity(netdev_priv(dev), true);
 + cpu_hotplug = false;
 + }
 +
 + txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 smp_processor_id();
  
   while (unlikely(txq = dev-real_num_tx_queues))
 @@ -1248,6 +1278,8 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
  {
   struct virtio_device *vdev = vi-vdev;
  
 + unregister_hotcpu_notifier(virtnet_cpu_notifier);
 +
   virtnet_set_affinity(vi, false);
  
   vdev-config-del_vqs(vdev);
 @@ -1372,6 +1404,11 @@ static int init_vqs(struct virtnet_info *vi)
   goto err_free;
  
   virtnet_set_affinity(vi, true);
 +
 + ret = register_hotcpu_notifier(virtnet_cpu_notifier);
 + if (ret)
 + goto err_free;
 +
   return 0;
  
  err_free:
 -- 
 1.8.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-26 Thread Jason Wang
On 12/26/2012 06:46 PM, Michael S. Tsirkin wrote:
 On Wed, Dec 26, 2012 at 03:06:54PM +0800, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.

 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
 Thanks for looking into this.
 Some comments:

 1. Looks like the logic in
 virtnet_set_affinity (and in virtnet_select_queue)
 will not work very well when CPU IDs are not
 consequitive. This can happen with hot unplug.

 Maybe we should add a VQ allocator, and defining
 a per-cpu variable specifying the VQ instead
 of using CPU ID.

Yes, and generate the affinity hint based on the mapping. Btw, what does
VQ allocator means here?


 2. The below code seems racy e.g. when CPU is added
   during device init.

 3. using a global cpu_hotplug seems inelegant.
 In any case we should document what is the
 meaning of this variable.

 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN   128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct virtnet_info 
 *vi, bool set)
  vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 +   unsigned long action, void *hcpu)
 +{
 +switch(action) {
 +case CPU_ONLINE:
 +case CPU_ONLINE_FROZEN:
 +case CPU_DEAD:
 +case CPU_DEAD_FROZEN:
 +cpu_hotplug = true;
 +break;
 +default:
 +break;
 +}
 +return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 +.notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
  struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device *dev, 
 int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
  {
 -int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 +int txq;
 +
 +if (unlikely(cpu_hotplug == true)) {
 +virtnet_set_affinity(netdev_priv(dev), true);
 +cpu_hotplug = false;
 +}
 +
 +txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
smp_processor_id();
  
  while (unlikely(txq = dev-real_num_tx_queues))
 @@ -1248,6 +1278,8 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
  {
  struct virtio_device *vdev = vi-vdev;
  
 +unregister_hotcpu_notifier(virtnet_cpu_notifier);
 +
  virtnet_set_affinity(vi, false);
  
  vdev-config-del_vqs(vdev);
 @@ -1372,6 +1404,11 @@ static int init_vqs(struct virtnet_info *vi)
  goto err_free;
  
  virtnet_set_affinity(vi, true);
 +
 +ret = register_hotcpu_notifier(virtnet_cpu_notifier);
 +if (ret)
 +goto err_free;
 +
  return 0;
  
  err_free:
 -- 
 1.8.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-26 Thread Jason Wang
On 12/26/2012 06:19 PM, Wanlong Gao wrote:
 On 12/26/2012 06:06 PM, Jason Wang wrote:
 On 12/26/2012 03:06 PM, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.
 Hi Wanlong:

 Thanks for looking at this.
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN  128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct virtnet_info 
 *vi, bool set)
 vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 +  unsigned long action, void *hcpu)
 +{
 +   switch(action) {
 +   case CPU_ONLINE:
 +   case CPU_ONLINE_FROZEN:
 +   case CPU_DEAD:
 +   case CPU_DEAD_FROZEN:
 +   cpu_hotplug = true;
 +   break;
 +   default:
 +   break;
 +   }
 +   return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 +   .notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
 struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device 
 *dev, int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff 
 *skb)
  {
 -   int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 +   int txq;
 +
 +   if (unlikely(cpu_hotplug == true)) {
 +   virtnet_set_affinity(netdev_priv(dev), true);
 +   cpu_hotplug = false;
 +   }
 +
 Why don't you just do this in callback?
 Callback can just give us a hcpu, can't get the virtnet_info from callback. 
 Am I missing something?

Well, I think you can just embed the notifier block into virtnet_info,
then use something like container_of in the callback to make the
notifier per device. This also solve the concern of Eric.
 btw. Does qemu/kvm support cpu-hotplug now?
 From http://www.linux-kvm.org/page/CPUHotPlug, I saw that qemu-kvm can 
 support hotplug
 but failed to merge to qemu.git, right?

Not sure, I just try latest qemu, it even does not have a cpu_set command.

Thanks

 Thanks,
 Wanlong Gao


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug

2012-12-26 Thread Wanlong Gao
On 12/27/2012 11:28 AM, Jason Wang wrote:
 On 12/26/2012 06:19 PM, Wanlong Gao wrote:
 On 12/26/2012 06:06 PM, Jason Wang wrote:
 On 12/26/2012 03:06 PM, Wanlong Gao wrote:
 Add a cpu notifier to virtio-net, so that we can reset the
 virtqueue affinity if the cpu hotplug happens. It improve
 the performance through enabling or disabling the virtqueue
 affinity after doing cpu hotplug.
 Hi Wanlong:

 Thanks for looking at this.
 Cc: Rusty Russell ru...@rustcorp.com.au
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Jason Wang jasow...@redhat.com
 Cc: virtualization@lists.linux-foundation.org
 Cc: net...@vger.kernel.org
 Signed-off-by: Wanlong Gao gaowanl...@cn.fujitsu.com
 ---
  drivers/net/virtio_net.c | 39 ++-
  1 file changed, 38 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index a6fcf15..9710cf4 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -26,6 +26,7 @@
  #include linux/scatterlist.h
  #include linux/if_vlan.h
  #include linux/slab.h
 +#include linux/cpu.h
  
  static int napi_weight = 128;
  module_param(napi_weight, int, 0444);
 @@ -34,6 +35,8 @@ static bool csum = true, gso = true;
  module_param(csum, bool, 0444);
  module_param(gso, bool, 0444);
  
 +static bool cpu_hotplug = false;
 +
  /* FIXME: MTU in config. */
  #define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
  #define GOOD_COPY_LEN 128
 @@ -1041,6 +1044,26 @@ static void virtnet_set_affinity(struct 
 virtnet_info *vi, bool set)
vi-affinity_hint_set = false;
  }
  
 +static int virtnet_cpu_callback(struct notifier_block *nfb,
 + unsigned long action, void *hcpu)
 +{
 +  switch(action) {
 +  case CPU_ONLINE:
 +  case CPU_ONLINE_FROZEN:
 +  case CPU_DEAD:
 +  case CPU_DEAD_FROZEN:
 +  cpu_hotplug = true;
 +  break;
 +  default:
 +  break;
 +  }
 +  return NOTIFY_OK;
 +}
 +
 +static struct notifier_block virtnet_cpu_notifier = {
 +  .notifier_call = virtnet_cpu_callback,
 +};
 +
  static void virtnet_get_ringparam(struct net_device *dev,
struct ethtool_ringparam *ring)
  {
 @@ -1131,7 +1154,14 @@ static int virtnet_change_mtu(struct net_device 
 *dev, int new_mtu)
   */
  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff 
 *skb)
  {
 -  int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
 +  int txq;
 +
 +  if (unlikely(cpu_hotplug == true)) {
 +  virtnet_set_affinity(netdev_priv(dev), true);
 +  cpu_hotplug = false;
 +  }
 +
 Why don't you just do this in callback?
 Callback can just give us a hcpu, can't get the virtnet_info from 
 callback. Am I missing something?
 
 Well, I think you can just embed the notifier block into virtnet_info,
 then use something like container_of in the callback to make the
 notifier per device. This also solve the concern of Eric.

Yeah, thank you very much for your suggestion. I'll try it.

 btw. Does qemu/kvm support cpu-hotplug now?
 From http://www.linux-kvm.org/page/CPUHotPlug, I saw that qemu-kvm can 
 support hotplug
 but failed to merge to qemu.git, right?
 
 Not sure, I just try latest qemu, it even does not have a cpu_set command.

Adding Igor to CC,

As I know, hotplug support is cleaned from qemu, and Igor want to rework it but 
not been completed?
I'm not sure about that, Igor, could you send out your tech-preview-patches?

Thanks,
Wanlong Gao

 
 Thanks

 Thanks,
 Wanlong Gao

 
 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization