* Re: [PATCH] mm: don't zero ballooned pages
From: Michal Hocko @ 2017-07-31 6:55 UTC (permalink / raw)
To: Wei Wang
Cc: mst, mawilcox, dave.hansen, linux-kernel, virtualization,
linux-mm, akpm, zhenwei.pi
In-Reply-To: <1501474413-21580-1-git-send-email-wei.w.wang@intel.com>
On Mon 31-07-17 12:13:33, Wei Wang wrote:
> Ballooned pages will be marked as MADV_DONTNEED by the hypervisor and
> shouldn't be given to the host ksmd to scan.
Could you point me where this MADV_DONTNEED is done, please?
> Therefore, it is not
> necessary to zero ballooned pages, which is very time consuming when
> the page amount is large. The ongoing fast balloon tests show that the
> time to balloon 7G pages is increased from ~491ms to 2.8 seconds with
> __GFP_ZERO added. So, this patch removes the flag.
Please make it obvious that this is a revert of bb01b64cfab7
("mm/balloon_compaction.c: enqueue zero page to balloon device").
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
> mm/balloon_compaction.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index 9075aa5..b06d9fe 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -24,7 +24,7 @@ struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info)
> {
> unsigned long flags;
> struct page *page = alloc_page(balloon_mapping_gfp_mask() |
> - __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_ZERO);
> + __GFP_NOMEMALLOC | __GFP_NORETRY);
> if (!page)
> return NULL;
>
> --
> 2.7.4
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH] mm: don't zero ballooned pages
From: Wei Wang @ 2017-07-31 4:13 UTC (permalink / raw)
To: linux-kernel, linux-mm, virtualization, mst, mawilcox, mhocko,
dave.hansen, akpm, wei.w.wang
Cc: zhenwei.pi
Ballooned pages will be marked as MADV_DONTNEED by the hypervisor and
shouldn't be given to the host ksmd to scan. Therefore, it is not
necessary to zero ballooned pages, which is very time consuming when
the page amount is large. The ongoing fast balloon tests show that the
time to balloon 7G pages is increased from ~491ms to 2.8 seconds with
__GFP_ZERO added. So, this patch removes the flag.
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
mm/balloon_compaction.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index 9075aa5..b06d9fe 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -24,7 +24,7 @@ struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info)
{
unsigned long flags;
struct page *page = alloc_page(balloon_mapping_gfp_mask() |
- __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_ZERO);
+ __GFP_NOMEMALLOC | __GFP_NORETRY);
if (!page)
return NULL;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Michael S. Tsirkin @ 2017-07-30 16:20 UTC (permalink / raw)
To: Wang, Wei W
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
yang.zhang.wz@gmail.com, quan.xu@aliyun.com,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <20170730191735-mutt-send-email-mst@kernel.org>
On Sun, Jul 30, 2017 at 07:18:33PM +0300, Michael S. Tsirkin wrote:
> On Sun, Jul 30, 2017 at 05:59:17AM +0000, Wang, Wei W wrote:
> > On Sunday, July 30, 2017 12:23 PM, Michael S. Tsirkin wrote:
> > > On Sat, Jul 29, 2017 at 08:47:08PM +0800, Wei Wang wrote:
> > > > On 07/29/2017 07:08 AM, Michael S. Tsirkin wrote:
> > > > > On Thu, Jul 27, 2017 at 10:50:11AM +0800, Wei Wang wrote:
> > > > > > > > > OK I thought this over. While we might need these new APIs
> > > > > > > > > in the future, I think that at the moment, there's a way to
> > > > > > > > > implement this feature that is significantly simpler. Just
> > > > > > > > > add each s/g as a separate input buffer.
> > > > > > > > Should it be an output buffer?
> > > > > > > Hypervisor overwrites these pages with zeroes. Therefore it is
> > > > > > > writeable by device: DMA_FROM_DEVICE.
> > > > > > Why would the hypervisor need to zero the buffer?
> > > > > The page is supplied to hypervisor and can lose the value that is
> > > > > there. That is the definition of writeable by device.
> > > >
> > > > I think for the free pages, it should be clear that they will be added
> > > > as output buffer to the device, because (as we discussed) they are
> > > > just hints, and some of them may be used by the guest after the report_ API is
> > > invoked.
> > > > The device/hypervisor should not use or discard them.
> > >
> > > Discarding contents is exactly what you propose doing if migration is going on,
> > > isn't it?
> >
> > That's actually a different concept. Please let me explain it with this example:
> >
> > The hypervisor receives the hint saying the guest PageX is a free page, but as we know,
> > after that report_ API exits, the guest kernel may take PageX to use, so PageX is not free
> > page any more. At this time, if the hypervisor writes to the page, that would crash the guest.
> > So, I think the cornerstone of this work is that the hypervisor should not touch the
> > reported pages.
> >
> > Best,
> > Wei
>
> That's a hypervisor implementation detail. From guest point of view,
> discarding contents can not be distinguished from writing old contents.
>
Besides, ignoring the free page tricks, consider regular ballooning.
We map page with DONTNEED then back with WILLNEED. Result is
getting a zero page. So at least one of deflate/inflate should be input.
I'd say both for symmetry.
--
MST
^ permalink raw reply
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Michael S. Tsirkin @ 2017-07-30 16:18 UTC (permalink / raw)
To: Wang, Wei W
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
yang.zhang.wz@gmail.com, quan.xu@aliyun.com,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <286AC319A985734F985F78AFA26841F739288D85@shsmsx102.ccr.corp.intel.com>
On Sun, Jul 30, 2017 at 05:59:17AM +0000, Wang, Wei W wrote:
> On Sunday, July 30, 2017 12:23 PM, Michael S. Tsirkin wrote:
> > On Sat, Jul 29, 2017 at 08:47:08PM +0800, Wei Wang wrote:
> > > On 07/29/2017 07:08 AM, Michael S. Tsirkin wrote:
> > > > On Thu, Jul 27, 2017 at 10:50:11AM +0800, Wei Wang wrote:
> > > > > > > > OK I thought this over. While we might need these new APIs
> > > > > > > > in the future, I think that at the moment, there's a way to
> > > > > > > > implement this feature that is significantly simpler. Just
> > > > > > > > add each s/g as a separate input buffer.
> > > > > > > Should it be an output buffer?
> > > > > > Hypervisor overwrites these pages with zeroes. Therefore it is
> > > > > > writeable by device: DMA_FROM_DEVICE.
> > > > > Why would the hypervisor need to zero the buffer?
> > > > The page is supplied to hypervisor and can lose the value that is
> > > > there. That is the definition of writeable by device.
> > >
> > > I think for the free pages, it should be clear that they will be added
> > > as output buffer to the device, because (as we discussed) they are
> > > just hints, and some of them may be used by the guest after the report_ API is
> > invoked.
> > > The device/hypervisor should not use or discard them.
> >
> > Discarding contents is exactly what you propose doing if migration is going on,
> > isn't it?
>
> That's actually a different concept. Please let me explain it with this example:
>
> The hypervisor receives the hint saying the guest PageX is a free page, but as we know,
> after that report_ API exits, the guest kernel may take PageX to use, so PageX is not free
> page any more. At this time, if the hypervisor writes to the page, that would crash the guest.
> So, I think the cornerstone of this work is that the hypervisor should not touch the
> reported pages.
>
> Best,
> Wei
That's a hypervisor implementation detail. From guest point of view,
discarding contents can not be distinguished from writing old contents.
^ permalink raw reply
* Re: [PATCH net] Revert "vhost: cache used event for better performance"
From: K. Den @ 2017-07-30 6:26 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20170726190745-mutt-send-email-mst@kernel.org>
On Wed, 2017-07-26 at 19:08 +0300, Michael S. Tsirkin wrote:
> On Wed, Jul 26, 2017 at 09:37:15PM +0800, Jason Wang wrote:
> >
> >
> > On 2017年07月26日 21:18, Jason Wang wrote:
> > >
> > >
> > > On 2017年07月26日 20:57, Michael S. Tsirkin wrote:
> > > > On Wed, Jul 26, 2017 at 04:03:17PM +0800, Jason Wang wrote:
> > > > > This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92. Since it
> > > > > was reported to break vhost_net. We want to cache used event and use
> > > > > it to check for notification. We try to valid cached used event by
> > > > > checking whether or not it was ahead of new, but this is not correct
> > > > > all the time, it could be stale and there's no way to know about this.
> > > > >
> > > > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > >
> > > > Could you supply a bit more data here please? How does it get stale?
> > > > What does guest need to do to make it stale? This will be helpful if
> > > > anyone wants to bring it back, or if we want to extend the protocol.
> > > >
> > >
> > > The problem we don't know whether or not guest has published a new used
> > > event. The check vring_need_event(vq->last_used_event, new + vq->num,
> > > new) is not sufficient to check for this.
> > >
> > > Thanks
> >
> > More notes, the previous assumption is that we don't move used event back,
> > but this could happen in fact if idx is wrapper around.
>
> You mean if the 16 bit index wraps around after 64K entries.
> Makes sense.
>
> > Will repost and add
> > this into commit log.
> >
> > Thanks
Hi,
I am just curious but I have got a question:
AFAIU, if you wanted to keep the caching mechanism alive in the code base,
the following two changes could clear off the issue, or not?:
(1) Always fetch the latest event value from guest when signalled_used event is
invalid, which includes last_used_idx wraps-around case. Otherwise we might need
changes which would complicate too much the logic to properly decide whether or
not to skip signalling in the next vhost_notify round.
(2) On top of that, split the signal-postponing logic to three cases like:
* if the interval of vq.num is [2^16, UINT_MAX]:
any cached event is in should-postpone-signalling interval, so paradoxically
must always do signalling.
* else if the interval of vq.num is [2^15, 2^16):
the logic in the original patch (809ecb9bca6a9) suffices
* else (= less than 2^15) (optional):
checking only (vring_need_event(vq->last_used_event, new + vq->num, new)
would suffice.
Am I missing something, or is this irrelevant?
I would appreciate if you could elaborate a bit more how the situation where
event idx wraps around and moves back would make trouble.
Thanks.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* RE: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wang, Wei W @ 2017-07-30 5:59 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange@redhat.com, virtio-dev@lists.oasis-open.org,
kvm@vger.kernel.org, qemu-devel@nongnu.org, amit.shah@redhat.com,
liliang.opensource@gmail.com, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
yang.zhang.wz@gmail.com, quan.xu@aliyun.com,
cornelia.huck@de.ibm.com, pbonzini@redhat.com,
akpm@linux-foundation.org, mgorman@techsingularity.net
In-Reply-To: <20170730043922-mutt-send-email-mst@kernel.org>
On Sunday, July 30, 2017 12:23 PM, Michael S. Tsirkin wrote:
> On Sat, Jul 29, 2017 at 08:47:08PM +0800, Wei Wang wrote:
> > On 07/29/2017 07:08 AM, Michael S. Tsirkin wrote:
> > > On Thu, Jul 27, 2017 at 10:50:11AM +0800, Wei Wang wrote:
> > > > > > > OK I thought this over. While we might need these new APIs
> > > > > > > in the future, I think that at the moment, there's a way to
> > > > > > > implement this feature that is significantly simpler. Just
> > > > > > > add each s/g as a separate input buffer.
> > > > > > Should it be an output buffer?
> > > > > Hypervisor overwrites these pages with zeroes. Therefore it is
> > > > > writeable by device: DMA_FROM_DEVICE.
> > > > Why would the hypervisor need to zero the buffer?
> > > The page is supplied to hypervisor and can lose the value that is
> > > there. That is the definition of writeable by device.
> >
> > I think for the free pages, it should be clear that they will be added
> > as output buffer to the device, because (as we discussed) they are
> > just hints, and some of them may be used by the guest after the report_ API is
> invoked.
> > The device/hypervisor should not use or discard them.
>
> Discarding contents is exactly what you propose doing if migration is going on,
> isn't it?
That's actually a different concept. Please let me explain it with this example:
The hypervisor receives the hint saying the guest PageX is a free page, but as we know,
after that report_ API exits, the guest kernel may take PageX to use, so PageX is not free
page any more. At this time, if the hypervisor writes to the page, that would crash the guest.
So, I think the cornerstone of this work is that the hypervisor should not touch the
reported pages.
Best,
Wei
^ permalink raw reply
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Michael S. Tsirkin @ 2017-07-30 4:22 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, linux-kernel, virtualization, linux-mm,
yang.zhang.wz, quan.xu, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <597C83CC.7060702@intel.com>
On Sat, Jul 29, 2017 at 08:47:08PM +0800, Wei Wang wrote:
> On 07/29/2017 07:08 AM, Michael S. Tsirkin wrote:
> > On Thu, Jul 27, 2017 at 10:50:11AM +0800, Wei Wang wrote:
> > > > > > OK I thought this over. While we might need these new APIs in
> > > > > > the future, I think that at the moment, there's a way to implement
> > > > > > this feature that is significantly simpler. Just add each s/g
> > > > > > as a separate input buffer.
> > > > > Should it be an output buffer?
> > > > Hypervisor overwrites these pages with zeroes. Therefore it is
> > > > writeable by device: DMA_FROM_DEVICE.
> > > Why would the hypervisor need to zero the buffer?
> > The page is supplied to hypervisor and can lose the value that
> > is there. That is the definition of writeable by device.
>
> I think for the free pages, it should be clear that they will be added as
> output buffer to the device, because (as we discussed) they are just hints,
> and some of them may be used by the guest after the report_ API is invoked.
> The device/hypervisor should not use or discard them.
Discarding contents is exactly what you propose doing if
migration is going on, isn't it?
> For the balloon pages, I kind of agree with the existing implementation
> (e.g. inside tell_host()), which uses virtqueue_add_outbuf (instead of
> _add_inbuf()).
This is because it does not pass SGs, it passes weirdly
formatted PA within the buffer.
> I think inbuf should be a buffer which will be written by the device and
> read by the
> driver.
If hypervisor can change it, it's an inbuf. Should not matter
whether driver reads it.
> The cmd buffer put on the vq for the device to send commands can be
> an
> inbuf, I think.
>
>
> >
> > > I think it may only
> > > need to read out the info(base,size).
> > And then do what?
>
>
> For the free pages, the info will be used to clear the corresponding "1" in
> the dirty bitmap.
> For balloon pages, they will be made DONTNEED and given to other host
> processes to
> use (the device won't write them, so no need to set "write" when
> virtqueue_map_desc() in
> the device).
>
>
> >
> > > I think it should be like this:
> > > the cmd hdr buffer: input, because the hypervisor would write it to
> > > send a cmd to the guest
> > > the payload buffer: output, for the hypervisor to read the info
> > These should be split.
> >
> > We have:
> >
> > 1. request that hypervisor sends to guest, includes ID: input
> > 2. synchronisation header with ID sent by guest: output
> > 3. list of pages: input
> >
> > 2 and 3 must be on the same VQ. 1 can come on any VQ - reusing stats VQ
> > might make sense.
>
> I would prefer to make the above item 1 come on the the free page vq,
> because the existing stat_vq doesn't support the cmd hdr.
> Now, I think it is also not necessary to move the existing stat_vq
> implementation to
> a new implementation under the cmd hdr, because
> that new support doesn't make a difference for stats, it will still use its
> stat_vq (the free
> page vq will be used for reporting free pages only)
>
> What do you think?
>
>
> Best,
> Wei
^ permalink raw reply
* Re: [PATCH V2 net] Revert "vhost: cache used event for better performance"
From: David Miller @ 2017-07-29 21:16 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
In-Reply-To: <1501125725-24529-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Thu, 27 Jul 2017 11:22:05 +0800
> This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92. Since it
> was reported to break vhost_net. We want to cache used event and use
> it to check for notification. The assumption was that guest won't move
> the event idx back, but this could happen in fact when 16 bit index
> wraps around after 64K entries.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2017-07-29 12:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, linux-kernel, virtualization, linux-mm,
yang.zhang.wz, quan.xu, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <20170729020231-mutt-send-email-mst@kernel.org>
On 07/29/2017 07:08 AM, Michael S. Tsirkin wrote:
> On Thu, Jul 27, 2017 at 10:50:11AM +0800, Wei Wang wrote:
>>>>> OK I thought this over. While we might need these new APIs in
>>>>> the future, I think that at the moment, there's a way to implement
>>>>> this feature that is significantly simpler. Just add each s/g
>>>>> as a separate input buffer.
>>>> Should it be an output buffer?
>>> Hypervisor overwrites these pages with zeroes. Therefore it is
>>> writeable by device: DMA_FROM_DEVICE.
>> Why would the hypervisor need to zero the buffer?
> The page is supplied to hypervisor and can lose the value that
> is there. That is the definition of writeable by device.
I think for the free pages, it should be clear that they will be added as
output buffer to the device, because (as we discussed) they are just hints,
and some of them may be used by the guest after the report_ API is invoked.
The device/hypervisor should not use or discard them.
For the balloon pages, I kind of agree with the existing implementation
(e.g. inside tell_host()), which uses virtqueue_add_outbuf (instead of
_add_inbuf()).
I think inbuf should be a buffer which will be written by the device and
read by the
driver. The cmd buffer put on the vq for the device to send commands can
be an
inbuf, I think.
>
>> I think it may only
>> need to read out the info(base,size).
> And then do what?
For the free pages, the info will be used to clear the corresponding "1"
in the dirty bitmap.
For balloon pages, they will be made DONTNEED and given to other host
processes to
use (the device won't write them, so no need to set "write" when
virtqueue_map_desc() in
the device).
>
>> I think it should be like this:
>> the cmd hdr buffer: input, because the hypervisor would write it to
>> send a cmd to the guest
>> the payload buffer: output, for the hypervisor to read the info
> These should be split.
>
> We have:
>
> 1. request that hypervisor sends to guest, includes ID: input
> 2. synchronisation header with ID sent by guest: output
> 3. list of pages: input
>
> 2 and 3 must be on the same VQ. 1 can come on any VQ - reusing stats VQ
> might make sense.
I would prefer to make the above item 1 come on the the free page vq,
because the existing stat_vq doesn't support the cmd hdr.
Now, I think it is also not necessary to move the existing stat_vq
implementation to
a new implementation under the cmd hdr, because
that new support doesn't make a difference for stats, it will still use
its stat_vq (the free
page vq will be used for reporting free pages only)
What do you think?
Best,
Wei
^ permalink raw reply
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Michael S. Tsirkin @ 2017-07-28 23:08 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, linux-kernel, virtualization, linux-mm,
yang.zhang.wz, quan.xu, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <597954E3.2070801@intel.com>
On Thu, Jul 27, 2017 at 10:50:11AM +0800, Wei Wang wrote:
> > > > OK I thought this over. While we might need these new APIs in
> > > > the future, I think that at the moment, there's a way to implement
> > > > this feature that is significantly simpler. Just add each s/g
> > > > as a separate input buffer.
> > >
> > > Should it be an output buffer?
> > Hypervisor overwrites these pages with zeroes. Therefore it is
> > writeable by device: DMA_FROM_DEVICE.
>
> Why would the hypervisor need to zero the buffer?
The page is supplied to hypervisor and can lose the value that
is there. That is the definition of writeable by device.
> I think it may only
> need to read out the info(base,size).
And then do what?
> I think it should be like this:
> the cmd hdr buffer: input, because the hypervisor would write it to
> send a cmd to the guest
> the payload buffer: output, for the hypervisor to read the info
These should be split.
We have:
1. request that hypervisor sends to guest, includes ID: input
2. synchronisation header with ID sent by guest: output
3. list of pages: input
2 and 3 must be on the same VQ. 1 can come on any VQ - reusing stats VQ
might make sense.
> > > I think output means from the
> > > driver to device (i.e. DMA_TO_DEVICE).
> > This part is correct I believe.
> >
> > > > This needs zero new APIs.
> > > >
> > > > I know that follow-up patches need to add a header in front
> > > > so you might be thinking: how am I going to add this
> > > > header? The answer is quite simple - add it as a separate
> > > > out header.
> > > >
> > > > Host will be able to distinguish between header and pages
> > > > by looking at the direction, and - should we want to add
> > > > IN data to header - additionally size (<4K => header).
> > >
> > > I think this works fine when the cmdq is only used for
> > > reporting the unused pages.
> > > It would be an issue
> > > if there are other usages (e.g. report memory statistics)
> > > interleaving. I think one solution would be to lock the cmdq until
> > > a cmd usage is done ((e.g. all the unused pages are reported) ) -
> > > in this case, the periodically updated guest memory statistics
> > > may be delayed for a while occasionally when live migration starts.
> > > Would this be acceptable? If not, probably we can have the cmdq
> > > for one usage only.
> > >
> > >
> > > Best,
> > > Wei
> > OK I see, I think the issue is that reporting free pages
> > was structured like stats. Let's split it -
> > send pages on e.g. free_vq, get commands on vq shared with
> > stats.
> >
>
> Would it be better to have the "report free page" command to be sent
> through the free_vq? In this case,we will have
> stats_vq: for the stats usage, which is already there
> free_vq: for reporting free pages.
>
> Best,
> Wei
See above. I would get requests on stats vq but report
free pages separately on free vq.
>
>
>
>
^ permalink raw reply
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Michael S. Tsirkin @ 2017-07-28 23:01 UTC (permalink / raw)
To: Wei Wang
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, linux-kernel, willy, virtualization, linux-mm,
yang.zhang.wz, quan.xu, cornelia.huck, pbonzini, akpm, mhocko,
mgorman
In-Reply-To: <597AF4EF.4020705@intel.com>
On Fri, Jul 28, 2017 at 04:25:19PM +0800, Wei Wang wrote:
> On 07/12/2017 08:40 PM, Wei Wang wrote:
> > Add a new feature, VIRTIO_BALLOON_F_SG, which enables to
> > transfer a chunk of ballooned (i.e. inflated/deflated) pages using
> > scatter-gather lists to the host.
> >
> > The implementation of the previous virtio-balloon is not very
> > efficient, because the balloon pages are transferred to the
> > host one by one. Here is the breakdown of the time in percentage
> > spent on each step of the balloon inflating process (inflating
> > 7GB of an 8GB idle guest).
> >
> > 1) allocating pages (6.5%)
> > 2) sending PFNs to host (68.3%)
> > 3) address translation (6.1%)
> > 4) madvise (19%)
> >
> > It takes about 4126ms for the inflating process to complete.
> > The above profiling shows that the bottlenecks are stage 2)
> > and stage 4).
> >
> > This patch optimizes step 2) by transferring pages to the host in
> > sgs. An sg describes a chunk of guest physically continuous pages.
> > With this mechanism, step 4) can also be optimized by doing address
> > translation and madvise() in chunks rather than page by page.
> >
> > With this new feature, the above ballooning process takes ~491ms
> > resulting in an improvement of ~88%.
> >
>
>
> I found a recent mm patch, bb01b64cfab7c22f3848cb73dc0c2b46b8d38499
> , zeros all the ballooned pages, which is very time consuming.
>
> Tests show that the time to balloon 7G pages is increased from ~491 ms to
> 2.8 seconds with the above patch.
Sounds like it should be reverted. Post a revert pls and
we'll discuss.
> How about moving the zero operation to the hypervisor? In this way, we
> will have a much faster balloon process.
>
>
> Best,
> Wei
Or in other words hypervisors should not be stupid and
should not try to run ksm on DONTNEED pages.
--
MST
^ permalink raw reply
* Re: [RFC] virtio-mem: paravirtualized memory
From: David Hildenbrand @ 2017-07-28 15:48 UTC (permalink / raw)
To: Dan Williams
Cc: Andrea Arcangeli, Pankaj Gupta, Rik van Riel, KVM,
Michael S. Tsirkin, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org
In-Reply-To: <CAPcyv4iYdEAv7wqun5L1C-gT7fMDpO+M918or-bg5XiWLnM3=w@mail.gmail.com>
On 28.07.2017 17:16, Dan Williams wrote:
> On Fri, Jul 28, 2017 at 4:09 AM, David Hildenbrand <david@redhat.com> wrote:
>> Btw, I am thinking about the following addition to the concept:
>>
>> 1. Add a type to each virtio-mem device.
>>
>> This describes the type of the memory region we expose to the guest.
>> Initially, we could have RAM and RAM_HUGE. The latter one would be
>> interesting, because the guest would know that this memory is based on
>> huge pages in case we would ever want to expose different RAM types to a
>> guest (the guest could conclude that this memory might be faster and
>> would also best be used with huge pages in the guest). But we could also
>> simply start only with RAM.
>
> I think it's up to the hypervisor to manage whether the guest is
> getting huge pages or not and the guest need not know. As for
> communicating differentiated memory media performance we have the ACPI
> HMAT (Heterogeneous Memory Attributes Table) for that.
Yes, in the world of ACPI I agree.
>
>> 2. Adding also a guest -> host command queue.
>>
>> That can be used to request/notify the host about something. As written
>> in the original proposal, for ordinary RAM this could be used to request
>> more/less memory out of the guest.
>
> I would hope that where possible we minimize paravirtualized
> interfaces and just use standardized interfaces. In the case of memory
> hotplug, ACPI already defines that interface.
I partly agree in the world of ACPI. If you just want to add/remove
memory in the form of DIMMs, yes. This already works just fine. For
other approaches in the context of virtualization (e.g. ballooners that
XEN or Hyper-V use, or also what virtio-mem tries to achieve), this is
not enough. They need a different way of memory hotplug (as e.g. XEN and
Hyper-V already have).
Especially when trying to standardize stuff in form of virtio - binding
it to a technology specific to a handful of architectures is not
desired. Until now (as far as I remember), all but 2 virtio types
(virtio-balloon and virtio-iommu) operate on their own system resources
only, not on some resources exposed/detected via different interfaces.
>
>> This might come in handy for other memory regions we just want to expose
>> to the guest via a paravirtualized interface. The resize features
>> (adding/removing memory) might not apply to these, but we can simply
>> restrict that to certain types.
>>
>> E.g. if we want to expose PMEM memory region to a guest using a
>> paravirtualized interface (or anything else that can be mapped into
>> guest memory in the form of memory regions), we could use this. The
>> guest->host control queue can be used for tasks that typically cannot be
>> done if moddeling something like this using ordinary ACPI DIMMs
>> (flushing etc).
>>
>> CCing a couple of people that just thought about something like this in
>> the concept of fake DAX.
>
> I'm not convinced that there is a use case for paravirtualized PMEM
> commands beyond this "fake-DAX" use case. Everything would seem to
> have a path through the standard ACPI platform communication
> mechanisms.
I don't know about further commands, most likely not really many more in
this scenario. I just pinged you guys to have a look when I heard the
term virtio-pmem.
In general, a paravirtualized interface (for detection of PMEM regions)
might have one big advantage: not limited to certain architectures.
With a paravirtualized interface, we can even support* fake DAX on
architectures that don't provide a "real HW" interface for it. I think
this sounds interesting.
*quite some effort will most likely be necessary for other architectures.
--
Thanks,
David
^ permalink raw reply
* Re: [RFC] virtio-mem: paravirtualized memory
From: Dan Williams @ 2017-07-28 15:16 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrea Arcangeli, Pankaj Gupta, Rik van Riel, KVM,
Michael S. Tsirkin, qemu-devel@nongnu.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org
In-Reply-To: <0a7cd2a8-45ff-11d1-ddb5-036ce36af163@redhat.com>
On Fri, Jul 28, 2017 at 4:09 AM, David Hildenbrand <david@redhat.com> wrote:
> Btw, I am thinking about the following addition to the concept:
>
> 1. Add a type to each virtio-mem device.
>
> This describes the type of the memory region we expose to the guest.
> Initially, we could have RAM and RAM_HUGE. The latter one would be
> interesting, because the guest would know that this memory is based on
> huge pages in case we would ever want to expose different RAM types to a
> guest (the guest could conclude that this memory might be faster and
> would also best be used with huge pages in the guest). But we could also
> simply start only with RAM.
I think it's up to the hypervisor to manage whether the guest is
getting huge pages or not and the guest need not know. As for
communicating differentiated memory media performance we have the ACPI
HMAT (Heterogeneous Memory Attributes Table) for that.
> 2. Adding also a guest -> host command queue.
>
> That can be used to request/notify the host about something. As written
> in the original proposal, for ordinary RAM this could be used to request
> more/less memory out of the guest.
I would hope that where possible we minimize paravirtualized
interfaces and just use standardized interfaces. In the case of memory
hotplug, ACPI already defines that interface.
> This might come in handy for other memory regions we just want to expose
> to the guest via a paravirtualized interface. The resize features
> (adding/removing memory) might not apply to these, but we can simply
> restrict that to certain types.
>
> E.g. if we want to expose PMEM memory region to a guest using a
> paravirtualized interface (or anything else that can be mapped into
> guest memory in the form of memory regions), we could use this. The
> guest->host control queue can be used for tasks that typically cannot be
> done if moddeling something like this using ordinary ACPI DIMMs
> (flushing etc).
>
> CCing a couple of people that just thought about something like this in
> the concept of fake DAX.
I'm not convinced that there is a use case for paravirtualized PMEM
commands beyond this "fake-DAX" use case. Everything would seem to
have a path through the standard ACPI platform communication
mechanisms.
^ permalink raw reply
* Re: Performance regression with virtio_net
From: Seth Forshee @ 2017-07-28 13:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, virtualization
In-Reply-To: <20170728002913-mutt-send-email-mst@kernel.org>
On Fri, Jul 28, 2017 at 12:30:54AM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 27, 2017 at 04:14:30PM -0500, Seth Forshee wrote:
> > On Thu, Jul 27, 2017 at 11:38:52PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Jul 27, 2017 at 12:09:42PM -0500, Seth Forshee wrote:
> > > > I'm seeing a performance regression with virtio_net that looks to have
> > > > started in 4.12-rc1. I only see it in one context though, downloading
> > > > snap packages from the Ubuntu snap store. For example:
> > > >
> > > > https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap
> > > >
> > > > which redirects to Internap's CDN. Normally this downloads in a few
> > > > seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
> > > > of ~150 KB/s. Everything else I've tried downloads as normal speeds.
> > >
> > > So just wget that URL should be enough?
> >
> > Yes. Note that sometimes it starts out faster then slows down.
> > > > I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
> > > > handling". If I revert this on top of 4.13-rc2 (along with other changes
> > > > needed to successfully revert it) speeds return to normal.
> > > >
> > > > Thanks,
> > > > Seth
> > >
> > >
> > > Interesting. A more likely suspect would be
> > > e377fcc8486d40867c6c217077ad0fa40977e060 - could you please try
> > > reverting that one instead?
> >
> > I tried it, and I still get slow download speeds. I did test at
> > 680557cf79f82623e2c4fd42733077d60a843513 during the bisect so I'm
> > reasonably confident that this is the one where things went bad.
> > > Also, could you please look at mergeable_rx_buffer_size in sysfs with
> > > and without the change?
> >
> > In all cases (stock 4.13-rc2, 680557cf79f8 reverted, and e377fcc8486d
> > reverted) mergeable_rx_buffer_size was 1536.
> >
> > Thanks,
> > Seth
>
> Do you see any error counters incrementing after it slows down?
I see rx_dropped and rx_length_errors increasing in lockstep once it
slows down.
Thanks,
Seth
^ permalink raw reply
* CFP SENSORNETS 2018 - 6th Int.l Conf. on Sensor Networks (Funchal, Madeira/Portugal)
From: sensornets @ 2017-07-28 11:25 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
6th International Conference on Sensor Networks
Submission Deadline: July 31, 2017
http://www.sensornets.org/
January 22 - 24, 2018
Funchal, Madeira, Portugal.
SENSORNETS is organized in 5 major tracks:
- Sensor Networks Software, Architectures and Applications
- Wireless Sensor Networks
- Energy and Environment
- Intelligent Data Analysis and Processing
- Security and Privacy in Sensor Networks
In Cooperation with IFSA.
With the presence of internationally distinguished keynote speakers:
Kort Bremer, Leibniz Universität Hannover, Germany
Leonhard Reindl, University of Freiburg, Germany
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don’t hesitate contacting me.
Kind regards,
SENSORNETS Secretariat
Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://www.sensornets.org/
e-mail: sensornets.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [RFC] virtio-mem: paravirtualized memory
From: David Hildenbrand @ 2017-07-28 11:09 UTC (permalink / raw)
To: KVM, virtualization@lists.linux-foundation.org,
qemu-devel@nongnu.org, linux-mm@kvack.org
Cc: Andrea Arcangeli, Pankaj Gupta, Rik van Riel, Dan Williams,
Michael S. Tsirkin
In-Reply-To: <547865a9-d6c2-7140-47e2-5af01e7d761d@redhat.com>
Btw, I am thinking about the following addition to the concept:
1. Add a type to each virtio-mem device.
This describes the type of the memory region we expose to the guest.
Initially, we could have RAM and RAM_HUGE. The latter one would be
interesting, because the guest would know that this memory is based on
huge pages in case we would ever want to expose different RAM types to a
guest (the guest could conclude that this memory might be faster and
would also best be used with huge pages in the guest). But we could also
simply start only with RAM.
2. Adding also a guest -> host command queue.
That can be used to request/notify the host about something. As written
in the original proposal, for ordinary RAM this could be used to request
more/less memory out of the guest.
This might come in handy for other memory regions we just want to expose
to the guest via a paravirtualized interface. The resize features
(adding/removing memory) might not apply to these, but we can simply
restrict that to certain types.
E.g. if we want to expose PMEM memory region to a guest using a
paravirtualized interface (or anything else that can be mapped into
guest memory in the form of memory regions), we could use this. The
guest->host control queue can be used for tasks that typically cannot be
done if moddeling something like this using ordinary ACPI DIMMs
(flushing etc).
CCing a couple of people that just thought about something like this in
the concept of fake DAX.
On 16.06.2017 16:20, David Hildenbrand wrote:
> Hi,
>
> this is an idea that is based on Andrea Arcangeli's original idea to
> host enforce guest access to memory given up using virtio-balloon using
> userfaultfd in the hypervisor. While looking into the details, I
> realized that host-enforcing virtio-balloon would result in way too many
> problems (mainly backwards compatibility) and would also have some
> conceptual restrictions that I want to avoid. So I developed the idea of
> virtio-mem - "paravirtualized memory".
>
> The basic idea is to add memory to the guest via a paravirtualized
> mechanism (so the guest can hotplug it) and remove memory via a
> mechanism similar to a balloon. This avoids having to online memory as
> "online-movable" in the guest and allows more fain grained memory
> hot(un)plug. In addition, migrating QEMU guests after adding/removing
> memory gets a lot easier.
>
> Actually, this has a lot in common with the XEN balloon or the Hyper-V
> balloon (namely: paravirtualized hotplug and ballooning), but is very
> different when going into the details.
>
> Getting this all implemented properly will take quite some effort,
> that's why I want to get some early feedback regarding the general
> concept. If you have some alternative ideas, or ideas how to modify this
> concept, I'll be happy to discuss. Just please make sure to have a look
> at the requirements first.
>
> -----------------------------------------------------------------------
> 0. Outline:
> -----------------------------------------------------------------------
> - I. General concept
> - II. Use cases
> - III. Identified requirements
> - IV. Possible modifications
> - V. Prototype
> - VI. Problems to solve / things to sort out / missing in prototype
> - VII. Questions
> - VIII. Q&A
>
> ------------------------------------------------------------------------
> I. General concept
> ------------------------------------------------------------------------
>
> We expose memory regions to the guest via a paravirtualize interface. So
> instead of e.g. a DIMM on x86, such memory is not anounced via ACPI.
> Unmodified guests (without a virtio-mem driver) won't be able to see/use
> this memory. The virtio-mem guest driver is needed to detect and manage
> these memory areas. What makes this memory special is that it can grow
> while the guest is running ("plug memory") and might shrink on a reboot
> (to compensate "unplugged" memory - see next paragraph). Each virtio-mem
> device manages exactly one such memory area. By having multiple ones
> assigned to different NUMA nodes, we can modify memory on a NUMA basis.
>
> Of course, we cannot shrink these memory areas while the guest is
> running. To be able to unplug memory, we do something like a balloon
> does, however limited to this very memory area that belongs to the
> virtio-mem device. The guest will hand back small chunks of memory. If
> we want to add memory to the guest, we first "replug" memory that has
> previously been given up by the guest, before we grow our memory area.
>
> On a reboot, we want to avoid any memory holes in our memory, therefore
> we resize our memory area (shrink it) to compensate memory that has been
> unplugged. This highly simplifies hotplugging memory in the guest (
> hotplugging memory with random memory holes is basically impossible).
>
> We have to make sure that all memory chunks the guest hands back on
> unplug requests will not consume memory in the host. We do this by
> write-protecting that memory chunk in the host and then dropping the
> backing pages. The guest can read this memory (reading from the ZERO
> page) but no longer write to it. For now, this will only work on
> anonymous memory. We will use userfaultfd WP (write-protect mode) to
> avoid creating too many VMAs. Huge pages will require more effort (no
> explicit ZERO page).
>
> As we unplug memory on a fine grained basis (and e.g. not on
> a complete DIMM basis), there is no need to online virtio-mem memory
> as online-movable. Also, memory unplug support for Windows might be
> supported that way. You can find more details in the Q/A section below.
>
>
> The important points here are:
> - After a reboot, every memory the guest sees can be accessed and used.
> (in contrast to e.g. the XEN balloon, see Q/A fore more details)
> - Rebooting into an unmodified guest will not result into random
> crashed. The guest will simply not be able to use all memory without a
> virtio-mem driver.
> - Adding/Removing memory will not require modifying the QEMU command
> line on the migration target. Migration simply works (re-sizing memory
> areas is already part of the migration protocol!). Essentially, this
> makes adding/removing memory to/from a guest way simpler and
> independent of the underlying architecture. If the guest OS can online
> new memory, we can add more memory this way.
> - Unplugged memory can be read. This allows e.g. kexec() without nasty
> modifications. Especially relevant for Windows' kexec() variant.
> - It will play nicely with other things mapped into the address space,
> e.g. also other DIMMs or NVDIMM. virtio-mem will only work on its own
> memory region (in contrast e.g. to virtio-balloon). Especially it will
> not give up ("allocate") memory on other DIMMs, hindering them to get
> unplugged the ACPI way.
> - We can add/remove memory without running into KVM memory slot or other
> (e.g. ACPI slot) restrictions. The granularity in which we can add
> memory is only limited by the granularity the guest can add memory
> (e.g. Windows 2MB, Linux on x86 128MB for now).
> - By not having to online memory as online-movable we don't run into any
> memory restrictions in the guest. E.g. page tables can only be created
> on !movable memory. So while there might be plenty of online-movable
> memory left, allocation of page tables might fail. See Q/A for more
> details.
> - The admin will not have to set memory offline in the guest first in
> order to unplug it. virtio-mem will handle this internally and not
> require interaction with an admin or a guest-agent.
>
> Important restrictions of this concept:
> - Guests without a virtio-mem guest driver can't see that memory.
> - We will always require some boot memory that cannot get unplugged.
> Also, virtio-mem memory (as all other hotplugged memory) cannot become
> DMA memory under Linux. So the boot memory also defines the amount of
> DMA memory.
> - Hibernation/Sleep+Restore while virtio-mem is active is not supported.
> On a reboot/fresh start, the size of the virtio-mem memory area might
> change and a running/loaded guest can't deal with that.
> - Unplug support for hugetlbfs/shmem will take quite some time to
> support. The larger the used page size, the harder for the guest to
> give up memory. We can still use DIMM based hotplug for that.
> - Huge huge pages are problematic, as the guest would have to give up
> e.g. 1GB chunks. This is not expected to be supported. We can still
> use DIMM based hotplug for setups that require that.
> - For any memory we unplug using this mechanism, for now we will still
> have struct pages allocated in the guest. This means, that roughly
> 1.6% of unplugged memory will still be allocated in the guest, being
> unusable.
>
>
> ------------------------------------------------------------------------
> II. Use cases
> ------------------------------------------------------------------------
>
> Of course, we want to deny any access to unplugged memory. In contrast
> to virtio-balloon or other similar ideas (free page hinting), this is
> not about cooperative memory management, but about guarantees. The idea
> is, that both concepts can coexist.
>
> So one use case is of course cloud providers. Customers can add
> or remove memory to/from a VM without having to care about how to
> online memory or in which amount to add memory in the first place in
> order to remove it again. In cloud environments, we care about
> guarantees. E.g. for virtio-balloon a malicious guest can simply reuse
> any deflated memory, and the hypervisor can't even tell if the guest is
> malicious (e.g. a harmless guest reboot might look like a malicious
> guest). For virtio-mem, we guarantee that the guest can't reuse any
> memory that it previously gave up.
>
> But also for ordinary VMs (!cloud), this avoids having to online memory
> in the guest as online-movable and therefore not running into allocation
> problems if there are e.g. many processes needing many page tables on
> !movable memory. Also here, we don't have to know how much memory we
> want to remove some-when in the future before we add memory. (e.g. if we
> add a 128GB DIMM, we can only remove that 128GB DIMM - if we are lucky).
>
> We might be able to support memory unplug for Windows (as for now,
> ACPI unplug is not supported), more details have to be clarified.
>
> As we can grow these memory areas quite easily, another use case might
> be guests that tell us they need more memory. Thinking about VMs to
> protect containers, there seems to be the general problem that we don't
> know how much memory the container will actually need. We could
> implement a mechanism (in virtio-mem or guest driver), by which the
> guest can request more memory. If the hypervisor agrees, it can simply
> give the guest more memory. As this is all handled within QEMU,
> migration is not a problem. Adding more memory will not result in new
> DIMM devices.
>
>
> ------------------------------------------------------------------------
> III. Identified requirements
> ------------------------------------------------------------------------
>
> I considered the following requirements.
>
> NUMA aware:
> We want to be able to add/remove memory to/from NUMA nodes.
> Different page-size support:
> We want to be able to support different page sizes, e.g. because of
> huge pages in the hypervisor or because host and guest have different
> page sizes (powerpc 64k vs 4k).
> Guarantees:
> There has to be no way the guest can reuse unplugged memory without
> host consent. Still, we could implement a mechanism for the guest to
> request more memory. The hypervisor then has to decide how it wants to
> handle that request.
> Architecture independence:
> We want this to work independently of other technologies bound to
> specific architectures, like ACPI.
> Avoid online-movable:
> We don't want to have to online memory in the guest as online-movable
> just to be able to unplug (at least parts of) it again.
> Migration support:
> Be able to migrate without too much hassle. Especially, to handle it
> completely within QEMU (not having to add new devices to the target
> command line).
> Windows support:
> We definitely want to support Windows guests in the long run.
> Coexistence with other hotplug mechanisms:
> Allow to hotplug DIMMs / NVDIMMs, therefore to share the "hotplug"
> address space part with other devices.
> Backwards compatibility:
> Don't break if rebooting into an unmodified guest after having
> unplugged some memory. All memory a freshly booted guest sees must not
> contain memory holes that will crash it if it tries to access it.
>
>
> ------------------------------------------------------------------------
> IV. Possible modifications
> ------------------------------------------------------------------------
>
> Adding a guest->host request mechanism would make sense to e.g. be able
> to request further memory from the hypervisor directly from the guest.
>
> Adding memory will be much easier than removing memory. We can split
> this up and first introduce "adding memory" and later add "removing
> memory". Removing memory will require userfaultfd WP in the hypervisor
> and a special fancy allocator in the guest. So this will take some time.
>
> Adding a mechanism to trade in memory blocks might make sense to allow
> some sort of memory compaction. However I expect this to be highly
> complicated and basically not feasible.
>
> Being able to unplug memory "any" memory instead of only memory
> belonging to the virtio-mem device sounds tempting (and simplifies
> certain parts), however it has a couple of side effects I want to avoid.
> You can read more about that in the Q/A below.
>
>
> ------------------------------------------------------------------------
> V. Prototype
> ------------------------------------------------------------------------
>
> To identify potential problems I developed a very basic prototype. It
> is incomplete, full of hacks and most probably broken in various ways.
> I used it only in the given setup, only on x86 and only with an initrd.
>
> It uses a fixed page size of 256k for now, has a very ugly allocator
> hack in the guest, the virtio protocol really needs some tuning and
> an async job interface towards the user is missing. Instead of using
> userfaultfd WP, I am using simply mprotect() in this prototype. Basic
> migration works (not involving userfaultfd).
>
> Please, don't even try to review it (that's why I will also not attach
> any patches to this mail :) ), just use this as an inspiration what this
> could look like. You can find the latest hack at:
>
> QEMU: https://github.com/davidhildenbrand/qemu/tree/virtio-mem
>
> Kernel: https://github.com/davidhildenbrand/linux/tree/virtio-mem
>
> Use the kernel in the guest and make sure to compile the virtio-mem
> driver into the kernel (CONFIG_VIRTIO_MEM=y). A host kernel patch is
> contained to allow atomic resize of KVM memory regions, however it is
> pretty much untested.
>
>
> 1. Starting a guest with virtio-mem memory:
> We will create a guest with 2 NUMA nodes and 4GB of "boot + DMA"
> memory. This memory is visible also to guests without virtio-mem.
> Also, we will add 4GB to NUMA node 0 and 3GB to NUMA node 1 using
> virtio-mem. We allow both virtio-mem devices to grow up to 8GB. The
> last 4 lines are the important part.
>
> --> qemu/x86_64-softmmu/qemu-system-x86_64 \
> --enable-kvm
> -m 4G,maxmem=20G \
> -smp sockets=2,cores=2 \
> -numa node,nodeid=0,cpus=0-1 -numa node,nodeid=1,cpus=2-3 \
> -machine pc \
> -kernel linux/arch/x86_64/boot/bzImage \
> -nodefaults \
> -chardev stdio,id=serial \
> -device isa-serial,chardev=serial \
> -append "console=ttyS0 rd.shell rd.luks=0 rd.lvm=0" \
> -initrd /boot/initramfs-4.10.8-200.fc25.x86_64.img \
> -chardev socket,id=monitor,path=/var/tmp/monitor,server,nowait \
> -mon chardev=monitor,mode=readline \
> -object memory-backend-ram,id=mem0,size=4G,max-size=8G \
> -device virtio-mem-pci,id=reg0,memdev=mem0,node=0 \
> -object memory-backend-ram,id=mem1,size=3G,max-size=8G \
> -device virtio-mem-pci,id=reg1,memdev=mem1,node=1
>
> 2. Listing current memory assignment:
>
> --> (qemu) info memory-devices
> Memory device [virtio-mem]: "reg0"
> addr: 0x140000000
> node: 0
> size: 4294967296
> max-size: 8589934592
> memdev: /objects/mem0
> Memory device [virtio-mem]: "reg1"
> addr: 0x340000000
> node: 1
> size: 3221225472
> max-size: 8589934592
> memdev: /objects/mem1
> --> (qemu) info numa
> 2 nodes
> node 0 cpus: 0 1
> node 0 size: 6144 MB
> node 1 cpus: 2 3
> node 1 size: 5120 MB
>
> 3. Resize a virtio-mem device: Unplugging memory.
> Setting reg0 to 2G (remove 2G from NUMA node 0)
>
> --> (qemu) virtio-mem reg0 2048
> virtio-mem reg0 2048
> --> (qemu) info numa
> info numa
> 2 nodes
> node 0 cpus: 0 1
> node 0 size: 4096 MB
> node 1 cpus: 2 3
> node 1 size: 5120 MB
>
> 4. Resize a virtio-mem device: Plugging memory
> Setting reg0 to 8G (adding 6G to NUMA node 0) will replug 2G and plug
> 4G, automatically re-sizing the memory area. You might experience
> random crashes at this point if the host kernel missed a KVM patch
> (as the memory slot is not re-sized in an atomic fashion).
>
> --> (qemu) virtio-mem reg0 8192
> virtio-mem reg0 8192
> --> (qemu) info numa
> info numa
> 2 nodes
> node 0 cpus: 0 1
> node 0 size: 10240 MB
> node 1 cpus: 2 3
> node 1 size: 5120 MB
>
> 5. Resize a virtio-mem device: Try to unplug all memory.
> Setting reg0 to 0G (removing 8G from NUMA node 0) will not work. The
> guest will not be able to unplug all memory. In my example, 164M
> cannot be unplugged (out of memory).
>
> --> (qemu) virtio-mem reg0 0
> virtio-mem reg0 0
> --> (qemu) info numa
> info numa
> 2 nodes
> node 0 cpus: 0 1
> node 0 size: 2212 MB
> node 1 cpus: 2 3
> node 1 size: 5120 MB
> --> (qemu) info virtio-mem reg0
> info virtio-mem reg0
> Status: ready
> Request status: vm-oom
> Page size: 2097152 bytes
> --> (qemu) info memory-devices
> Memory device [virtio-mem]: "reg0"
> addr: 0x140000000
> node: 0
> size: 171966464
> max-size: 8589934592
> memdev: /objects/mem0
> Memory device [virtio-mem]: "reg1"
> addr: 0x340000000
> node: 1
> size: 3221225472
> max-size: 8589934592
> memdev: /objects/mem1
>
> At any point, we can migrate our guest without having to care about
> modifying the QEMU command line on the target side. Simply start the
> target e.g. with an additional '-incoming "exec: cat IMAGE"' and you're
> done.
>
> ------------------------------------------------------------------------
> VI. Problems to solve / things to sort out / missing in prototype
> ------------------------------------------------------------------------
>
> General:
> - We need an async job API to send the unplug/replug/plug requests to
> the guest and query the state. [medium/hard]
> - Handle various alignment problems. [medium]
> - We need a virtio spec
>
> Relevant for plug:
> - Resize QEMU memory regions while the guest is running (esp. grow).
> While I implemented a demo solution for KVM memory slots, something
> similar would be needed for vhost. Re-sizing of memory slots has to be
> an atomic operation. [medium]
> - NUMA: Most probably the NUMA node should not be part of the virtio-mem
> device, this should rather be indicated via e.g. ACPI. [medium]
> - x86: Add the complete possible memory to the a820 map as reserved.
> [medium]
> - x86/powerpc/...: Indicate to which NUMA node the memory belongs using
> ACPI. [medium]
> - x86/powerpc/...: Share address space with ordinary DIMMS/NVDIMMs, for
> now this is blocked for simplicity. [medium/hard]
> - If the bitmaps become too big, migrate them like memory. [medium]
>
> Relevant for unplug:
> - Allocate memory in Linux from a specific memory range. Windows has a
> nice interface for that (at least it looks nice when reading the API).
> This could be done using fake NUMA nodes or a new ZONE. My prototype
> just uses a very ugly hack. [very hard]
> - Use userfaultfd WP (write-protect) insted of mprotect. Especially,
> have multiple userfaultfd user in QEMU at a time (postcopy).
> [medium/hard]
>
> Stuff for the future:
> - Huge pages are problematic (no ZERO page support). This might not be
> trivial to support. [hard/very hard]
> - Try to free struct pages, to avoid the 1.6% overhead [very very hard]
>
>
> ------------------------------------------------------------------------
> VII. Questions
> ------------------------------------------------------------------------
>
> To get unplug working properly, it will require quite some effort,
> that's why I want to get some basic feedback before continuing working
> on a RFC implementation + RFC virtio spec.
>
> a) Did I miss anything important? Are there any ultimate blockers that I
> ignored? Any concepts that are broken?
>
> b) Are there any alternatives? Any modifications that could make life
> easier while still taking care of the requirements?
>
> c) Are there other use cases we should care about and focus on?
>
> d) Am I missing any requirements? What else could be important for
> !cloud and cloud?
>
> e) Are there any possible solutions to the allocator problem (allocating
> memory from a specific memory area)? Please speak up!
>
> f) Anything unclear?
>
> e) Any feelings about this? Yay or nay?
>
>
> As you reached this point: Thanks for having a look!!! Highly appreciated!
>
>
> ------------------------------------------------------------------------
> VIII. Q&A
> ------------------------------------------------------------------------
>
> ---
> Q: What's the problem with ordinary memory hot(un)plug?
>
> A: 1. We can only unplug in the granularity we plugged. So we have to
> know in advance, how much memory we want to remove later on. If we
> plug a 2G dimm, we can only unplug a 2G dimm.
> 2. We might run out of memory slots. Although very unlikely, this
> would strike if we try to always plug small modules in order to be
> able to unplug again (e.g. loads of 128MB modules).
> 3. Any locked page in the guest can hinder us from unplugging a dimm.
> Even if memory was onlined as online_movable, a single locked page
> can hinder us from unplugging that memory dimm.
> 4. Memory has to be onlined as online_movable. If we don't put that
> memory into the movable zone, any non-movable kernel allocation
> could end up on it, turning the complete dimm unpluggable. As
> certain allocations cannot go into the movable zone (e.g. page
> tables), the ratio between online_movable/online memory depends on
> the workload in the guest. Ratios of 50% -70% are usually fine.
> But it could happen, that there is plenty of memory available,
> but kernel allocations fail. (source: Andrea Arcangeli)
> 5. Unplugging might require several attempts. It takes some time to
> migrate all memory from the dimm. At that point, it is then not
> really obvious why it failed, and whether it could ever succeed.
> 6. Windows does support memory hotplug but not memory hotunplug. So
> this could be a way to support it also for Windows.
> ---
> Q: Will this work with Windows?
>
> A: Most probably not in the current form. Memory has to be at least
> added to the a820 map and ACPI (NUMA). Hyper-V ballon is also able to
> hotadd memory using a paravirtualized interface, so there are very
> good chances that this will work. But we won't know for sure until we
> also start prototyping.
> ---
> Q: How does this compare to virtio-balloo?
>
> A: In contrast to virtio-balloon, virtio-mem
> 1. Supports multiple page sizes, even different ones for different
> virtio-mem devices in a guest.
> 2. Is NUMA aware.
> 3. Is able to add more memory.
> 4. Doesn't work on all memory, but only on the managed one.
> 5. Has guarantees. There is now way for the guest to reclaim memory.
> ---
> Q: How does this compare to XEN balloon?
>
> A: XEN balloon also has a way to hotplug new memory. However, on a
> reboot, the guest will "see" more memory than it actually has.
> Compared to XEN balloon, virtio-mem:
> 1. Supports multiple page sizes.
> 2. Is NUMA aware.
> 3. The guest can survive a reboot into a system without the guest
> driver. If the XEN guest driver doesn't come up, the guest will
> get killed once it touches too much memory.
> 4. Reboots don't require any hacks.
> 5. The guest knows which memory is special. And it remains special
> during a reboot. Hotplugged memory not suddenly becomes base
> memory. The balloon mechanism will only work on a specific memory
> area.
> ---
> Q: How does this compare to Hyper-V balloon?
>
> A: Based on the code from the Linux Hyper-V balloon driver, I can say
> that Hyper-V also has a way to hotplug new memory. However, memory
> will remain plugged on a reboot. Therefore, the guest will see more
> memory than the hypervisor actually wants to assign to it.
> Virtio-mem in contrast:
> 1. Supports multiple page sizes.
> 2. Is NUMA aware.
> 3. I have no idea what happens under Hyper-v when
> a) rebooting into a guest without a fitting guest driver
> b) kexec() touches all memory
> c) the guest misbehaves
> 4. The guest knows which memory is special. And it remains special
> during a reboot. Hotpplugged memory not suddenly becomes base
> memory. The balloon mechanism will only work on a specific memory
> area.
> In general, it looks like the hypervisor has to deal with malicious
> guests trying to access more memory than desired by providing enough
> swap space.
> ---
> Q: How is virtio-mem NUMA aware?
>
> A: Each virtio-mem device belongs exactly to one NUMA node (if NUMA is
> enabled). As we can resize these regions separately, we can control
> from/to which node to remove/add memory.
> ---
> Q: Why do we need support for multiple page sizes?
>
> A: If huge pages are used in the host, we can only guarantee that they
> are not accessible by the guest anymore, if the guest gives up memory
> in this granularity. We prepare for that. Also, powerpc can have 64k
> pages in the host but 4k pages in the guest. So the guest must only
> give up 64k chunks. In addition, unplugging 4k pages might be bad
> when it comes to fragmentation. My prototype currently uses 256k. We
> can make this configurable - and it can vary for each virtio-mem
> device.
> ---
> Q: What are the limitations with paravirtualized memory hotplug?
>
> A: The same as for DIMM based hotplug, but we don't run out of any
> memory/ACPI slots. E.g. on x86 Linux, only 128MB chunks can be
> hotplugged, on x86 Windows it's 2MB. In addition, of course we
> have to take care of maximum address limits in the guest. The idea
> is to communicate these limits to the hypervisor via virtio-mem,
> to give hints when trying to add/remove memory.
> ---
> Q: Why not simply unplug *any* memory like virtio-balloon does?
>
> A: This could be done and a previous prototype did it like that.
> However, there are some points to consider here.
> 1. If we combine this with ordinary memory hotplug (DIMM), we most
> likely won't be able to unplug DIMMs anymore as virtio-mem memory
> gets "allocated" on these.
> 2. All guests using virtio-mem cannot use huge pages as backing
> storage at all (as virtio-mem only supports anonymous pages).
> 3. We need to track unplugged memory for the complete address space,
> so we need a global state in QEMU. Bitmaps get bigger. We will not
> be abe to dynamically grow the bitmaps for a virtio-mem device.
> 4. Resolving/checking memory to be unplugged gets significantly
> harder. How should the guest know which memory it can unplug for a
> specific virtio-mem device? E.g. if NUMA is active, only that NUMA
> node to which a virtio-mem device belongs can be used.
> 5. We will need userfaultfd handler for the complete address space,
> not just for the virtio-mem managed memory.
> Especially, if somebody hotplugs a DIMM, we dynamically will have
> to enable the userfaultfd handler.
> 6. What shall we do if somebody hotplugs a DIMM with huge pages? How
> should we tell the guest, that this memory cannot be used for
> unplugging?
> In summary: This concept is way cleaner, but also harder to
> implement.
> ---
> Q: Why not reuse virtio-balloon?
>
> A: virtio-balloon is for cooperative memory management. It has a fixed
> page size and will deflate in certain situations. Any change we
> introduce will break backwards compatibility. virtio-balloon was not
> designed to give guarantees. Nobody can hinder the guest from
> deflating/reusing inflated memory. In addition, it might make perfect
> sense to have both, virtio-balloon and virtio-mem at the same time,
> especially looking at the DEFLATE_ON_OOM or STATS features of
> virtio-balloon. While virtio-mem is all about guarantees, virtio-
> balloon is about cooperation.
> ---
> Q: Why not reuse acpi hotplug?
>
> A: We can easily run out of slots, migration in QEMU will just be
> horrible and we don't want to bind virtio* to architecture specific
> technologies.
> E.g. thinking about s390x - no ACPI. Also, mixing an ACPI driver with
> a virtio-driver sounds very weird. If the virtio-driver performs the
> hotplug itself, we might later perform some extra tricks: e.g.
> actually unplug certain regions to give up some struct pages.
>
> We want to manage the way memory is added/removed completely in QEMU.
> We cannot simply add new device from within QEMU and expect that
> migration in QEMU will work.
> ---
> Q: Why do we need resizable memory regions?
>
> A: Migration in QEMU is special. Any device we have on our source VM has
> to already be around on our target VM. So simply creating random
> devides internally in QEMU is not going to work. The concept of
> resizable memory regions in QEMU already exists and is part of the
> migration protocol. Before memory is migrated, the memory is resized.
> So in essence, this makes migration support _a lot_ easier.
>
> In addition, we won't run in any slot number restriction when
> automatically managing how to add memory in QEMU.
> ---
> Q: Why do we have to resize memory regions on a reboot?
>
> A: We have to compensate all memory that has been unplugged for that
> area by shrinking it, so that a fresh guest can use all memory when
> initializing the virtio-mem device.
> ---
> Q: Why do we need userfaultfd?
>
> A: mprotect() will create a lot of VMAs in the kernel. This will degrade
> performance and might even fail at one point. userfaultfd avoids this
> by not creating a new VMA for every protected range. userfaultfd WP
> is currently still under development and suffers from false positives
> that make it currently impossible to properly integrate this into the
> prototype.
> ---
> Q: Why do we have to allow reading unplugged memory?
>
> A: E.g. if the guest crashes and want's to write a memory dump, it will
> blindly access all memory. While we could find ways to fixup kexec,
> Windows dumps might be more problematic. Allowing the guest to read
> all memory (resulting in reading all 0's) safes us from a lot of
> trouble.
>
> The downside is, that page tables full of zero pages might be
> created. (we might be able to find ways to optimize this)
> ---
> Q: Will this work with postcopy live-migration?
>
> A: Not in the current form. And it doesn't really make sense to spend
> time on it as long as we don't use userfaultfd. Combining both
> handlers will be interesting. It can be done with some effort on the
> QEMU side.
> ---
> Q: What's the problem with shmem/hugetlbfs?
>
> A: We currently rely on the ZERO page to be mapped when the guest tries
> to read unplugged memory. For shmem/hugetlbfs, there is no ZERO page,
> so read access would result in memory getting populated. We could
> either introduce an explicit ZERO page, or manage it using one dummy
> ZERO page (using regular usefaultfd, allow only one such page to be
> mapped at a time). For now, only anonymous memory.
> ---
> Q: Ripping out random page ranges, won't this fragment our guest memory?
>
> A: Yes, but depending on the virtio-mem page size, this might be more or
> less problematic. The smaller the virtio-mem page size, the more we
> fragment and make small allocations fail. The bigger the virtio-mem
> page size, the higher the chance that we can't unplug any more
> memory.
> ---
> Q: Why can't we use memory compaction like virtio-balloon?
>
> A: If the virtio-mem page size > PAGE_SIZE, we can't do ordinary
> page migration, migration would have to be done in blocks. We could
> later add an guest->host virtqueue, via which the guest can
> "exchange" memory ranges. However, also mm has to support this kind
> of migration. So it is not completely out of scope, but will require
> quite some work.
> ---
> Q: Do we really need yet another paravirtualized interface for this?
>
> A: You tell me :)
> ---
>
> Thanks,
>
> David
>
--
Thanks,
David
^ permalink raw reply
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2017-07-28 8:25 UTC (permalink / raw)
To: linux-kernel, qemu-devel, virtualization, kvm, linux-mm, mst,
david, cornelia.huck, akpm, mgorman, aarcange, amit.shah,
pbonzini, liliang.opensource, mhocko, willy
Cc: yang.zhang.wz, virtio-dev, quan.xu
In-Reply-To: <1499863221-16206-6-git-send-email-wei.w.wang@intel.com>
On 07/12/2017 08:40 PM, Wei Wang wrote:
> Add a new feature, VIRTIO_BALLOON_F_SG, which enables to
> transfer a chunk of ballooned (i.e. inflated/deflated) pages using
> scatter-gather lists to the host.
>
> The implementation of the previous virtio-balloon is not very
> efficient, because the balloon pages are transferred to the
> host one by one. Here is the breakdown of the time in percentage
> spent on each step of the balloon inflating process (inflating
> 7GB of an 8GB idle guest).
>
> 1) allocating pages (6.5%)
> 2) sending PFNs to host (68.3%)
> 3) address translation (6.1%)
> 4) madvise (19%)
>
> It takes about 4126ms for the inflating process to complete.
> The above profiling shows that the bottlenecks are stage 2)
> and stage 4).
>
> This patch optimizes step 2) by transferring pages to the host in
> sgs. An sg describes a chunk of guest physically continuous pages.
> With this mechanism, step 4) can also be optimized by doing address
> translation and madvise() in chunks rather than page by page.
>
> With this new feature, the above ballooning process takes ~491ms
> resulting in an improvement of ~88%.
>
I found a recent mm patch, bb01b64cfab7c22f3848cb73dc0c2b46b8d38499
, zeros all the ballooned pages, which is very time consuming.
Tests show that the time to balloon 7G pages is increased from ~491 ms to
2.8 seconds with the above patch.
How about moving the zero operation to the hypervisor? In this way, we
will have a much faster balloon process.
Best,
Wei
^ permalink raw reply
* Re: Performance regression with virtio_net
From: Michael S. Tsirkin @ 2017-07-27 21:30 UTC (permalink / raw)
To: Seth Forshee; +Cc: netdev, virtualization
In-Reply-To: <20170727211430.viki2jcxbzk2u62c@ubuntu-hedt>
On Thu, Jul 27, 2017 at 04:14:30PM -0500, Seth Forshee wrote:
> On Thu, Jul 27, 2017 at 11:38:52PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jul 27, 2017 at 12:09:42PM -0500, Seth Forshee wrote:
> > > I'm seeing a performance regression with virtio_net that looks to have
> > > started in 4.12-rc1. I only see it in one context though, downloading
> > > snap packages from the Ubuntu snap store. For example:
> > >
> > > https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap
> > >
> > > which redirects to Internap's CDN. Normally this downloads in a few
> > > seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
> > > of ~150 KB/s. Everything else I've tried downloads as normal speeds.
> >
> > So just wget that URL should be enough?
>
> Yes. Note that sometimes it starts out faster then slows down.
> > > I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
> > > handling". If I revert this on top of 4.13-rc2 (along with other changes
> > > needed to successfully revert it) speeds return to normal.
> > >
> > > Thanks,
> > > Seth
> >
> >
> > Interesting. A more likely suspect would be
> > e377fcc8486d40867c6c217077ad0fa40977e060 - could you please try
> > reverting that one instead?
>
> I tried it, and I still get slow download speeds. I did test at
> 680557cf79f82623e2c4fd42733077d60a843513 during the bisect so I'm
> reasonably confident that this is the one where things went bad.
> > Also, could you please look at mergeable_rx_buffer_size in sysfs with
> > and without the change?
>
> In all cases (stock 4.13-rc2, 680557cf79f8 reverted, and e377fcc8486d
> reverted) mergeable_rx_buffer_size was 1536.
>
> Thanks,
> Seth
Do you see any error counters incrementing after it slows down?
--
MST
^ permalink raw reply
* Re: Performance regression with virtio_net
From: Seth Forshee @ 2017-07-27 21:14 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, virtualization
In-Reply-To: <20170727233244-mutt-send-email-mst@kernel.org>
On Thu, Jul 27, 2017 at 11:38:52PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 27, 2017 at 12:09:42PM -0500, Seth Forshee wrote:
> > I'm seeing a performance regression with virtio_net that looks to have
> > started in 4.12-rc1. I only see it in one context though, downloading
> > snap packages from the Ubuntu snap store. For example:
> >
> > https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap
> >
> > which redirects to Internap's CDN. Normally this downloads in a few
> > seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
> > of ~150 KB/s. Everything else I've tried downloads as normal speeds.
>
> So just wget that URL should be enough?
Yes. Note that sometimes it starts out faster then slows down.
> > I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
> > handling". If I revert this on top of 4.13-rc2 (along with other changes
> > needed to successfully revert it) speeds return to normal.
> >
> > Thanks,
> > Seth
>
>
> Interesting. A more likely suspect would be
> e377fcc8486d40867c6c217077ad0fa40977e060 - could you please try
> reverting that one instead?
I tried it, and I still get slow download speeds. I did test at
680557cf79f82623e2c4fd42733077d60a843513 during the bisect so I'm
reasonably confident that this is the one where things went bad.
> Also, could you please look at mergeable_rx_buffer_size in sysfs with
> and without the change?
In all cases (stock 4.13-rc2, 680557cf79f8 reverted, and e377fcc8486d
reverted) mergeable_rx_buffer_size was 1536.
Thanks,
Seth
^ permalink raw reply
* Re: Performance regression with virtio_net
From: Michael S. Tsirkin @ 2017-07-27 20:38 UTC (permalink / raw)
To: Seth Forshee; +Cc: netdev, virtualization
In-Reply-To: <20170727170942.ub24lwgrl3x2outp@ubuntu-hedt>
On Thu, Jul 27, 2017 at 12:09:42PM -0500, Seth Forshee wrote:
> I'm seeing a performance regression with virtio_net that looks to have
> started in 4.12-rc1. I only see it in one context though, downloading
> snap packages from the Ubuntu snap store. For example:
>
> https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap
>
> which redirects to Internap's CDN. Normally this downloads in a few
> seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
> of ~150 KB/s. Everything else I've tried downloads as normal speeds.
So just wget that URL should be enough?
> I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
> handling". If I revert this on top of 4.13-rc2 (along with other changes
> needed to successfully revert it) speeds return to normal.
>
> Thanks,
> Seth
Interesting. A more likely suspect would be
e377fcc8486d40867c6c217077ad0fa40977e060 - could you please try
reverting that one instead?
Also, could you please look at mergeable_rx_buffer_size in sysfs with
and without the change?
--
MST
^ permalink raw reply
* Re: [PATCH V2 net] Revert "vhost: cache used event for better performance"
From: Michael S. Tsirkin @ 2017-07-27 20:29 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1501125725-24529-1-git-send-email-jasowang@redhat.com>
On Thu, Jul 27, 2017 at 11:22:05AM +0800, Jason Wang wrote:
> This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92. Since it
> was reported to break vhost_net. We want to cache used event and use
> it to check for notification. The assumption was that guest won't move
> the event idx back, but this could happen in fact when 16 bit index
> wraps around after 64K entries.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> - Changes from V1: tweak commit log
> - The patch is needed for -stable.
> ---
> drivers/vhost/vhost.c | 28 ++++++----------------------
> drivers/vhost/vhost.h | 3 ---
> 2 files changed, 6 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index e4613a3..9cb3f72 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -308,7 +308,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->avail = NULL;
> vq->used = NULL;
> vq->last_avail_idx = 0;
> - vq->last_used_event = 0;
> vq->avail_idx = 0;
> vq->last_used_idx = 0;
> vq->signalled_used = 0;
> @@ -1402,7 +1401,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> r = -EINVAL;
> break;
> }
> - vq->last_avail_idx = vq->last_used_event = s.num;
> + vq->last_avail_idx = s.num;
> /* Forget the cached index value. */
> vq->avail_idx = vq->last_avail_idx;
> break;
> @@ -2241,6 +2240,10 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> __u16 old, new;
> __virtio16 event;
> bool v;
> + /* Flush out used index updates. This is paired
> + * with the barrier that the Guest executes when enabling
> + * interrupts. */
> + smp_mb();
>
> if (vhost_has_feature(vq, VIRTIO_F_NOTIFY_ON_EMPTY) &&
> unlikely(vq->avail_idx == vq->last_avail_idx))
> @@ -2248,10 +2251,6 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>
> if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
> __virtio16 flags;
> - /* Flush out used index updates. This is paired
> - * with the barrier that the Guest executes when enabling
> - * interrupts. */
> - smp_mb();
> if (vhost_get_avail(vq, flags, &vq->avail->flags)) {
> vq_err(vq, "Failed to get flags");
> return true;
> @@ -2266,26 +2265,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> if (unlikely(!v))
> return true;
>
> - /* We're sure if the following conditions are met, there's no
> - * need to notify guest:
> - * 1) cached used event is ahead of new
> - * 2) old to new updating does not cross cached used event. */
> - if (vring_need_event(vq->last_used_event, new + vq->num, new) &&
> - !vring_need_event(vq->last_used_event, new, old))
> - return false;
> -
> - /* Flush out used index updates. This is paired
> - * with the barrier that the Guest executes when enabling
> - * interrupts. */
> - smp_mb();
> -
> if (vhost_get_avail(vq, event, vhost_used_event(vq))) {
> vq_err(vq, "Failed to get used event idx");
> return true;
> }
> - vq->last_used_event = vhost16_to_cpu(vq, event);
> -
> - return vring_need_event(vq->last_used_event, new, old);
> + return vring_need_event(vhost16_to_cpu(vq, event), new, old);
> }
>
> /* This actually signals the guest, using eventfd. */
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index f720958..bb7c29b 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -115,9 +115,6 @@ struct vhost_virtqueue {
> /* Last index we used. */
> u16 last_used_idx;
>
> - /* Last used evet we've seen */
> - u16 last_used_event;
> -
> /* Used flags */
> u16 used_flags;
>
> --
> 2.7.4
^ permalink raw reply
* Performance regression with virtio_net
From: Seth Forshee @ 2017-07-27 17:09 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang; +Cc: netdev, virtualization
I'm seeing a performance regression with virtio_net that looks to have
started in 4.12-rc1. I only see it in one context though, downloading
snap packages from the Ubuntu snap store. For example:
https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap
which redirects to Internap's CDN. Normally this downloads in a few
seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
of ~150 KB/s. Everything else I've tried downloads as normal speeds.
I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
handling". If I revert this on top of 4.13-rc2 (along with other changes
needed to successfully revert it) speeds return to normal.
Thanks,
Seth
^ permalink raw reply
* [PATCH V2 net] Revert "vhost: cache used event for better performance"
From: Jason Wang @ 2017-07-27 3:22 UTC (permalink / raw)
To: mst, jasowang; +Cc: netdev, linux-kernel, kvm, virtualization
This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92. Since it
was reported to break vhost_net. We want to cache used event and use
it to check for notification. The assumption was that guest won't move
the event idx back, but this could happen in fact when 16 bit index
wraps around after 64K entries.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
- Changes from V1: tweak commit log
- The patch is needed for -stable.
---
drivers/vhost/vhost.c | 28 ++++++----------------------
drivers/vhost/vhost.h | 3 ---
2 files changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e4613a3..9cb3f72 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -308,7 +308,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->avail = NULL;
vq->used = NULL;
vq->last_avail_idx = 0;
- vq->last_used_event = 0;
vq->avail_idx = 0;
vq->last_used_idx = 0;
vq->signalled_used = 0;
@@ -1402,7 +1401,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
r = -EINVAL;
break;
}
- vq->last_avail_idx = vq->last_used_event = s.num;
+ vq->last_avail_idx = s.num;
/* Forget the cached index value. */
vq->avail_idx = vq->last_avail_idx;
break;
@@ -2241,6 +2240,10 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
__u16 old, new;
__virtio16 event;
bool v;
+ /* Flush out used index updates. This is paired
+ * with the barrier that the Guest executes when enabling
+ * interrupts. */
+ smp_mb();
if (vhost_has_feature(vq, VIRTIO_F_NOTIFY_ON_EMPTY) &&
unlikely(vq->avail_idx == vq->last_avail_idx))
@@ -2248,10 +2251,6 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
__virtio16 flags;
- /* Flush out used index updates. This is paired
- * with the barrier that the Guest executes when enabling
- * interrupts. */
- smp_mb();
if (vhost_get_avail(vq, flags, &vq->avail->flags)) {
vq_err(vq, "Failed to get flags");
return true;
@@ -2266,26 +2265,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- /* We're sure if the following conditions are met, there's no
- * need to notify guest:
- * 1) cached used event is ahead of new
- * 2) old to new updating does not cross cached used event. */
- if (vring_need_event(vq->last_used_event, new + vq->num, new) &&
- !vring_need_event(vq->last_used_event, new, old))
- return false;
-
- /* Flush out used index updates. This is paired
- * with the barrier that the Guest executes when enabling
- * interrupts. */
- smp_mb();
-
if (vhost_get_avail(vq, event, vhost_used_event(vq))) {
vq_err(vq, "Failed to get used event idx");
return true;
}
- vq->last_used_event = vhost16_to_cpu(vq, event);
-
- return vring_need_event(vq->last_used_event, new, old);
+ return vring_need_event(vhost16_to_cpu(vq, event), new, old);
}
/* This actually signals the guest, using eventfd. */
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index f720958..bb7c29b 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -115,9 +115,6 @@ struct vhost_virtqueue {
/* Last index we used. */
u16 last_used_idx;
- /* Last used evet we've seen */
- u16 last_used_event;
-
/* Used flags */
u16 used_flags;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v12 5/8] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2017-07-27 2:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: aarcange, virtio-dev, kvm, qemu-devel, amit.shah,
liliang.opensource, linux-kernel, virtualization, linux-mm,
yang.zhang.wz, quan.xu, cornelia.huck, pbonzini, akpm, mgorman
In-Reply-To: <20170726155856-mutt-send-email-mst@kernel.org>
On 07/27/2017 01:02 AM, Michael S. Tsirkin wrote:
> On Wed, Jul 26, 2017 at 11:48:41AM +0800, Wei Wang wrote:
>> On 07/23/2017 09:45 AM, Michael S. Tsirkin wrote:
>>> On Fri, Jul 14, 2017 at 03:12:43PM +0800, Wei Wang wrote:
>>>> On 07/14/2017 04:19 AM, Michael S. Tsirkin wrote:
>>>>> On Thu, Jul 13, 2017 at 03:42:35PM +0800, Wei Wang wrote:
>>>>>> On 07/12/2017 09:56 PM, Michael S. Tsirkin wrote:
>>>>>>> So the way I see it, there are several issues:
>>>>>>>
>>>>>>> - internal wait - forces multiple APIs like kick/kick_sync
>>>>>>> note how kick_sync can fail but your code never checks return code
>>>>>>> - need to re-write the last descriptor - might not work
>>>>>>> for alternative layouts which always expose descriptors
>>>>>>> immediately
>>>>>> Probably it wasn't clear. Please let me explain the two functions here:
>>>>>>
>>>>>> 1) virtqueue_add_chain_desc(vq, head_id, prev_id,..):
>>>>>> grabs a desc from the vq and inserts it to the chain tail (which is indexed
>>>>>> by
>>>>>> prev_id, probably better to call it tail_id). Then, the new added desc
>>>>>> becomes
>>>>>> the tail (i.e. the last desc). The _F_NEXT flag is cleared for each desc
>>>>>> when it's
>>>>>> added to the chain, and set when another desc comes to follow later.
>>>>> And this only works if there are multiple rings like
>>>>> avail + descriptor ring.
>>>>> It won't work e.g. with the proposed new layout where
>>>>> writing out a descriptor exposes it immediately.
>>>> I think it can support the 1.1 proposal, too. But before getting
>>>> into that, I think we first need to deep dive into the implementation
>>>> and usage of _first/next/last. The usage would need to lock the vq
>>>> from the first to the end (otherwise, the returned info about the number
>>>> of available desc in the vq, i.e. num_free, would be invalid):
>>>>
>>>> lock(vq);
>>>> add_first();
>>>> add_next();
>>>> add_last();
>>>> unlock(vq);
>>>>
>>>> However, I think the case isn't this simple, since we need to check more
>>>> things
>>>> after each add_xx() step. For example, if only one entry is available at the
>>>> time
>>>> we start to use the vq, that is, num_free is 0 after add_first(), we
>>>> wouldn't be
>>>> able to add_next and add_last. So, it would work like this:
>>>>
>>>> start:
>>>> ...get free page block..
>>>> lock(vq)
>>>> retry:
>>>> ret = add_first(..,&num_free,);
>>>> if(ret == -ENOSPC) {
>>>> goto retry;
>>>> } else if (!num_free) {
>>>> add_chain_head();
>>>> unlock(vq);
>>>> kick & wait;
>>>> goto start;
>>>> }
>>>> next_one:
>>>> ...get free page block..
>>>> add_next(..,&num_free,);
>>>> if (!num_free) {
>>>> add_chain_head();
>>>> unlock(vq);
>>>> kick & wait;
>>>> goto start;
>>>> } if (num_free == 1) {
>>>> ...get free page block..
>>>> add_last(..);
>>>> unlock(vq);
>>>> kick & wait;
>>>> goto start;
>>>> } else {
>>>> goto next_one;
>>>> }
>>>>
>>>> The above seems unnecessary to me to have three different APIs.
>>>> That's the reason to combine them into one virtqueue_add_chain_desc().
>>>>
>>>> -- or, do you have a different thought about using the three APIs?
>>>>
>>>>
>>>> Implementation Reference:
>>>>
>>>> struct desc_iterator {
>>>> unsigned int head;
>>>> unsigned int tail;
>>>> };
>>>>
>>>> add_first(*vq, *desc_iterator, *num_free, ..)
>>>> {
>>>> if (vq->vq.num_free < 1)
>>>> return -ENOSPC;
>>>> get_desc(&desc_id);
>>>> desc[desc_id].flag &= ~_F_NEXT;
>>>> desc_iterator->head = desc_id
>>>> desc_iterator->tail = desc_iterator->head;
>>>> *num_free = vq->vq.num_free;
>>>> }
>>>>
>>>> add_next(vq, desc_iterator, *num_free,..)
>>>> {
>>>> get_desc(&desc_id);
>>>> desc[desc_id].flag &= ~_F_NEXT;
>>>> desc[desc_iterator.tail].next = desc_id;
>>>> desc[desc_iterator->tail].flag |= _F_NEXT;
>>>> desc_iterator->tail = desc_id;
>>>> *num_free = vq->vq.num_free;
>>>> }
>>>>
>>>> add_last(vq, desc_iterator,..)
>>>> {
>>>> get_desc(&desc_id);
>>>> desc[desc_id].flag &= ~_F_NEXT;
>>>> desc[desc_iterator.tail].next = desc_id;
>>>> desc_iterator->tail = desc_id;
>>>>
>>>> add_chain_head(); // put the desc_iterator.head to the ring
>>>> }
>>>>
>>>>
>>>> Best,
>>>> Wei
>>> OK I thought this over. While we might need these new APIs in
>>> the future, I think that at the moment, there's a way to implement
>>> this feature that is significantly simpler. Just add each s/g
>>> as a separate input buffer.
>>
>> Should it be an output buffer?
> Hypervisor overwrites these pages with zeroes. Therefore it is
> writeable by device: DMA_FROM_DEVICE.
Why would the hypervisor need to zero the buffer? I think it may only
need to read out the info(base,size).
I think it should be like this:
the cmd hdr buffer: input, because the hypervisor would write it to
send a cmd to the guest
the payload buffer: output, for the hypervisor to read the info
>> I think output means from the
>> driver to device (i.e. DMA_TO_DEVICE).
> This part is correct I believe.
>
>>> This needs zero new APIs.
>>>
>>> I know that follow-up patches need to add a header in front
>>> so you might be thinking: how am I going to add this
>>> header? The answer is quite simple - add it as a separate
>>> out header.
>>>
>>> Host will be able to distinguish between header and pages
>>> by looking at the direction, and - should we want to add
>>> IN data to header - additionally size (<4K => header).
>>
>> I think this works fine when the cmdq is only used for
>> reporting the unused pages.
>> It would be an issue
>> if there are other usages (e.g. report memory statistics)
>> interleaving. I think one solution would be to lock the cmdq until
>> a cmd usage is done ((e.g. all the unused pages are reported) ) -
>> in this case, the periodically updated guest memory statistics
>> may be delayed for a while occasionally when live migration starts.
>> Would this be acceptable? If not, probably we can have the cmdq
>> for one usage only.
>>
>>
>> Best,
>> Wei
> OK I see, I think the issue is that reporting free pages
> was structured like stats. Let's split it -
> send pages on e.g. free_vq, get commands on vq shared with
> stats.
>
Would it be better to have the "report free page" command to be sent
through the free_vq? In this case,we will have
stats_vq: for the stats usage, which is already there
free_vq: for reporting free pages.
Best,
Wei
^ permalink raw reply
* Re: [PATCH 7/8] drm: Nuke drm_atomic_helper_connector_dpms
From: Noralf Trønnes @ 2017-07-26 19:00 UTC (permalink / raw)
To: Daniel Vetter, Intel Graphics Development
Cc: Martyn Welch, Neil Armstrong, David Airlie, nouveau,
DRI Development, virtualization, Andrzej Hajda, Tomi Valkeinen,
Laurent Pinchart, Benjamin Gaignard, Romain Perier, Daniel Vetter,
Jyri Sarha, Heiko Stuebner, Marek Vasut, linux-renesas-soc,
Archit Taneja, Lars-Peter Clausen, Joonyoung Shim, Kevin Hilman,
Alexey Brodkin, Russell King, Krzy
In-Reply-To: <20170725080122.20548-8-daniel.vetter@ffwll.ch>
Den 25.07.2017 10.01, skrev Daniel Vetter:
> It's dead code, the core handles all this directly now.
>
> The only special case is nouveau and tda988x which used one function
> for both legacy modeset code and -nv50 atomic world instead of 2
> vtables. But amounts to exactly the same.
>
> v2: Rebase over the panel/brideg refactorings in stm/ltdc.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
...
> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> index ec43fb7ad9e4..79b6687977d3 100644
> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
> @@ -71,7 +71,6 @@ static void tinydrm_connector_destroy(struct drm_connector *connector)
> }
>
> static const struct drm_connector_funcs tinydrm_connector_funcs = {
> - .dpms = drm_atomic_helper_connector_dpms,
> .reset = drm_atomic_helper_connector_reset,
> .detect = tinydrm_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
Acked-by: Noralf Trønnes <noralf@tronnes.org>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ 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