* Re: [PATCH 2/4] mm/compaction: enable mobile-page migration
From: Minchan Kim @ 2015-08-10 7:19 UTC (permalink / raw)
To: Gioh Kim
Cc: aquini, mst, linux-api, linux-kernel, dri-devel, virtualization,
bfields, linux-mm, Gioh Kim, viro, akpm, linux-fsdevel, jlayton,
koct9i, iamjoonsoo.kim, vbabka
In-Reply-To: <20150731104309.GA22158@bbox>
On Fri, Jul 31, 2015 at 07:43:09PM +0900, Minchan Kim wrote:
> Hi Gioh,
>
> On Mon, Jul 13, 2015 at 05:35:17PM +0900, Gioh Kim wrote:
> > From: Gioh Kim <gurugio@hanmail.net>
> >
> > Add framework to register callback functions and check page mobility.
> > There are some modes for page isolation so that isolate interface
> > has arguments of page address and isolation mode while putback
> > interface has only page address as argument.
> >
> > Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> > Acked-by: Rafael Aquini <aquini@redhat.com>
> > ---
> > fs/proc/page.c | 3 ++
> > include/linux/compaction.h | 80 ++++++++++++++++++++++++++++++++++
> > include/linux/fs.h | 2 +
> > include/linux/page-flags.h | 19 ++++++++
> > include/uapi/linux/kernel-page-flags.h | 1 +
> > 5 files changed, 105 insertions(+)
> >
> > diff --git a/fs/proc/page.c b/fs/proc/page.c
> > index 7eee2d8..a4f5a00 100644
> > --- a/fs/proc/page.c
> > +++ b/fs/proc/page.c
> > @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page)
> > if (PageBalloon(page))
> > u |= 1 << KPF_BALLOON;
> >
> > + if (PageMobile(page))
> > + u |= 1 << KPF_MOBILE;
> > +
>
> Need a new page flag for non-LRU page migration?
> I am worry that we don't have empty slot for page flag on 32-bit.
>
> Aha, see SetPageMobile below. You use _mapcount.
> It seems to be work for non-LRU pages but unfortunately, zsmalloc
> already have used that field as own purpose so there is no room
> for it.
Gioh, I just sent a patch which zsmalloc doesn't use page->mapping
and _mapcount so I think zsmalloc could support compaction with your
patchset. Although zsmalloc can support compaction with it, I still
don't think it's a good that driver have to mark pages mobile via
_mapcount.
I hope you can find another way.
Thanks.
>
>
> > u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
> >
> > u |= kpf_copy_bit(k, KPF_SLAB, PG_slab);
> > diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> > index aa8f61c..f693072 100644
> > --- a/include/linux/compaction.h
> > +++ b/include/linux/compaction.h
> > @@ -1,6 +1,9 @@
> > #ifndef _LINUX_COMPACTION_H
> > #define _LINUX_COMPACTION_H
> >
> > +#include <linux/page-flags.h>
> > +#include <linux/pagemap.h>
> > +
> > /* Return values for compact_zone() and try_to_compact_pages() */
> > /* compaction didn't start as it was deferred due to past failures */
> > #define COMPACT_DEFERRED 0
> > @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone, int order,
> > bool alloc_success);
> > extern bool compaction_restarting(struct zone *zone, int order);
> >
> > +static inline bool mobile_page(struct page *page)
> > +{
> > + return page->mapping && (PageMobile(page) || PageBalloon(page));
>
>
> What's the locking rule to test mobile_page?
> Why we need such many checking?
>
> Why we need PageBalloon check?
> You are trying to make generic abstraction for non-LRU page to migrate
> but PageBalloon check in here breaks your goal.
>
> > +}
> > +
> > +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t mode)
> > +{
> > + bool ret = false;
> > +
> > + /*
> > + * Avoid burning cycles with pages that are yet under __free_pages(),
> > + * or just got freed under us.
> > + *
> > + * In case we 'win' a race for a mobile page being freed under us and
> > + * raise its refcount preventing __free_pages() from doing its job
> > + * the put_page() at the end of this block will take care of
> > + * release this page, thus avoiding a nasty leakage.
> > + */
>
> Good.
>
> > + if (unlikely(!get_page_unless_zero(page)))
> > + goto out;
> > +
> > + /*
> > + * As mobile pages are not isolated from LRU lists, concurrent
> > + * compaction threads can race against page migration functions
> > + * as well as race against the releasing a page.
>
> How can it race with releasing a page?
> We already get refcount above.
>
> Do you mean page->mapping tearing race?
> If so, zsmalloc should be chaned to hold a lock.
>
>
> > + *
> > + * In order to avoid having an already isolated mobile page
> > + * being (wrongly) re-isolated while it is under migration,
> > + * or to avoid attempting to isolate pages being released,
> > + * lets be sure we have the page lock
> > + * before proceeding with the mobile page isolation steps.
> > + */
> > + if (unlikely(!trylock_page(page)))
> > + goto out_putpage;
> > +
> > + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage))
> > + goto out_not_isolated;
>
> I cannot know how isolate_mobilepage is called by upper layer
> because this patch doesn't include it.
> Anyway, why we need such many checks to prove it's mobile page?
>
> mobile_page() {
> page->mapping && (PageMobile(page) || PageBalloon(page));
> }
>
> Additionally, added page->mapping->a_ops->isolatepage check
>
> Is it possible that a non-LRU page's address space provides
> only page->maping->migratepage without isolate/putback?
>
> Couldn't we make it simple like this?
>
> page->mapping && page->mapping->migratepage
>
> So, couldn't we make mobile_page like this
>
> PageMobile(struct page *page)
> {
> VM_BUG_ON_PAGE(!PageLocked(page), page);
> VM_BUG_ON_PAGE(PageLRU(page), page);
>
> return page->mapping && page->mapping->migratepage;
> }
>
> It will save _mapcount of struct page.
>
> > + ret = page->mapping->a_ops->isolatepage(page, mode);
> > + if (!ret)
> > + goto out_not_isolated;
> > + unlock_page(page);
> > + return ret;
> > +
> > +out_not_isolated:
> > + unlock_page(page);
> > +out_putpage:
> > + put_page(page);
> > +out:
> > + return ret;
> > +}
> > +
> > +static inline void putback_mobilepage(struct page *page)
> > +{
> > + /*
> > + * 'lock_page()' stabilizes the page and prevents races against
>
> What does it mean 'stabilize'?
> Clear comment is always welcome rather than vague word.
>
> > + * concurrent isolation threads attempting to re-isolate it.
> > + */
> > + lock_page(page);
> > + if (page->mapping && page->mapping->a_ops->putbackpage)
> > + page->mapping->a_ops->putbackpage(page);
> > + unlock_page(page);
> > + /* drop the extra ref count taken for mobile page isolation */
> > + put_page(page);
> > +}
> > #else
> > static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
> > unsigned int order, int alloc_flags,
> > @@ -83,6 +150,19 @@ static inline bool compaction_deferred(struct zone *zone, int order)
> > return true;
> > }
> >
> > +static inline bool mobile_page(struct page *page)
> > +{
> > + return false;
> > +}
> > +
> > +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t mode)
> > +{
> > + return false;
> > +}
> > +
> > +static inline void putback_mobilepage(struct page *page)
> > +{
> > +}
> > #endif /* CONFIG_COMPACTION */
> >
> > #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index a0653e5..2cc4b24 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -396,6 +396,8 @@ struct address_space_operations {
> > */
> > int (*migratepage) (struct address_space *,
> > struct page *, struct page *, enum migrate_mode);
> > + bool (*isolatepage) (struct page *, isolate_mode_t);
> > + void (*putbackpage) (struct page *);
> > int (*launder_page) (struct page *);
> > int (*is_partially_uptodate) (struct page *, unsigned long,
> > unsigned long);
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index f34e040..abef145 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -582,6 +582,25 @@ static inline void __ClearPageBalloon(struct page *page)
> > atomic_set(&page->_mapcount, -1);
> > }
> >
> > +#define PAGE_MOBILE_MAPCOUNT_VALUE (-255)
> > +
> > +static inline int PageMobile(struct page *page)
> > +{
> > + return atomic_read(&page->_mapcount) == PAGE_MOBILE_MAPCOUNT_VALUE;
> > +}
> > +
> > +static inline void __SetPageMobile(struct page *page)
> > +{
> > + VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
> > + atomic_set(&page->_mapcount, PAGE_MOBILE_MAPCOUNT_VALUE);
> > +}
> > +
> > +static inline void __ClearPageMobile(struct page *page)
> > +{
> > + VM_BUG_ON_PAGE(!PageMobile(page), page);
> > + atomic_set(&page->_mapcount, -1);
> > +}
> > +
> > /*
> > * If network-based swap is enabled, sl*b must keep track of whether pages
> > * were allocated from pfmemalloc reserves.
> > diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
> > index a6c4962..d50d9e8 100644
> > --- a/include/uapi/linux/kernel-page-flags.h
> > +++ b/include/uapi/linux/kernel-page-flags.h
> > @@ -33,6 +33,7 @@
> > #define KPF_THP 22
> > #define KPF_BALLOON 23
> > #define KPF_ZERO_PAGE 24
> > +#define KPF_MOBILE 25
> >
> >
> > #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
> > --
> > 2.1.4
> >
^ permalink raw reply
* Re: [PATCH net V2] virtio-net: drop NETIF_F_FRAGLIST
From: David Miller @ 2015-08-07 7:13 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, sergei.shtylyov, mst
In-Reply-To: <1438742044-31064-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 5 Aug 2015 10:34:04 +0800
> virtio declares support for NETIF_F_FRAGLIST, but assumes
> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
> always true with a fraglist.
>
> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
> the sg array, leading to memory corruption.
>
> Drop NETIF_F_FRAGLIST so we only get what we can handle.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied, thanks Jason.
^ permalink raw reply
* [PULL] vhost: fix for 4.2
From: Michael S. Tsirkin @ 2015-08-07 5:28 UTC (permalink / raw)
To: Linus Torvalds; +Cc: kvm, mst, netdev, linux-kernel, virtualization
The following changes since commit 74d33293e467df61de1b1d8b2fbe29e550dec33b:
Linux 4.2-rc5 (2015-08-02 18:34:55 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to df4198b1e0c4a7d1adde1e5c2ceb67ac10b391bb:
virtio-input: reset device and detach unused during remove (2015-08-06 10:40:35 +0300)
----------------------------------------------------------------
virtio: fix for 4.2
A last minute fix for the new virtio input driver. It seems pretty obvious, and
the problem it's fixing would be quite hard to debug.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
----------------------------------------------------------------
Jason Wang (1):
virtio-input: reset device and detach unused during remove
drivers/virtio/virtio_input.c | 4 ++++
1 file changed, 4 insertions(+)
^ permalink raw reply
* Re: [PATCH net V2] virtio-net: drop NETIF_F_FRAGLIST
From: David Miller @ 2015-08-06 22:49 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, sergei.shtylyov, virtualization
In-Reply-To: <20150806161042-mutt-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 6 Aug 2015 16:13:11 +0300
> On Wed, Aug 05, 2015 at 10:34:04AM +0800, Jason Wang wrote:
>> virtio declares support for NETIF_F_FRAGLIST, but assumes
>> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
>> always true with a fraglist.
>>
>> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
>> the sg array, leading to memory corruption.
>>
>> Drop NETIF_F_FRAGLIST so we only get what we can handle.
>>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> I noticed only now that this didn't Cc Dave, so of course it's
> not applied.
CC:'ing me isn't the requirement. CC:'ing netdev is, which he
did. If you check patchwork, it's there in the queue.
I'm just behind and heavily backlogged because I've been working
on other things.
^ permalink raw reply
* Re: [PATCH net V2] virtio-net: drop NETIF_F_FRAGLIST
From: Michael S. Tsirkin @ 2015-08-06 13:13 UTC (permalink / raw)
To: Jason Wang
Cc: netdev, David Miller, linux-kernel, sergei.shtylyov,
virtualization
In-Reply-To: <1438742044-31064-1-git-send-email-jasowang@redhat.com>
On Wed, Aug 05, 2015 at 10:34:04AM +0800, Jason Wang wrote:
> virtio declares support for NETIF_F_FRAGLIST, but assumes
> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
> always true with a fraglist.
>
> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
> the sg array, leading to memory corruption.
>
> Drop NETIF_F_FRAGLIST so we only get what we can handle.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
I noticed only now that this didn't Cc Dave, so of course it's
not applied. I'm preparing a pull request anyway, so
I'll merge this through my tree, and add Cc stable.
Dave - ok with you?
> ---
> - Change from V1: coding style fixes.
> - The patch is needed for stable.
> ---
> drivers/net/virtio_net.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7fbca37..237f8e5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* Do we support "hardware" checksums? */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
> - dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG;
> if (csum)
> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG;
>
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
> --
> 2.1.4
^ permalink raw reply
* maintainership of arch/x86/include/asm/paravirt_types.h
From: Juergen Gross @ 2015-08-06 12:20 UTC (permalink / raw)
To: jeremy@goop.org, chrisw@sous-sol.org, Alok Kataria, Rusty Russell,
virtualization, Linux Kernel Mailing List
I just realized that according to MAINTAINERS the file
arch/x86/include/asm/paravirt_types.h isn't part of the
PARAVIRT_OPS INTERFACE, but of x86.
Is this on purpose? I don't think so.
Juergen
^ permalink raw reply
* [PATCH] x86/paravirt: remove unused operation
From: Juergen Gross @ 2015-08-06 11:55 UTC (permalink / raw)
To: linux-kernel, jeremy, chrisw, akataria, rusty, virtualization
Cc: Juergen Gross
Remove the paravirt operation "get_tsc_khz" as it is used nowhere.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/include/asm/paravirt_types.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index a6b8f9f..5a18a66 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -97,7 +97,6 @@ struct pv_lazy_ops {
struct pv_time_ops {
unsigned long long (*sched_clock)(void);
unsigned long long (*steal_clock)(int cpu);
- unsigned long (*get_tsc_khz)(void);
};
struct pv_cpu_ops {
--
2.1.4
^ permalink raw reply related
* [PATCH V2] virtio-input: reset device and detach unused during remove
From: Jason Wang @ 2015-08-06 5:54 UTC (permalink / raw)
To: virtualization, linux-kernel; +Cc: mst
Spec requires a device reset during cleanup, so do it and avoid warn
in virtio core. And detach unused buffers to avoid memory leak.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from V1: detach unused buffers for sts queue.
---
drivers/virtio/virtio_input.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index 60e2a16..c96944b 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -313,6 +313,7 @@ err_init_vq:
static void virtinput_remove(struct virtio_device *vdev)
{
struct virtio_input *vi = vdev->priv;
+ void *buf;
unsigned long flags;
spin_lock_irqsave(&vi->lock, flags);
@@ -320,6 +321,9 @@ static void virtinput_remove(struct virtio_device *vdev)
spin_unlock_irqrestore(&vi->lock, flags);
input_unregister_device(vi->idev);
+ vdev->config->reset(vdev);
+ while ((buf = virtqueue_detach_unused_buf(vi->sts)) != NULL)
+ kfree(buf);
vdev->config->del_vqs(vdev);
kfree(vi);
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] virtio-input: reset device during remove
From: Jason Wang @ 2015-08-05 8:18 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, virtualization
In-Reply-To: <20150805104654-mutt-send-email-mst@redhat.com>
On 08/05/2015 03:56 PM, Michael S. Tsirkin wrote:
> On Wed, Aug 05, 2015 at 03:20:18PM +0800, Jason Wang wrote:
>> > Spec requires a device reset during cleanup, so do it and avoid warn
>> > in virtio core.
>> >
>> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> OK but now that I look at this driver, that's not enough.
>
> Need to also detach and free unused buffers, otherwise
> we leak memory in evt and sts queues.
Probably only sts. For evt queue, all buffer it used was an array
embedded in virtio_input structure (vi->evts[])
^ permalink raw reply
* Re: [PATCH] virtio-input: reset device during remove
From: Michael S. Tsirkin @ 2015-08-05 7:56 UTC (permalink / raw)
To: Jason Wang; +Cc: linux-kernel, virtualization
In-Reply-To: <1438759218-30038-1-git-send-email-jasowang@redhat.com>
On Wed, Aug 05, 2015 at 03:20:18PM +0800, Jason Wang wrote:
> Spec requires a device reset during cleanup, so do it and avoid warn
> in virtio core.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
OK but now that I look at this driver, that's not enough.
Need to also detach and free unused buffers, otherwise
we leak memory in evt and sts queues.
> ---
> drivers/virtio/virtio_input.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> index 60e2a16..6222f9b 100644
> --- a/drivers/virtio/virtio_input.c
> +++ b/drivers/virtio/virtio_input.c
> @@ -320,6 +320,7 @@ static void virtinput_remove(struct virtio_device *vdev)
> spin_unlock_irqrestore(&vi->lock, flags);
>
> input_unregister_device(vi->idev);
> + vdev->config->reset(vdev);
> vdev->config->del_vqs(vdev);
> kfree(vi);
> }
> --
> 2.1.4
^ permalink raw reply
* [PATCH] virtio-input: reset device during remove
From: Jason Wang @ 2015-08-05 7:20 UTC (permalink / raw)
To: virtualization, linux-kernel; +Cc: mst
Spec requires a device reset during cleanup, so do it and avoid warn
in virtio core.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/virtio/virtio_input.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index 60e2a16..6222f9b 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -320,6 +320,7 @@ static void virtinput_remove(struct virtio_device *vdev)
spin_unlock_irqrestore(&vi->lock, flags);
input_unregister_device(vi->idev);
+ vdev->config->reset(vdev);
vdev->config->del_vqs(vdev);
kfree(vi);
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net V2] virtio-net: drop NETIF_F_FRAGLIST
From: Michael S. Tsirkin @ 2015-08-05 6:33 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, sergei.shtylyov, virtualization
In-Reply-To: <1438742044-31064-1-git-send-email-jasowang@redhat.com>
On Wed, Aug 05, 2015 at 10:34:04AM +0800, Jason Wang wrote:
> virtio declares support for NETIF_F_FRAGLIST, but assumes
> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
> always true with a fraglist.
>
> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
> the sg array, leading to memory corruption.
>
> Drop NETIF_F_FRAGLIST so we only get what we can handle.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> - Change from V1: coding style fixes.
> - The patch is needed for stable.
> ---
> drivers/net/virtio_net.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7fbca37..237f8e5 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* Do we support "hardware" checksums? */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
> - dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG;
> if (csum)
> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG;
>
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
> --
> 2.1.4
^ permalink raw reply
* [PATCH net V2] virtio-net: drop NETIF_F_FRAGLIST
From: Jason Wang @ 2015-08-05 2:34 UTC (permalink / raw)
To: mst, virtualization, netdev, linux-kernel; +Cc: sergei.shtylyov
virtio declares support for NETIF_F_FRAGLIST, but assumes
that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
always true with a fraglist.
A longer fraglist in the skb will make the call to skb_to_sgvec overflow
the sg array, leading to memory corruption.
Drop NETIF_F_FRAGLIST so we only get what we can handle.
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
- Change from V1: coding style fixes.
- The patch is needed for stable.
---
drivers/net/virtio_net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7fbca37..237f8e5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Do we support "hardware" checksums? */
if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
/* This opens up the world of extra features. */
- dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+ dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG;
if (csum)
- dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+ dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG;
if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net] virtio-net: drop NETIF_F_FRAGLIST
From: Jason Wang @ 2015-08-05 2:32 UTC (permalink / raw)
To: Sergei Shtylyov, virtualization, netdev, linux-kernel; +Cc: mst
In-Reply-To: <55C09DC4.4070108@cogentembedded.com>
On 08/04/2015 07:11 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 8/4/2015 12:55 PM, Jason Wang wrote:
>
>> virtio declares support for NETIF_F_FRAGLIST, but assumes
>> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
>> always true with a fraglist.
>
>> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
>> the sg array, leading to memory corruption.
>
>> Drop NETIF_F_FRAGLIST so we only get what we can handle.
>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> The patch is needed for stable.
>> ---
>> drivers/net/virtio_net.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 7fbca37..2347a73 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device
>> *vdev)
>> /* Do we support "hardware" checksums? */
>> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
>> /* This opens up the world of extra features. */
>> - dev->hw_features |=
>> NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
>> + dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG;
>> if (csum)
>> - dev->features |=
>> NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
>> + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
>
> I'd have added spaces around | to match the style seen below.
>
Ok, will fix this in V2.
>> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
>> dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
>
> MBR, Sergei
>
^ permalink raw reply
* Re: [PATCH net] virtio-net: drop NETIF_F_FRAGLIST
From: Sergei Shtylyov @ 2015-08-04 11:11 UTC (permalink / raw)
To: Jason Wang, virtualization, netdev, linux-kernel; +Cc: mst
In-Reply-To: <1438682145-25986-1-git-send-email-jasowang@redhat.com>
Hello.
On 8/4/2015 12:55 PM, Jason Wang wrote:
> virtio declares support for NETIF_F_FRAGLIST, but assumes
> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
> always true with a fraglist.
> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
> the sg array, leading to memory corruption.
> Drop NETIF_F_FRAGLIST so we only get what we can handle.
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> The patch is needed for stable.
> ---
> drivers/net/virtio_net.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7fbca37..2347a73 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* Do we support "hardware" checksums? */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
> - dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG;
> if (csum)
> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
I'd have added spaces around | to match the style seen below.
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net] virtio-net: drop NETIF_F_FRAGLIST
From: Michael S. Tsirkin @ 2015-08-04 10:05 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <1438682145-25986-1-git-send-email-jasowang@redhat.com>
On Tue, Aug 04, 2015 at 05:55:45PM +0800, Jason Wang wrote:
> virtio declares support for NETIF_F_FRAGLIST, but assumes
> that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
> always true with a fraglist.
>
> A longer fraglist in the skb will make the call to skb_to_sgvec overflow
> the sg array, leading to memory corruption.
>
> Drop NETIF_F_FRAGLIST so we only get what we can handle.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Especially important now that virtio_net: add gro capability
was merged for net-next, making bridged setups create fraglists.
> ---
> The patch is needed for stable.
> ---
> drivers/net/virtio_net.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7fbca37..2347a73 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* Do we support "hardware" checksums? */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
> /* This opens up the world of extra features. */
> - dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG;
> if (csum)
> - dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
> + dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
>
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
> --
> 2.1.4
^ permalink raw reply
* [PATCH net] virtio-net: drop NETIF_F_FRAGLIST
From: Jason Wang @ 2015-08-04 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel; +Cc: mst
virtio declares support for NETIF_F_FRAGLIST, but assumes
that there are at most MAX_SKB_FRAGS + 2 fragments which isn't
always true with a fraglist.
A longer fraglist in the skb will make the call to skb_to_sgvec overflow
the sg array, leading to memory corruption.
Drop NETIF_F_FRAGLIST so we only get what we can handle.
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
The patch is needed for stable.
---
drivers/net/virtio_net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7fbca37..2347a73 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1756,9 +1756,9 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Do we support "hardware" checksums? */
if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
/* This opens up the world of extra features. */
- dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+ dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG;
if (csum)
- dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+ dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG;
if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: add gro capability
From: David Miller @ 2015-08-03 21:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, rick.jones2, virtualization, mst
In-Reply-To: <1438359917.2748.18.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 31 Jul 2015 18:25:17 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Straightforward patch to add GRO processing to virtio_net.
>
> napi_complete_done() usage allows more aggressive aggregation,
> opted-in by setting /sys/class/net/xxx/gro_flush_timeout
>
> Tested:
>
> Setting /sys/class/net/xxx/gro_flush_timeout to 1000 nsec,
> Rick Jones reported following results.
>
> One VM of each on a pair of OpenStack compute nodes with E5-2650Lv3 CPUs
> and Intel 82599ES-based NICs. So, two "before" and two "after" VMs.
> The OpenStack compute nodes were running OpenStack Kilo, with VxLAN
> encapsulation being used through OVS so no GRO coming-up the host
> stack. The compute nodes themselves were running a 3.14-based kernel.
>
> Single-stream netperf, CPU utilizations and thus service demands are
> based on intra-guest reported CPU.
...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Rick Jones <rick.jones2@hp.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: add gro capability
From: Rick Jones @ 2015-08-03 16:22 UTC (permalink / raw)
To: Michael S. Tsirkin, Eric Dumazet; +Cc: netdev, David Miller, virtualization
In-Reply-To: <20150803162529-mutt-send-email-mst@redhat.com>
On 08/03/2015 06:37 AM, Michael S. Tsirkin wrote:
> Ideally this needs to also be tested on non-vxlan configs with gro in
> host, to make sure this doesn't cause regressions.
Measured with the same instances on the same hardware and software,
taking a path through the stack (public rather than private IPs, with
Distributed Virtual Router (DVR) enabled) which gives them GRO:
Throughput
Min Median Average Max
4.2.0-rc3+_hostGRO 6713 8351 8232 9102
4.2.0-rc3+flush1k_hostGRO 6539 8267 8206 8982
As singletons, Mins and Maxes probably have rather high variability, I'd
focus on the Median and Average and those are within 1%.
Send Service Demand
Min Median Average Max
4.2.0-rc3+_hostGRO 0.332 0.496 0.490 0.651
4.2.0-rc3+flush1k_hostGRO 0.328 0.493 0.488 0.678
Receive Service Demand
Min Median Average Max
4.2.0-rc3+_hostGRO 0.386 0.469 0.485 0.677
4.2.0-rc3+flush1k_hostGRO 0.369 0.466 0.477 0.665
happy benchmarking,
rick
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: add gro capability
From: Michael S. Tsirkin @ 2015-08-03 13:37 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Rick Jones, David Miller, virtualization
In-Reply-To: <1438359917.2748.18.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, Jul 31, 2015 at 06:25:17PM +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Straightforward patch to add GRO processing to virtio_net.
>
> napi_complete_done() usage allows more aggressive aggregation,
> opted-in by setting /sys/class/net/xxx/gro_flush_timeout
>
> Tested:
>
> Setting /sys/class/net/xxx/gro_flush_timeout to 1000 nsec,
> Rick Jones reported following results.
>
> One VM of each on a pair of OpenStack compute nodes with E5-2650Lv3 CPUs
> and Intel 82599ES-based NICs. So, two "before" and two "after" VMs.
> The OpenStack compute nodes were running OpenStack Kilo, with VxLAN
> encapsulation being used through OVS so no GRO coming-up the host
> stack. The compute nodes themselves were running a 3.14-based kernel.
>
> Single-stream netperf, CPU utilizations and thus service demands are
> based on intra-guest reported CPU.
>
> Throughput Mbit/s, bigger is better
> Min Median Average Max
> 4.2.0-rc3+ 1364 1686 1678 1938
> 4.2.0-rc3+flush1k 1824 2269 2275 2647
>
> Send Service Demand, smaller is better
> Min Median Average Max
> 4.2.0-rc3+ 0.236 0.558 0.524 0.802
> 4.2.0-rc3+flush1k 0.176 0.503 0.471 0.738
>
> Receive Service Demand, smaller is better.
> Min Median Average Max
> 4.2.0-rc3+ 1.906 2.188 2.191 2.531
> 4.2.0-rc3+flush1k 0.448 0.529 0.533 0.692
>
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Rick Jones <rick.jones2@hp.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
Ideally this needs to also be tested on non-vxlan configs with gro in
host, to make sure this doesn't cause regressions.
But I don't see why it should: GRO overhead is pretty small if packets
don't need to be combined.
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7fbca37a1adf..66f08f622dc6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -518,7 +518,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>
> skb_mark_napi_id(skb, &rq->napi);
>
> - netif_receive_skb(skb);
> + napi_gro_receive(&rq->napi, skb);
> return;
>
> frame_err:
> @@ -756,7 +756,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
> /* Out of packets? */
> if (received < budget) {
> r = virtqueue_enable_cb_prepare(rq->vq);
> - napi_complete(napi);
> + napi_complete_done(napi, received);
> if (unlikely(virtqueue_poll(rq->vq, r)) &&
> napi_schedule_prep(napi)) {
> virtqueue_disable_cb(rq->vq);
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v2] arm: change vendor ID for virtio-mmio
From: Andrew Jones @ 2015-08-03 12:09 UTC (permalink / raw)
To: Peter Maydell
Cc: Graeme Gregory, Michael S. Tsirkin, QEMU Developers,
lkml - Kernel Mailing List,
virtualization@lists.linux-foundation.org, Shannon Zhao,
Igor Mammedov
In-Reply-To: <CAFEAcA-dr_QBnM+h-4EQ9uZMRwVVi=UUazBLWmDWxLH+35ZKLQ@mail.gmail.com>
On Fri, Jul 31, 2015 at 01:37:44PM +0100, Peter Maydell wrote:
> On 29 July 2015 at 20:16, Michael S. Tsirkin <mst@redhat.com> wrote:
> > ACPI spec 5.0 allows the use of PCI vendor IDs.
> >
> > Since we have one for virtio, it seems neater to use that
> > rather than LNRO. For the device ID, use 103F which is a legacy ID that
> > isn't used in virtio PCI spec - seems to make sense since virtio-mmio is
> > a legacy device but we don't know the correct device type.
> >
> > Guests should probably match everything in the range 1000-103F
> > (just like legacy pci drivers do) which will allow us to pass in the
> > actual ID in the future if we want to.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > hw/arm/virt-acpi-build.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index f365140..dea61ba 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -145,7 +145,7 @@ static void acpi_dsdt_add_virtio(Aml *scope,
> >
> > for (i = 0; i < num; i++) {
> > Aml *dev = aml_device("VR%02u", i);
> > - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> > + aml_append(dev, aml_name_decl("_HID", aml_string("1AF4103F")));
> > aml_append(dev, aml_name_decl("_UID", aml_int(i)));
>
> So, I've just checked, and I believe that the kernel that RedHat
> are shipping in their RHEL7 dev preview for AArch64 (and probably
> thus also the Fedora/Centos one) includes a patch which adds
> ACPI support to the virtio-mmio driver using the LNRO0005 ID string.
Yes, we have https://lkml.org/lkml/2015/7/28/216 in the RHELSA kernel.
Although, while that kernel is floating around already, it hasn't
really been released, particularly not for virt use cases. So we could
change it (although that change needs to be soon).
>
> This to me suggests that we should just stick with that ID,
> rather than changing to QEMUxxxx, the hex one based on the PCI
> vendor ID, or anything else.
>
> We're obviously under no obligation to make life easy for people
> who ship kernels full of patches that haven't gone upstream yet,
> but in this case there doesn't seem to me to be any benefit to
> QEMU from picking an ID string that would break compatibility...
I'll remain abstained on this debate, but if the consensus is to
change it, then let me know, and I'll handle the changes on the
RHELSA side.
Thanks,
drew
>
> [The kernel I checked was the one in
> https://git.centos.org/sources/kernel-aarch64/c7-aarch64/c589ab77889df6d93dbe817c373080631ab3275b
> which despite the filename is actually an 80MB .tar.xz archive,
> as pointed to by
> https://git.centos.org/blob/rpms!kernel-aarch64/910dbce5f13419d68002f58e67ee6e762a93a425/.kernel-aarch64.metadata
> ]
>
> thanks
> -- PMM
>
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: add gro capability
From: Michael S. Tsirkin @ 2015-08-02 8:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, rick.jones2, eric.dumazet, virtualization
In-Reply-To: <20150731.165732.2136164140565390142.davem@davemloft.net>
On Fri, Jul 31, 2015 at 04:57:32PM -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 31 Jul 2015 18:25:17 +0200
>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Straightforward patch to add GRO processing to virtio_net.
> >
> ...
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Tested-by: Rick Jones <rick.jones2@hp.com>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
>
> Michael, please review :-)
Will do shortly :)
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: add gro capability
From: David Miller @ 2015-07-31 23:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, rick.jones2, virtualization, mst
In-Reply-To: <1438359917.2748.18.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 31 Jul 2015 18:25:17 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Straightforward patch to add GRO processing to virtio_net.
>
...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Rick Jones <rick.jones2@hp.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
Michael, please review :-)
^ permalink raw reply
* [PATCH net-next] virtio_net: add gro capability
From: Eric Dumazet @ 2015-07-31 16:25 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Rick Jones, virtualization, Michael S. Tsirkin
From: Eric Dumazet <edumazet@google.com>
Straightforward patch to add GRO processing to virtio_net.
napi_complete_done() usage allows more aggressive aggregation,
opted-in by setting /sys/class/net/xxx/gro_flush_timeout
Tested:
Setting /sys/class/net/xxx/gro_flush_timeout to 1000 nsec,
Rick Jones reported following results.
One VM of each on a pair of OpenStack compute nodes with E5-2650Lv3 CPUs
and Intel 82599ES-based NICs. So, two "before" and two "after" VMs.
The OpenStack compute nodes were running OpenStack Kilo, with VxLAN
encapsulation being used through OVS so no GRO coming-up the host
stack. The compute nodes themselves were running a 3.14-based kernel.
Single-stream netperf, CPU utilizations and thus service demands are
based on intra-guest reported CPU.
Throughput Mbit/s, bigger is better
Min Median Average Max
4.2.0-rc3+ 1364 1686 1678 1938
4.2.0-rc3+flush1k 1824 2269 2275 2647
Send Service Demand, smaller is better
Min Median Average Max
4.2.0-rc3+ 0.236 0.558 0.524 0.802
4.2.0-rc3+flush1k 0.176 0.503 0.471 0.738
Receive Service Demand, smaller is better.
Min Median Average Max
4.2.0-rc3+ 1.906 2.188 2.191 2.531
4.2.0-rc3+flush1k 0.448 0.529 0.533 0.692
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Rick Jones <rick.jones2@hp.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
---
drivers/net/virtio_net.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7fbca37a1adf..66f08f622dc6 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -518,7 +518,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
skb_mark_napi_id(skb, &rq->napi);
- netif_receive_skb(skb);
+ napi_gro_receive(&rq->napi, skb);
return;
frame_err:
@@ -756,7 +756,7 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
/* Out of packets? */
if (received < budget) {
r = virtqueue_enable_cb_prepare(rq->vq);
- napi_complete(napi);
+ napi_complete_done(napi, received);
if (unlikely(virtqueue_poll(rq->vq, r)) &&
napi_schedule_prep(napi)) {
virtqueue_disable_cb(rq->vq);
^ permalink raw reply related
* Re: [PATCH v2] arm: change vendor ID for virtio-mmio
From: Peter Maydell @ 2015-07-31 12:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Graeme Gregory, QEMU Developers, lkml - Kernel Mailing List,
virtualization@lists.linux-foundation.org, Shannon Zhao,
Igor Mammedov
In-Reply-To: <1438196676-30255-1-git-send-email-mst@redhat.com>
On 29 July 2015 at 20:16, Michael S. Tsirkin <mst@redhat.com> wrote:
> ACPI spec 5.0 allows the use of PCI vendor IDs.
>
> Since we have one for virtio, it seems neater to use that
> rather than LNRO. For the device ID, use 103F which is a legacy ID that
> isn't used in virtio PCI spec - seems to make sense since virtio-mmio is
> a legacy device but we don't know the correct device type.
>
> Guests should probably match everything in the range 1000-103F
> (just like legacy pci drivers do) which will allow us to pass in the
> actual ID in the future if we want to.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/arm/virt-acpi-build.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f365140..dea61ba 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -145,7 +145,7 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>
> for (i = 0; i < num; i++) {
> Aml *dev = aml_device("VR%02u", i);
> - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005")));
> + aml_append(dev, aml_name_decl("_HID", aml_string("1AF4103F")));
> aml_append(dev, aml_name_decl("_UID", aml_int(i)));
So, I've just checked, and I believe that the kernel that RedHat
are shipping in their RHEL7 dev preview for AArch64 (and probably
thus also the Fedora/Centos one) includes a patch which adds
ACPI support to the virtio-mmio driver using the LNRO0005 ID string.
This to me suggests that we should just stick with that ID,
rather than changing to QEMUxxxx, the hex one based on the PCI
vendor ID, or anything else.
We're obviously under no obligation to make life easy for people
who ship kernels full of patches that haven't gone upstream yet,
but in this case there doesn't seem to me to be any benefit to
QEMU from picking an ID string that would break compatibility...
[The kernel I checked was the one in
https://git.centos.org/sources/kernel-aarch64/c7-aarch64/c589ab77889df6d93dbe817c373080631ab3275b
which despite the filename is actually an 80MB .tar.xz archive,
as pointed to by
https://git.centos.org/blob/rpms!kernel-aarch64/910dbce5f13419d68002f58e67ee6e762a93a425/.kernel-aarch64.metadata
]
thanks
-- PMM
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox