* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-26 7:06 [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug Wanlong Gao
@ 2012-12-26 10:06 ` Jason Wang
2012-12-26 10:19 ` Wanlong Gao
2012-12-26 10:46 ` Michael S. Tsirkin
2012-12-26 15:51 ` Eric Dumazet
2 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2012-12-26 10:06 UTC (permalink / raw)
To: Wanlong Gao; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin
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 <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@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?
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:
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-26 10:06 ` Jason Wang
@ 2012-12-26 10:19 ` Wanlong Gao
2012-12-27 3:28 ` Jason Wang
0 siblings, 1 reply; 10+ messages in thread
From: Wanlong Gao @ 2012-12-26 10:19 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin
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 <rusty@rustcorp.com.au>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: netdev@vger.kernel.org
>> Signed-off-by: Wanlong Gao <gaowanlong@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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-26 10:19 ` Wanlong Gao
@ 2012-12-27 3:28 ` Jason Wang
2012-12-27 3:43 ` Wanlong Gao
0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2012-12-27 3:28 UTC (permalink / raw)
To: gaowanlong; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin
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 <rusty@rustcorp.com.au>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: netdev@vger.kernel.org
>>> Signed-off-by: Wanlong Gao <gaowanlong@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
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-27 3:28 ` Jason Wang
@ 2012-12-27 3:43 ` Wanlong Gao
2014-04-07 6:06 ` Igor Mammedov
0 siblings, 1 reply; 10+ messages in thread
From: Wanlong Gao @ 2012-12-27 3:43 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
imammedo
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 <rusty@rustcorp.com.au>
>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>> Cc: virtualization@lists.linux-foundation.org
>>>> Cc: netdev@vger.kernel.org
>>>> Signed-off-by: Wanlong Gao <gaowanlong@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
>>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-27 3:43 ` Wanlong Gao
@ 2014-04-07 6:06 ` Igor Mammedov
0 siblings, 0 replies; 10+ messages in thread
From: Igor Mammedov @ 2014-04-07 6:06 UTC (permalink / raw)
To: gaowanlong; +Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization
On Thu, 27 Dec 2012 11:43:30 +0800
Wanlong Gao <gaowanlong@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 <rusty@rustcorp.com.au>
> >>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >>>> Cc: Jason Wang <jasowang@redhat.com>
> >>>> Cc: virtualization@lists.linux-foundation.org
> >>>> Cc: netdev@vger.kernel.org
> >>>> Signed-off-by: Wanlong Gao <gaowanlong@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
> >>
> >
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-26 7:06 [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug Wanlong Gao
2012-12-26 10:06 ` Jason Wang
@ 2012-12-26 10:46 ` Michael S. Tsirkin
2012-12-27 3:34 ` Jason Wang
2012-12-26 15:51 ` Eric Dumazet
2 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-12-26 10:46 UTC (permalink / raw)
To: Wanlong Gao; +Cc: netdev, linux-kernel, virtualization
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 <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@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_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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-26 10:46 ` Michael S. Tsirkin
@ 2012-12-27 3:34 ` Jason Wang
2012-12-27 11:51 ` Michael S. Tsirkin
0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2012-12-27 3:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Wanlong Gao, linux-kernel, Rusty Russell, virtualization, netdev
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 <rusty@rustcorp.com.au>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: netdev@vger.kernel.org
>> Signed-off-by: Wanlong Gao <gaowanlong@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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-27 3:34 ` Jason Wang
@ 2012-12-27 11:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-12-27 11:51 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, virtualization
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 <rusty@rustcorp.com.au>
> >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >> Cc: Jason Wang <jasowang@redhat.com>
> >> Cc: virtualization@lists.linux-foundation.org
> >> Cc: netdev@vger.kernel.org
> >> Signed-off-by: Wanlong Gao <gaowanlong@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug
2012-12-26 7:06 [RFC PATCH] virtio-net: reset virtqueue affinity when doing cpu hotplug Wanlong Gao
2012-12-26 10:06 ` Jason Wang
2012-12-26 10:46 ` Michael S. Tsirkin
@ 2012-12-26 15:51 ` Eric Dumazet
2 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2012-12-26 15:51 UTC (permalink / raw)
To: Wanlong Gao
Cc: linux-kernel, Rusty Russell, Michael S. Tsirkin, Jason Wang,
virtualization, netdev
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 <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@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;
> + }
> +
> + 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...
^ permalink raw reply [flat|nested] 10+ messages in thread