From: David Hildenbrand <hidden> Date: 2020-02-03 16:34:47
On 03.02.20 17:18, Alexander Duyck wrote:
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
> On 29.01.20 20:11, Tyler Sanderson wrote:
> >
> >
> > On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
> > <mailto:david@redhat.com>> wrote:
> >
> > On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
> > > A primary advantage of virtio balloon over other memory reclaim
> > > mechanisms is that it can pressure the guest's page cache into
> > shrinking.
> > >
> > > However, since the balloon driver changed to using the shrinker
API
> > >
> >
> <https://github.com/torvalds/linux/commit/71994620bb25a8b109388fefa9
> e99a28e355255a#diff-fd202acf694d9eba19c8c64da3e480c9> this
> > > use case has become a bit more tricky. I'm wondering what the
> intended
> > > device implementation is.
> > >
> > > When inflating the balloon against page cache (i.e. no free
memory
> > > remains) vmscan.c will both shrink page cache, but also invoke
the
> > > shrinkers -- including the balloon's shrinker. So the balloon
driver
> > > allocates memory which requires reclaim, vmscan gets this memory
> by
> > > shrinking the balloon, and then the driver adds the memory back
to
> the
> > > balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
--
Thanks,
David / dhildenb
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-03 17:04:17
On Mon, Feb 03, 2020 at 05:34:20PM +0100, David Hildenbrand wrote:
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
> On 29.01.20 20:11, Tyler Sanderson wrote:
> >
> >
> > On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
> > <mailto:david@redhat.com>> wrote:
> >
> > On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
> > > A primary advantage of virtio balloon over other memory reclaim
> > > mechanisms is that it can pressure the guest's page cache into
> > shrinking.
> > >
> > > However, since the balloon driver changed to using the shrinker
API
> > >
> >
> <https://github.com/torvalds/linux/commit/71994620bb25a8b109388fefa9
> e99a28e355255a#diff-fd202acf694d9eba19c8c64da3e480c9> this
> > > use case has become a bit more tricky. I'm wondering what the
> intended
> > > device implementation is.
> > >
> > > When inflating the balloon against page cache (i.e. no free
memory
> > > remains) vmscan.c will both shrink page cache, but also invoke
the
> > > shrinkers -- including the balloon's shrinker. So the balloon
driver
> > > allocates memory which requires reclaim, vmscan gets this memory
> by
> > > shrinking the balloon, and then the driver adds the memory back
to
> the
> > > balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
OK ... I guess that means we need to fix shrinker to take
VIRTIO_BALLOON_F_MUST_TELL_HOST into account correctly.
Hosts ignore it at the moment but it's a fragile thing
to do what it does and ignore used buffers.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
--
Thanks,
David / dhildenb
There were apparently good reasons for moving away from OOM notifier
callback:
https://lkml.org/lkml/2018/7/12/314https://lkml.org/lkml/2018/8/2/322
In particular the OOM notifier is worse than the shrinker because:
1. It is last-resort, which means the system has already gone through
heroics to prevent OOM. Those heroic reclaim efforts are expensive and
impact application performance.
2. It lacks understanding of NUMA or other OOM constraints.
3. It has a higher potential for bugs due to the subtlety of the
callback context.
Given the above, I think the shrinker API certainly makes the most sense
_if_ the balloon size is static. In that case memory should be reclaimed
from the balloon early and proportionally to balloon size, which the
shrinker API achieves.
However, if the balloon is inflating and intentionally causing memory
pressure then this results in the inefficiency pointed out earlier.
If the balloon is inflating but not causing memory pressure then there is
no problem with either API.
This suggests another route: rather than cause memory pressure to shrink
the page cache, the balloon could issue the equivalent of "echo 3 >
/proc/sys/vm/drop_caches".
Of course ideally, we want to be more fine grained than "drop everything".
We really want an API that says "drop everything that hasn't been accessed
in the last 5 minutes".
This would eliminate the need for the balloon to cause memory pressure at
all which avoids the inefficiency in question. Furthermore, this pairs
nicely with the FREE_PAGE_HINT feature.
On Mon, Feb 3, 2020 at 9:04 AM Michael S. Tsirkin [off-list ref] wrote:
On Mon, Feb 03, 2020 at 05:34:20PM +0100, David Hildenbrand wrote:
quoted
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref]
wrote:
quoted
quoted
quoted
quoted
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
> On 29.01.20 20:11, Tyler Sanderson wrote:
> >
> >
> > On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <
david@redhat.com
quoted
quoted
quoted
quoted
> > <mailto:david@redhat.com>> wrote:
> >
> > On 29.01.20 01:22, Tyler Sanderson via Virtualization
wrote:
quoted
quoted
quoted
quoted
> > > A primary advantage of virtio balloon over other
memory reclaim
quoted
quoted
quoted
quoted
> > > mechanisms is that it can pressure the guest's page
cache into
quoted
quoted
quoted
quoted
> > shrinking.
> > >
> > > However, since the balloon driver changed to using the
> by
> > > shrinking the balloon, and then the driver adds the
memory back
quoted
quoted
quoted
quoted
to
> the
> > > balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke
shrinker as
quoted
quoted
quoted
quoted
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of
activity on
quoted
quoted
quoted
quoted
the deflate queue. The balloon is being shrunk. And this only starts
once all
quoted
quoted
quoted
quoted
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier
with shrinker")
quoted
quoted
quoted
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively
trying
quoted
quoted
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
OK ... I guess that means we need to fix shrinker to take
VIRTIO_BALLOON_F_MUST_TELL_HOST into account correctly.
Hosts ignore it at the moment but it's a fragile thing
to do what it does and ignore used buffers.
quoted
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
--
Thanks,
David / dhildenb
From: Nadav Amit <hidden> Date: 2020-02-03 22:50:43
On Feb 3, 2020, at 8:34 AM, David Hildenbrand [off-list ref] wrote:
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
quoted
On 29.01.20 20:11, Tyler Sanderson wrote:
quoted
On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
quoted
A primary advantage of virtio balloon over other memory reclaim
mechanisms is that it can pressure the guest's page cache into
shrinking.
quoted
However, since the balloon driver changed to using the shrinker
use case has become a bit more tricky. I'm wondering what the
intended
quoted
quoted
device implementation is.
When inflating the balloon against page cache (i.e. no free
memory
quoted
quoted
quoted
remains) vmscan.c will both shrink page cache, but also invoke
the
quoted
quoted
quoted
shrinkers -- including the balloon's shrinker. So the balloon
driver
quoted
quoted
quoted
allocates memory which requires reclaim, vmscan gets this memory
by
quoted
quoted
shrinking the balloon, and then the driver adds the memory back
to
quoted
the
quoted
quoted
balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Regards,
Nadav
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-04 05:45:52
On Mon, Feb 03, 2020 at 12:32:05PM -0800, Tyler Sanderson wrote:
There were apparently good reasons for moving away from OOM notifier callback:
https://lkml.org/lkml/2018/7/12/314https://lkml.org/lkml/2018/8/2/322
In particular the OOM notifier is worse than the shrinker because:
1. It is last-resort, which means the system has already gone through heroics
to prevent OOM. Those heroic reclaim efforts are expensive and impact
application performance.
2. It lacks understanding of NUMA or other OOM constraints.
3. It has a higher potential for bugs due to the subtlety of the callback
context.
Given the above, I think the shrinker API certainly makes the most sense _if_
the balloon size is static. In that case memory should be reclaimed from the
balloon early and proportionally to balloon size, which the shrinker API
achieves.
OK that sounds like VIRTIO_BALLOON_F_FREE_PAGE_HINT then.
However, if the balloon is inflating and intentionally causing memory pressure
then this results in the inefficiency pointed out earlier.
And that sounds like VIRTIO_BALLOON_F_DEFLATE_ON_OOM.
If the balloon is inflating but not causing memory pressure then there is no
problem with either API.
This suggests another route: rather than cause memory pressure to shrink the
page cache, the balloon could issue the equivalent of "echo 3 > /proc/sys/vm/
drop_caches".
Of course ideally, we want to be more fine grained than "drop everything". We
really want an API that says "drop everything that hasn't been accessed in the
last 5 minutes".
This would eliminate the need for the balloon to cause memory pressure at all
which avoids the inefficiency in question. Furthermore, this pairs nicely with
the FREE_PAGE_HINT feature.
Well we still do have a regression. So we probably should revert
for now, and separately look for better solutions.
On Mon, Feb 3, 2020 at 9:04 AM Michael S. Tsirkin [off-list ref] wrote:
On Mon, Feb 03, 2020 at 05:34:20PM +0100, David Hildenbrand wrote:
> On 03.02.20 17:18, Alexander Duyck wrote:
> > On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
> >> On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
> >>>
> >>> On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref]
wrote:
> >>>
> >>> On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
> >>> > On 29.01.20 20:11, Tyler Sanderson wrote:
> >>> > >
> >>> > >
> >>> > > On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <
david@redhat.com
> >>> > > <mailto:david@redhat.com>> wrote:
> >>> > >
> >>> > > On 29.01.20 01:22, Tyler Sanderson via Virtualization
wrote:
> >>> > > > A primary advantage of virtio balloon over other memory
reclaim
> >>> > > > mechanisms is that it can pressure the guest's page
cache into
> >>> > > shrinking.
> >>> > > >
> >>> > > > However, since the balloon driver changed to using the
shrinker
> >>> API
> >>> > > >
> >>> > >
> >>> > <https://github.com/torvalds/linux/commit/
71994620bb25a8b109388fefa9
> >>> > e99a28e355255a#diff-fd202acf694d9eba19c8c64da3e480c9> this
> >>> > > > use case has become a bit more tricky. I'm wondering
what the
> >>> > intended
> >>> > > > device implementation is.
> >>> > > >
> >>> > > > When inflating the balloon against page cache (i.e. no
free
> >>> memory
> >>> > > > remains) vmscan.c will both shrink page cache, but also
invoke
> >>> the
> >>> > > > shrinkers -- including the balloon's shrinker. So the
balloon
> >>> driver
> >>> > > > allocates memory which requires reclaim, vmscan gets
this memory
> >>> > by
> >>> > > > shrinking the balloon, and then the driver adds the
memory back
> >>> to
> >>> > the
> >>> > > > balloon. Basically a busy no-op.
> >>>
> >>> Per my understanding, the balloon allocation won’t invoke
shrinker as
> >>> __GFP_DIRECT_RECLAIM isn't set, no?
> >>>
> >>> I could be wrong about the mechanism, but the device sees lots of
activity on
> >>> the deflate queue. The balloon is being shrunk. And this only starts
once all
> >>> free memory is depleted and we're inflating into page cache.
> >>
> >> So given this looks like a regression, maybe we should revert the
> >> patch in question 71994620bb25 ("virtio_balloon: replace oom notifier
with shrinker")
> >> Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
> >> shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
> >> at all.
> >>
> >> So it looks like all this rework introduced more issues than it
> >> addressed ...
> >>
> >> I also CC Alex Duyck for an opinion on this.
> >> Alex, what do you use to put pressure on page cache?
> >
> > I would say reverting probably makes sense. I'm not sure there is much
> > value to having a shrinker running deflation when you are actively
trying
> > to increase the balloon. It would make more sense to wait until you are
> > actually about to start hitting oom.
>
> I think the shrinker makes sense for free page hinting feature
> (everything on free_page_list).
>
> So instead of only reverting, I think we should split it up and always
> register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
> notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
OK ... I guess that means we need to fix shrinker to take
VIRTIO_BALLOON_F_MUST_TELL_HOST into account correctly.
Hosts ignore it at the moment but it's a fragile thing
to do what it does and ignore used buffers.
> (Of course, adapting what is being done in the shrinker and in the OOM
> notifier)
>
> --
> Thanks,
>
> David / dhildenb
The issue is that DEFLATE_ON_OOM is under-specified.
1. It is last-resort, which means the system has already gone through
heroics to prevent OOM. Those heroic reclaim efforts are expensive
and impact application performance.
That's *exactly* what "deflate on OOM" suggests.
Assume you are using virtio-balloon for some weird way of memory
hotunplug (which is what some people do) and you want to minimize the
footprint of your guest. Then you really only want to give the guest
more memory (or rather, let it take back memory automatically in this
case) in case it really needs more memory. It should try to reclaim first.
Under-specified.
2. It lacks understanding of NUMA or other OOM constraints.
Ballooning in general lacks the understanding of NUMA.
3. It has a higher potential for bugs due to the subtlety of the
callback context.
While that is a valid point, it doesn't explain why existing
functionality is changed.
Personally, I think DEFLATE_ON_OOM should never have been introduced (at
least not in this form).
--
Thanks,
David / dhildenb
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: David Hildenbrand <hidden> Date: 2020-02-04 08:35:34
quoted
quoted
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Will do. It all sounds sub-optimal to me at this point ... but I prefer
the old variant where a simple "drop_slab()" won't deflate the balloon.
That looks broken to me.
--
Thanks,
David / dhildenb
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-04 08:40:53
On Tue, Feb 04, 2020 at 09:35:21AM +0100, David Hildenbrand wrote:
quoted
quoted
quoted
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
Well VIRTIO_BALLOON_F_MUST_TELL_HOST is also broken by shrinker
with VIRTIO_BALLOON_F_FREE_PAGE_HINT as that code adds buffers
but does not wait for them to be used even with VIRTIO_BALLOON_F_MUST_TELL_HOST.
We never noticed because QEMU does not advertize
VIRTIO_BALLOON_F_MUST_TELL_HOST.
quoted
quoted
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Will do. It all sounds sub-optimal to me at this point ... but I prefer
the old variant where a simple "drop_slab()" won't deflate the balloon.
That looks broken to me.
From: David Hildenbrand <hidden> Date: 2020-02-04 08:49:03
On 04.02.20 09:40, Michael S. Tsirkin wrote:
On Tue, Feb 04, 2020 at 09:35:21AM +0100, David Hildenbrand wrote:
quoted
quoted
quoted
quoted
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
Well VIRTIO_BALLOON_F_MUST_TELL_HOST is also broken by shrinker
with VIRTIO_BALLOON_F_FREE_PAGE_HINT as that code adds buffers
but does not wait for them to be used even with VIRTIO_BALLOON_F_MUST_TELL_HOST.
We never noticed because QEMU does not advertize
VIRTIO_BALLOON_F_MUST_TELL_HOST.
Will try to figure out how to best undo this mess :)
quoted
quoted
quoted
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Will do. It all sounds sub-optimal to me at this point ... but I prefer
the old variant where a simple "drop_slab()" won't deflate the balloon.
That looks broken to me.
From: David Hildenbrand <hidden> Date: 2020-02-04 14:30:34
On 04.02.20 09:40, Michael S. Tsirkin wrote:
On Tue, Feb 04, 2020 at 09:35:21AM +0100, David Hildenbrand wrote:
quoted
quoted
quoted
quoted
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
Well VIRTIO_BALLOON_F_MUST_TELL_HOST is also broken by shrinker
with VIRTIO_BALLOON_F_FREE_PAGE_HINT as that code adds buffers
but does not wait for them to be used even with VIRTIO_BALLOON_F_MUST_TELL_HOST.
We never noticed because QEMU does not advertize
VIRTIO_BALLOON_F_MUST_TELL_HOST.
So, I am trying to understand how the code is intended to work, but I
am afraid I am missing something (or to rephrase: I think I found a BUG :) and
there is lack of proper documentation about this feature).
a) We allocate pages and add them to the list as long as we are told to do so.
We send these pages to the host one by one.
b) We free all pages once we get a STOP signal. Until then, we keep pages allocated.
c) When called via the shrinker, we want to free pages from the list, even
though the hypervisor did not notify us to do so.
Issue 1: When we unload the balloon driver in the guest in an unlucky event,
we won't free the pages. We are missing something like (if I am not wrong):
@@ -929,6 +929,10 @@ static void remove_common(struct virtio_balloon *vb)leak_balloon(vb,vb->num_pages);update_balloon_size(vb);+/* There might be free pages that are being reported: release them. */+if(virtio_has_feature(vb->vdev,VIRTIO_BALLOON_F_FREE_PAGE_HINT))+return_free_pages_to_mm(vb,ULONG_MAX);+/* Now we reset the device so we can clean up the queues. */vb->vdev->config->reset(vb->vdev);
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be
that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means
(-ENOCLUE) that we have to wait until the hypervisor notifies us via the STOP? Or
for which event do we have to wait? Because there is no way to *tell host* here
that we want to reuse a page. The hypervisor will *tell us* when we can reuse pages.
For the shrinker it is simple: Don't use the shrinker with
VIRTIO_BALLOON_F_MUST_TELL_HOST :) . But to fix Issue 1, we *would* have to wait
until we get a STOP signal. That is not really possible because it might
take an infinite amount of time.
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to VIRTIO_BALLOON_F_FREE_PAGE_HINT and
we'd better document that. It introduces complexity with no clear benefit.
--
Thanks,
David / dhildenb
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-04 16:50:32
On Tue, Feb 04, 2020 at 03:30:19PM +0100, David Hildenbrand wrote:
quoted hunk
On 04.02.20 09:40, Michael S. Tsirkin wrote:
quoted
On Tue, Feb 04, 2020 at 09:35:21AM +0100, David Hildenbrand wrote:
quoted
quoted
quoted
quoted
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
Well VIRTIO_BALLOON_F_MUST_TELL_HOST is also broken by shrinker
with VIRTIO_BALLOON_F_FREE_PAGE_HINT as that code adds buffers
but does not wait for them to be used even with VIRTIO_BALLOON_F_MUST_TELL_HOST.
We never noticed because QEMU does not advertize
VIRTIO_BALLOON_F_MUST_TELL_HOST.
So, I am trying to understand how the code is intended to work, but I
am afraid I am missing something (or to rephrase: I think I found a BUG :) and
there is lack of proper documentation about this feature).
a) We allocate pages and add them to the list as long as we are told to do so.
We send these pages to the host one by one.
b) We free all pages once we get a STOP signal. Until then, we keep pages allocated.
c) When called via the shrinker, we want to free pages from the list, even
though the hypervisor did not notify us to do so.
Issue 1: When we unload the balloon driver in the guest in an unlucky event,
we won't free the pages. We are missing something like (if I am not wrong):
@@ -929,6 +929,10 @@ static void remove_common(struct virtio_balloon *vb)leak_balloon(vb,vb->num_pages);update_balloon_size(vb);+/* There might be free pages that are being reported: release them. */+if(virtio_has_feature(vb->vdev,VIRTIO_BALLOON_F_FREE_PAGE_HINT))+return_free_pages_to_mm(vb,ULONG_MAX);+/* Now we reset the device so we can clean up the queues. */vb->vdev->config->reset(vb->vdev);
Indeed.
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be
that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means
(-ENOCLUE) that we have to wait until the hypervisor notifies us via the STOP? Or
for which event do we have to wait? Because there is no way to *tell host* here
that we want to reuse a page. The hypervisor will *tell us* when we can reuse pages.
For the shrinker it is simple: Don't use the shrinker with
VIRTIO_BALLOON_F_MUST_TELL_HOST :) . But to fix Issue 1, we *would* have to wait
until we get a STOP signal. That is not really possible because it might
take an infinite amount of time.
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to VIRTIO_BALLOON_F_FREE_PAGE_HINT and
we'd better document that. It introduces complexity with no clear benefit.
I meant that we must wait for host to see the hint. Signalled via using
the buffer. But maybe that's too far in the meaning from
VIRTIO_BALLOON_F_MUST_TELL_HOST and we need a separate new flag for
that. Then current code won't be broken (yay!) but we need to
document another flag that's pretty similar.
From: David Hildenbrand <hidden> Date: 2020-02-04 16:56:33
[...]
quoted
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be
that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means
(-ENOCLUE) that we have to wait until the hypervisor notifies us via the STOP? Or
for which event do we have to wait? Because there is no way to *tell host* here
that we want to reuse a page. The hypervisor will *tell us* when we can reuse pages.
For the shrinker it is simple: Don't use the shrinker with
VIRTIO_BALLOON_F_MUST_TELL_HOST :) . But to fix Issue 1, we *would* have to wait
until we get a STOP signal. That is not really possible because it might
take an infinite amount of time.
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to VIRTIO_BALLOON_F_FREE_PAGE_HINT and
we'd better document that. It introduces complexity with no clear benefit.
I meant that we must wait for host to see the hint. Signalled via using
the buffer. But maybe that's too far in the meaning from
VIRTIO_BALLOON_F_MUST_TELL_HOST and we need a separate new flag for
Yes, that's what I think.
that. Then current code won't be broken (yay!) but we need to
document another flag that's pretty similar.
I mean, do we need a flag at all as long as there is no user?
Introducing a flag and documenting it if nobody uses it does not sound
like a work I will enjoy :)
We can simply document "VIRTIO_BALLOON_F_MUST_TELL_HOST does not apply
to FREE_PAGE_HINTING" and "with FREE_PAGE_HINTING, the guest can reuse
pages any time, without waiting for a response/ack from the hypervisor".
Thoughts?
--
Thanks,
David / dhildenb
The issue is that DEFLATE_ON_OOM is under-specified.
quoted
1. It is last-resort, which means the system has already gone through
heroics to prevent OOM. Those heroic reclaim efforts are expensive
and impact application performance.
That's *exactly* what "deflate on OOM" suggests.
It seems there are some use cases where "deflate on OOM" is desired and
others where "deflate on pressure" is desired.
This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that registers
the shrinker, and reverting DEFLATE_ON_OOM to use the OOM notifier callback.
This lets users configure the balloon for their use case.
Assume you are using virtio-balloon for some weird way of memory
hotunplug (which is what some people do) and you want to minimize the
footprint of your guest. Then you really only want to give the guest
more memory (or rather, let it take back memory automatically in this
case) in case it really needs more memory. It should try to reclaim first.
Under-specified.
quoted
2. It lacks understanding of NUMA or other OOM constraints.
Ballooning in general lacks the understanding of NUMA.
quoted
3. It has a higher potential for bugs due to the subtlety of the
callback context.
While that is a valid point, it doesn't explain why existing
functionality is changed.
Personally, I think DEFLATE_ON_OOM should never have been introduced (at
least not in this form).
I'm actually not sure how you would safely do memory overcommit without
DEFLATE_ON_OOM. So I think it unlocks a huge use case.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-04 18:56:40
On Tue, Feb 04, 2020 at 10:52:42AM -0800, Tyler Sanderson wrote:
On Tue, Feb 4, 2020 at 12:29 AM David Hildenbrand [off-list ref] wrote:
On 03.02.20 21:32, Tyler Sanderson wrote:
> There were apparently good reasons for moving away from OOM notifier
> callback:
> https://lkml.org/lkml/2018/7/12/314
> https://lkml.org/lkml/2018/8/2/322
>
> In particular the OOM notifier is worse than the shrinker because:
The issue is that DEFLATE_ON_OOM is under-specified.
>
>Â 1. It is last-resort, which means the system has already gone through
>Â Â Â heroics to prevent OOM. Those heroic reclaim efforts are expensive
>Â Â Â and impact application performance.
That's *exactly* what "deflate on OOM" suggests.
It seems there are some use cases where "deflate on OOM" is desired and others
where "deflate on pressure" is desired.
This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that registers the
shrinker, and reverting DEFLATE_ON_OOM to use the OOM notifier callback.
This lets users configure the balloon for their use case.
Right. Let's not repeat past mistakes and let's try to specify this
new one properly though :)
Assume you are using virtio-balloon for some weird way of memory
hotunplug (which is what some people do) and you want to minimize the
footprint of your guest. Then you really only want to give the guest
more memory (or rather, let it take back memory automatically in this
case) in case it really needs more memory. It should try to reclaim first.
Under-specified.
>Â 2. It lacks understanding of NUMA or other OOM constraints.
Ballooning in general lacks the understanding of NUMA.
> 3. It has a higher potential for bugs due to the subtlety of the
>Â Â Â callback context.
While that is a valid point, it doesn't explain why existing
functionality is changed.
Personally, I think DEFLATE_ON_OOM should never have been introduced (at
least not in this form).
I'm actually not sure how you would safely do memory overcommit without
DEFLATE_ON_OOM. So I think it unlocks a huge use case.
Â
--
Thanks,
David / dhildenb
From: David Hildenbrand <hidden> Date: 2020-02-04 19:17:30
On 04.02.20 19:52, Tyler Sanderson wrote:
On Tue, Feb 4, 2020 at 12:29 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 03.02.20 21:32, Tyler Sanderson wrote:
> There were apparently good reasons for moving away from OOM notifier
> callback:
> https://lkml.org/lkml/2018/7/12/314
> https://lkml.org/lkml/2018/8/2/322
>
> In particular the OOM notifier is worse than the shrinker because:
The issue is that DEFLATE_ON_OOM is under-specified.
>
> 1. It is last-resort, which means the system has already gone through
> heroics to prevent OOM. Those heroic reclaim efforts are expensive
> and impact application performance.
That's *exactly* what "deflate on OOM" suggests.
It seems there are some use cases where "deflate on OOM" is desired and
others where "deflate on pressure" is desired.
This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that
registers the shrinker, and reverting DEFLATE_ON_OOM to use the OOM
notifier callback.
This lets users configure the balloon for their use case.
You want the old behavior back, so why should we introduce a new one? Or
am I missing something? (you did want us to revert to old handling, no?)
I consider virtio-balloon to this very day a big hack. And I don't see
it getting better with new config knobs. Having that said, the
technologies that are candidates to replace it (free page reporting,
taming the guest page cache, etc.) are still not ready - so we'll have
to stick with it for now :( .
I'm actually not sure how you would safely do memory overcommit without
DEFLATE_ON_OOM. So I think it unlocks a huge use case.
Using better suited technologies that are not ready yet (well, some form
of free page reporting is available under IBM z already but in a
proprietary form) ;) Anyhow, I remember that DEFLATE_ON_OOM only makes
it less likely to crash your guest, but not that you are safe to squeeze
the last bit out of your guest VM.
--
Thanks,
David / dhildenb
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-04 20:33:32
On Tue, Feb 04, 2020 at 05:56:22PM +0100, David Hildenbrand wrote:
[...]
quoted
quoted
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be
that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means
(-ENOCLUE) that we have to wait until the hypervisor notifies us via the STOP? Or
for which event do we have to wait? Because there is no way to *tell host* here
that we want to reuse a page. The hypervisor will *tell us* when we can reuse pages.
For the shrinker it is simple: Don't use the shrinker with
VIRTIO_BALLOON_F_MUST_TELL_HOST :) . But to fix Issue 1, we *would* have to wait
until we get a STOP signal. That is not really possible because it might
take an infinite amount of time.
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to VIRTIO_BALLOON_F_FREE_PAGE_HINT and
we'd better document that. It introduces complexity with no clear benefit.
I meant that we must wait for host to see the hint. Signalled via using
the buffer. But maybe that's too far in the meaning from
VIRTIO_BALLOON_F_MUST_TELL_HOST and we need a separate new flag for
Yes, that's what I think.
quoted
that. Then current code won't be broken (yay!) but we need to
document another flag that's pretty similar.
I mean, do we need a flag at all as long as there is no user?
Introducing a flag and documenting it if nobody uses it does not sound
like a work I will enjoy :)
It's not the user. It's the non-orthogonality that I find inelegant.
Let me try to formulate the issue, forgive me for thinking aloud
(and I Cc'd virtio-dev since we are talking spec things here):
The annoying thing is that with Alex's VIRTIO_BALLOON_F_REPORTING
host does depend on guest not touching memory before host uses it.
So functionally VIRTIO_BALLOON_F_FREE_PAGE_HINT and
VIRTIO_BALLOON_F_REPORTING really are supposed to do
exectly the same thing, with the differences being
- VIRTIO_BALLOON_F_FREE_PAGE_HINT comes upon host's request.
VIRTIO_BALLOON_F_REPORTING is initiated by guest.
- VIRTIO_BALLOON_F_FREE_PAGE_HINT does not always wait for
host to use the hint before touching the page.
Well it almost always does, but there's an exception in the
shrinker which tries to stop reporting as quickly as possible
in the case of a slow host.
VIRTIO_BALLOON_F_REPORTING always does.
This means host can blow the page away when it sees the hint.
Now the point is that with VIRTIO_BALLOON_F_REPORTING
I think you really must wait for host to use the hint.
But with VIRTIO_BALLOON_F_FREE_PAGE_HINT it depends
on how host uses it. Something to think about,
I'm not sure what is the best thing to do here.
We can simply document "VIRTIO_BALLOON_F_MUST_TELL_HOST does not apply
to FREE_PAGE_HINTING" and "with FREE_PAGE_HINTING, the guest can reuse
pages any time, without waiting for a response/ack from the hypervisor".
Thoughts?
--
Thanks,
David / dhildenb
On Tue, Feb 4, 2020 at 11:17 AM David Hildenbrand [off-list ref] wrote:
On 04.02.20 19:52, Tyler Sanderson wrote:
quoted
On Tue, Feb 4, 2020 at 12:29 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 03.02.20 21:32, Tyler Sanderson wrote:
> There were apparently good reasons for moving away from OOM
notifier
quoted
> callback:
> https://lkml.org/lkml/2018/7/12/314
> https://lkml.org/lkml/2018/8/2/322
>
> In particular the OOM notifier is worse than the shrinker because:
The issue is that DEFLATE_ON_OOM is under-specified.
>
> 1. It is last-resort, which means the system has already gone
through
quoted
> heroics to prevent OOM. Those heroic reclaim efforts are
expensive
quoted
> and impact application performance.
That's *exactly* what "deflate on OOM" suggests.
It seems there are some use cases where "deflate on OOM" is desired and
others where "deflate on pressure" is desired.
This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that
registers the shrinker, and reverting DEFLATE_ON_OOM to use the OOM
notifier callback.
This lets users configure the balloon for their use case.
You want the old behavior back, so why should we introduce a new one? Or
am I missing something? (you did want us to revert to old handling, no?)
Reverting actually doesn't help me because this has been the behavior since
Linux 4.19 which is already widely in use. So my device implementation
needs to handle the shrinker behavior anyways. I started this conversation
to ask what the intended device implementation was.
I think there are reasonable device implementations that would prefer the
shrinker behavior (it turns out that mine doesn't).
For example, an implementation that slowly inflates the balloon for the
purpose of memory overcommit. It might leave the balloon inflated and
expect any memory pressure (including page cache usage) to deflate the
balloon as a way to dynamically right-size the balloon.
Two reasons I didn't go with the above implementation:
1. I need to support guests before Linux 4.19 which don't have the shrinker
behavior.
2. Memory in the balloon does not appear as "available" in /proc/meminfo
even though it is freeable. This is confusing to users, but isn't a deal
breaker.
If we added a DEFLATE_ON_PRESSURE feature bit that indicated shrinker API
support then that would resolve reason #1 (ideally we would backport the
bit to 4.19).
In any case, the shrinker behavior when pressuring page cache is more of an
inefficiency than a bug. It's not clear to me that it necessitates
reverting. If there were/are reasons to be on the shrinker interface then I
think those carry similar weight as the problem itself.
I consider virtio-balloon to this very day a big hack. And I don't see
it getting better with new config knobs. Having that said, the
technologies that are candidates to replace it (free page reporting,
taming the guest page cache, etc.) are still not ready - so we'll have
to stick with it for now :( .
quoted
I'm actually not sure how you would safely do memory overcommit without
DEFLATE_ON_OOM. So I think it unlocks a huge use case.
Using better suited technologies that are not ready yet (well, some form
of free page reporting is available under IBM z already but in a
proprietary form) ;) Anyhow, I remember that DEFLATE_ON_OOM only makes
it less likely to crash your guest, but not that you are safe to squeeze
the last bit out of your guest VM.
Can you elaborate on the danger of DEFLATE_ON_OOM? I haven't seen any
problems in testing but I'd really like to know about the dangers.
Is there a difference in safety between the OOM notifier callback and the
shrinker API?
On Tue, Feb 4, 2020 at 3:58 PM Tyler Sanderson [off-list ref] wrote:
On Tue, Feb 4, 2020 at 11:17 AM David Hildenbrand [off-list ref]
wrote:
quoted
On 04.02.20 19:52, Tyler Sanderson wrote:
quoted
On Tue, Feb 4, 2020 at 12:29 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 03.02.20 21:32, Tyler Sanderson wrote:
> There were apparently good reasons for moving away from OOM
notifier
quoted
> callback:
> https://lkml.org/lkml/2018/7/12/314
> https://lkml.org/lkml/2018/8/2/322
>
> In particular the OOM notifier is worse than the shrinker because:
The issue is that DEFLATE_ON_OOM is under-specified.
>
> 1. It is last-resort, which means the system has already gone
through
quoted
> heroics to prevent OOM. Those heroic reclaim efforts are
expensive
quoted
> and impact application performance.
That's *exactly* what "deflate on OOM" suggests.
It seems there are some use cases where "deflate on OOM" is desired and
others where "deflate on pressure" is desired.
This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that
registers the shrinker, and reverting DEFLATE_ON_OOM to use the OOM
notifier callback.
This lets users configure the balloon for their use case.
You want the old behavior back, so why should we introduce a new one? Or
am I missing something? (you did want us to revert to old handling, no?)
Reverting actually doesn't help me because this has been the behavior
since Linux 4.19 which is already widely in use. So my device
implementation needs to handle the shrinker behavior anyways. I started
this conversation to ask what the intended device implementation was.
I should clarify: reverting _would_ improve guest performance under my
implementation. So I guess I'm in favor. But I think we should consider
reasonable alternative implementations. I think this suggests adding a new
feature bit to allow device implementations to choose.
I think there are reasonable device implementations that would prefer the
shrinker behavior (it turns out that mine doesn't).
For example, an implementation that slowly inflates the balloon for the
purpose of memory overcommit. It might leave the balloon inflated and
expect any memory pressure (including page cache usage) to deflate the
balloon as a way to dynamically right-size the balloon.
Two reasons I didn't go with the above implementation:
1. I need to support guests before Linux 4.19 which don't have the
shrinker behavior.
2. Memory in the balloon does not appear as "available" in /proc/meminfo
even though it is freeable. This is confusing to users, but isn't a deal
breaker.
If we added a DEFLATE_ON_PRESSURE feature bit that indicated shrinker API
support then that would resolve reason #1 (ideally we would backport the
bit to 4.19).
In any case, the shrinker behavior when pressuring page cache is more of
an inefficiency than a bug. It's not clear to me that it necessitates
reverting. If there were/are reasons to be on the shrinker interface then I
think those carry similar weight as the problem itself.
quoted
I consider virtio-balloon to this very day a big hack. And I don't see
it getting better with new config knobs. Having that said, the
technologies that are candidates to replace it (free page reporting,
taming the guest page cache, etc.) are still not ready - so we'll have
to stick with it for now :( .
quoted
I'm actually not sure how you would safely do memory overcommit without
DEFLATE_ON_OOM. So I think it unlocks a huge use case.
Using better suited technologies that are not ready yet (well, some form
of free page reporting is available under IBM z already but in a
proprietary form) ;) Anyhow, I remember that DEFLATE_ON_OOM only makes
it less likely to crash your guest, but not that you are safe to squeeze
the last bit out of your guest VM.
Can you elaborate on the danger of DEFLATE_ON_OOM? I haven't seen any
problems in testing but I'd really like to know about the dangers.
Is there a difference in safety between the OOM notifier callback and the
shrinker API?
From: Wang, Wei W <hidden> Date: 2020-02-05 06:49:32
On Tuesday, February 4, 2020 10:30 PM, David Hildenbrand wrote:
So, I am trying to understand how the code is intended to work, but I am
afraid I am missing something (or to rephrase: I think I found a BUG :) and
there is lack of proper documentation about this feature).
a) We allocate pages and add them to the list as long as we are told to do
so.
We send these pages to the host one by one.
b) We free all pages once we get a STOP signal. Until then, we keep pages
allocated.
Yes. Either host sends to the guest a STOP cmd or when the guest fails to allocate a page (meaning that all the possible free pages are taken already),
the reporting ends.
quoted hunk
c) When called via the shrinker, we want to free pages from the list, even
though the hypervisor did not notify us to do so.
Issue 1: When we unload the balloon driver in the guest in an unlucky event,
we won't free the pages. We are missing something like (if I am not wrong):
*vb)
leak_balloon(vb, vb->num_pages);
update_balloon_size(vb);
+ /* There might be free pages that are being reported: release them.
*/
+ if (virtio_has_feature(vb->vdev,
VIRTIO_BALLOON_F_FREE_PAGE_HINT))
+ return_free_pages_to_mm(vb, ULONG_MAX);
+
/* Now we reset the device so we can clean up the queues. */
vb->vdev->config->reset(vb->vdev);
Right, thanks!
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be that
we do have VIRTIO_BALLOON_F_MUST_TELL_HOST.
I don't think it is an issue here.
MUST_TELL_HOST is for the ballooning pages, where pages are offered to host to _USE_.
For free page hint, as the name already suggests, it's just a _HINT_ , so in whatever use case,
the host should not take the page to use. So the guest doesn't need to tell host and wait.
Back to the implementation of virtio_balloon_shrinker_scan, which I don't see an issue so far:
shrink_free_pages just return pages to mm without waiting for the ack from host
shrink_balloon_pages goes through leak_balloon which tell_host before release the balloon pages.
Best,
Wei
From: Wang, Wei W <hidden> Date: 2020-02-05 06:52:44
On Wednesday, February 5, 2020 12:50 AM, Michael S. Tsirkin wrote:
quoted
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to
VIRTIO_BALLOON_F_FREE_PAGE_HINT and we'd better document that. It
introduces complexity with no clear benefit.
I meant that we must wait for host to see the hint.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-05 06:57:56
On Tue, Feb 04, 2020 at 03:58:51PM -0800, Tyler Sanderson wrote:
>Â Â Â >
>Â Â Â >Â 1. It is last-resort, which means the system has already gone through
>Â Â Â >Â Â Â heroics to prevent OOM. Those heroic reclaim efforts are expensive
>Â Â Â >Â Â Â and impact application performance.
>
>Â Â Â That's *exactly* what "deflate on OOM" suggests.
>
>
> It seems there are some use cases where "deflate on OOM" is desired and
> others where "deflate on pressure" is desired.
> This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that
> registers the shrinker, and reverting DEFLATE_ON_OOM to use the OOM
> notifier callback.
>
> This lets users configure the balloon for their use case.
You want the old behavior back, so why should we introduce a new one? Or
am I missing something? (you did want us to revert to old handling, no?)
Reverting actually doesn't help me because this has been the behavior since
Linux 4.19 which is already widely in use. So my device implementation needs to
handle the shrinker behavior anyways. I started this conversation to ask what
the intended device implementation was.
I think there are reasonable device implementations that would prefer the
shrinker behavior (it turns out that mine doesn't).
For example, an implementation that slowly inflates the balloon for the purpose
of memory overcommit. It might leave the balloon inflated and expect any memory
pressure (including page cache usage) to deflate the balloon as a way to
dynamically right-size the balloon.
So just to make sure we understand, what exactly does your
implementation do?
Two reasons I didn't go with the above implementation:
1. I need to support guests before Linux 4.19 which don't have the shrinker
behavior.
2. Memory in the balloon does not appear as "available" in /proc/meminfo even
though it is freeable. This is confusing to users, but isn't a deal breaker.
If we added a DEFLATE_ON_PRESSURE feature bit that indicated shrinker API
support then that would resolve reason #1 (ideally we would backport the bit to
4.19).
We could declare lack of pagecache pressure with DEFLATE_ON_OOM a
regression and backport the revert but not I think the new
DEFLATE_ON_PRESSURE.
In any case, the shrinker behavior when pressuring page cache is more of an
inefficiency than a bug. It's not clear to me that it necessitates reverting.
If there were/are reasons to be on the shrinker interface then I think those
carry similar weight as the problem itself.
Â
I consider virtio-balloon to this very day a big hack. And I don't see
it getting better with new config knobs. Having that said, the
technologies that are candidates to replace it (free page reporting,
taming the guest page cache, etc.) are still not ready - so we'll have
to stick with it for now :( .
>
> I'm actually not sure how you would safely do memory overcommit without
> DEFLATE_ON_OOM. So I think it unlocks a huge use case.
Using better suited technologies that are not ready yet (well, some form
of free page reporting is available under IBM z already but in a
proprietary form) ;) Anyhow, I remember that DEFLATE_ON_OOM only makes
it less likely to crash your guest, but not that you are safe to squeeze
the last bit out of your guest VM.
Can you elaborate on the danger of DEFLATE_ON_OOM? I haven't seen any problems
in testing but I'd really like to know about the dangers.
Is there a difference in safety between the OOM notifier callback and the
shrinker API?
It's not about dangers as such. It's just that when linux hits OOM
all kind of error paths are being hit, latent bugs start triggering,
latency goes up drastically.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-05 07:05:30
On Wed, Feb 05, 2020 at 06:52:34AM +0000, Wang, Wei W wrote:
On Wednesday, February 5, 2020 12:50 AM, Michael S. Tsirkin wrote:
quoted
quoted
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to
VIRTIO_BALLOON_F_FREE_PAGE_HINT and we'd better document that. It
introduces complexity with no clear benefit.
I meant that we must wait for host to see the hint.
Why?
Best,
Wei
Well if we did the hint would be reliable, allowing host to immediately
drop any pages it gets in the hint. Originally I wanted to speed up
hinting by never waiting for host, but that does not seem to be what was
implemented: the only place we don't wait is the shrinker and it seems a
waste that we introduced complexity to host without getting any real
benefit out of it.
VIRTIO_BALLOON_F_MUST_TELL_HOST doesn't really apply to hinting
right now, so we could have used it to mean "hints must wait for host
to use buffers". I'm afraid it's already a wasted opportunity at
this point, reusing it isn't worth the compatibility headaches.
--
MST
From: Nadav Amit <hidden> Date: 2020-02-05 07:35:29
On Feb 3, 2020, at 2:50 PM, Nadav Amit [off-list ref] wrote:
quoted
On Feb 3, 2020, at 8:34 AM, David Hildenbrand [off-list ref] wrote:
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
quoted
On 29.01.20 20:11, Tyler Sanderson wrote:
quoted
On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
quoted
A primary advantage of virtio balloon over other memory reclaim
mechanisms is that it can pressure the guest's page cache into
shrinking.
quoted
However, since the balloon driver changed to using the shrinker
use case has become a bit more tricky. I'm wondering what the
intended
quoted
quoted
device implementation is.
When inflating the balloon against page cache (i.e. no free
memory
quoted
quoted
quoted
remains) vmscan.c will both shrink page cache, but also invoke
the
quoted
quoted
quoted
shrinkers -- including the balloon's shrinker. So the balloon
driver
quoted
quoted
quoted
allocates memory which requires reclaim, vmscan gets this memory
by
quoted
quoted
shrinking the balloon, and then the driver adds the memory back
to
quoted
the
quoted
quoted
balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Adding some information for the record (if someone googles this thread):
In the VMware balloon driver, the shrinker is disabled by default since we
encountered a performance degradation in testing. I tried to avoid rapid
inflation/shrinker-deflation cycles by adding a timeout, but apparently it
did not help in avoiding the performance regression.
So there is no such issue in VMware balloon driver, unless someone
intentionally enables the shrinker through a module parameter.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: David Hildenbrand <hidden> Date: 2020-02-05 08:19:22
quoted
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be that
we do have VIRTIO_BALLOON_F_MUST_TELL_HOST.
I don't think it is an issue here.
MUST_TELL_HOST is for the ballooning pages, where pages are offered to host to _USE_.
For free page hint, as the name already suggests, it's just a _HINT_ , so in whatever use case,
the host should not take the page to use. So the guest doesn't need to tell host and wait.
Yes, I agree with you. Yet, I am thinking about one
(unlikely?impossible?) scenario. Can you refresh my brain why that
cannot happen (IOW, why we don't have to wait for the host to process
the request)?
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. After that,
it is done using that page for the next hour.
4. The host processes the request and clears the bit in the dirty bitmap.
5. The guest is being migrated by the host. The modified page is not
being migrated.
--
Thanks,
David / dhildenb
From: David Hildenbrand <hidden> Date: 2020-02-05 08:20:11
On 05.02.20 08:35, Nadav Amit wrote:
quoted
On Feb 3, 2020, at 2:50 PM, Nadav Amit [off-list ref] wrote:
quoted
On Feb 3, 2020, at 8:34 AM, David Hildenbrand [off-list ref] wrote:
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
quoted
On 29.01.20 20:11, Tyler Sanderson wrote:
quoted
On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
quoted
A primary advantage of virtio balloon over other memory reclaim
mechanisms is that it can pressure the guest's page cache into
shrinking.
quoted
However, since the balloon driver changed to using the shrinker
use case has become a bit more tricky. I'm wondering what the
intended
quoted
quoted
device implementation is.
When inflating the balloon against page cache (i.e. no free
memory
quoted
quoted
quoted
remains) vmscan.c will both shrink page cache, but also invoke
the
quoted
quoted
quoted
shrinkers -- including the balloon's shrinker. So the balloon
driver
quoted
quoted
quoted
allocates memory which requires reclaim, vmscan gets this memory
by
quoted
quoted
shrinking the balloon, and then the driver adds the memory back
to
quoted
the
quoted
quoted
balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Adding some information for the record (if someone googles this thread):
In the VMware balloon driver, the shrinker is disabled by default since we
encountered a performance degradation in testing. I tried to avoid rapid
inflation/shrinker-deflation cycles by adding a timeout, but apparently it
did not help in avoiding the performance regression.
Thanks for that info. To me that sounds like the shrinker is the wrong
approach to "auto-deflation". It's not just "some slab cache".
--
Thanks,
David / dhildenb
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: David Hildenbrand <hidden> Date: 2020-02-05 08:32:01
On 04.02.20 21:33, Michael S. Tsirkin wrote:
On Tue, Feb 04, 2020 at 05:56:22PM +0100, David Hildenbrand wrote:
quoted
[...]
quoted
quoted
Issue 2: When called via the shrinker, (but also to fix Issue 1), it could be
that we do have VIRTIO_BALLOON_F_MUST_TELL_HOST. I assume this means
(-ENOCLUE) that we have to wait until the hypervisor notifies us via the STOP? Or
for which event do we have to wait? Because there is no way to *tell host* here
that we want to reuse a page. The hypervisor will *tell us* when we can reuse pages.
For the shrinker it is simple: Don't use the shrinker with
VIRTIO_BALLOON_F_MUST_TELL_HOST :) . But to fix Issue 1, we *would* have to wait
until we get a STOP signal. That is not really possible because it might
take an infinite amount of time.
Michael, any clue on which event we have to wait with
VIRTIO_BALLOON_F_MUST_TELL_HOST? IMHO, I don't think
VIRTIO_BALLOON_F_MUST_TELL_HOST applies to VIRTIO_BALLOON_F_FREE_PAGE_HINT and
we'd better document that. It introduces complexity with no clear benefit.
I meant that we must wait for host to see the hint. Signalled via using
the buffer. But maybe that's too far in the meaning from
VIRTIO_BALLOON_F_MUST_TELL_HOST and we need a separate new flag for
Yes, that's what I think.
quoted
that. Then current code won't be broken (yay!) but we need to
document another flag that's pretty similar.
I mean, do we need a flag at all as long as there is no user?
Introducing a flag and documenting it if nobody uses it does not sound
like a work I will enjoy :)
It's not the user. It's the non-orthogonality that I find inelegant.
Let me try to formulate the issue, forgive me for thinking aloud
(and I Cc'd virtio-dev since we are talking spec things here):
The annoying thing is that with Alex's VIRTIO_BALLOON_F_REPORTING
host does depend on guest not touching memory before host uses it.
So functionally VIRTIO_BALLOON_F_FREE_PAGE_HINT and
VIRTIO_BALLOON_F_REPORTING really are supposed to do
exectly the same thing, with the differences being
- VIRTIO_BALLOON_F_FREE_PAGE_HINT comes upon host's request.
VIRTIO_BALLOON_F_REPORTING is initiated by guest.
- VIRTIO_BALLOON_F_FREE_PAGE_HINT does not always wait for
host to use the hint before touching the page.
Well it almost always does, but there's an exception in the
shrinker which tries to stop reporting as quickly as possible
in the case of a slow host.
VIRTIO_BALLOON_F_REPORTING always does.
This means host can blow the page away when it sees the hint.
Now the point is that with VIRTIO_BALLOON_F_REPORTING
I think you really must wait for host to use the hint.
But with VIRTIO_BALLOON_F_FREE_PAGE_HINT it depends
on how host uses it. Something to think about,
I'm not sure what is the best thing to do here.
I think VIRTIO_BALLOON_F_FREE_PAGE_HINT is really the special case and
shall be left alone (not messed with VIRTIO_BALLOON_F_MUST_TELL_HOST).
Initiated by the host, complicated protocol and semantics, guest can
reuse pages any time it wants ("hint").
VIRTIO_BALLOON_F_REPORTING is *basically* ordinary inflation on
stereoids (be able to report a size for each page and multiple pages in
one go) BUT, we can currently *never* have
VIRTIO_BALLOON_F_MUST_TELL_HOST semantics - there is no deflation.
We could rename VIRTIO_BALLOON_F_REPORTING to something like
VIRTIO_BALLOON_F_SIZE and make it obey to
VIRTIO_BALLOON_F_MUST_TELL_HOST (meaning, there would have to be a
deflate queue as well!) - but it contradicts to the real needs.
VIRTIO_BALLOON_F_REPORTING comnbined with
VIRTIO_BALLOON_F_MUST_TELL_HOST would not be usable by Linux for free
page reporting.
Well, as QEMU never sets VIRTIO_BALLOON_F_MUST_TELL_HOST we would be
fine. Alexander would have to add an inflate+deflate queue and make his
feature depend on !VIRTIO_BALLOON_F_MUST_TELL_HOST.
Is that the consistency you're looking for? Alexander, thoughts?
--
Thanks,
David / dhildenb
From: Wang, Wei W <hidden> Date: 2020-02-05 08:51:03
On Wednesday, February 5, 2020 3:05 PM, Michael S. Tsirkin wrote:
Well if we did the hint would be reliable, allowing host to immediately drop
any pages it gets in the hint.
"drop", you mean host to unmap the page from guest? I think that's not allowed for hints.
Originally I wanted to speed up hinting by never
waiting for host, but that does not seem to be what was
implemented: the only place we don't wait is the shrinker
Didn't get this one. For FREE_PAGE_HINT, the hints are always sent to host without
an ack from host about whether it has read the hint or not. (please see get_free_page_and_send)
and it seems a
waste that we introduced complexity to host without getting any real
benefit out of it.
VIRTIO_BALLOON_F_MUST_TELL_HOST doesn't really apply to hinting right
now,
There is no need I think, as host isn't allowed to use or unmap the hint page.
Best,
Wei
From: Wang, Wei W <hidden> Date: 2020-02-05 08:54:46
On Wednesday, February 5, 2020 4:19 PM, David Hildenbrand wrote:
Yes, I agree with you. Yet, I am thinking about one
(unlikely?impossible?) scenario. Can you refresh my brain why that cannot
happen (IOW, why we don't have to wait for the host to process the
request)?
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. After that, it is
done using that page for the next hour.
4. The host processes the request and clears the bit in the dirty bitmap.
5. The guest is being migrated by the host. The modified page is not being
migrated.
Whenever the guest modifies a page during migration, it will be captured by the
dirty logging and the hypervisor will send the dirtied the page in the following round.
Just more thoughts to clarify the difference. I think it's all about the page ownership.
For VIRTIO_BALLOON_F_FREE_PAGE_HINT, the guest always owns the page,
so host should not use or unmap the page.
For VIRTIO_BALLOON_F_REPORTING or the legacy balloon inflation,
guest intends to transfer the ownership of the underlying physical page to the host,
that's why host and guest needs a sync about - if the "ownership" transfer completes or not.
Best,
Wei
From: David Hildenbrand <hidden> Date: 2020-02-05 08:56:52
On 05.02.20 09:54, Wang, Wei W wrote:
On Wednesday, February 5, 2020 4:19 PM, David Hildenbrand wrote:
quoted
Yes, I agree with you. Yet, I am thinking about one
(unlikely?impossible?) scenario. Can you refresh my brain why that cannot
happen (IOW, why we don't have to wait for the host to process the
request)?
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. After that, it is
done using that page for the next hour.
4. The host processes the request and clears the bit in the dirty bitmap.
5. The guest is being migrated by the host. The modified page is not being
migrated.
Whenever the guest modifies a page during migration, it will be captured by the
dirty logging and the hypervisor will send the dirtied the page in the following round.
Please explain why the steps I outlined don't apply esp. in the last
round. Your general statement does not explain why this race can't happen.
--
Thanks,
David / dhildenb
From: Wang, Wei W <hidden> Date: 2020-02-05 09:00:11
On Wednesday, February 5, 2020 4:57 PM, David Hildenbrand wrote:
quoted
quoted
Yes, I agree with you. Yet, I am thinking about one
(unlikely?impossible?) scenario. Can you refresh my brain why that
cannot happen (IOW, why we don't have to wait for the host to process
the request)?
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. After
that, it is done using that page for the next hour.
4. The host processes the request and clears the bit in the dirty bitmap.
5. The guest is being migrated by the host. The modified page is not
being migrated.
Whenever the guest modifies a page during migration, it will be
captured by the dirty logging and the hypervisor will send the dirtied the
page in the following round.
Please explain why the steps I outlined don't apply esp. in the last round.
Your general statement does not explain why this race can't happen.
The guest is stopped in the last round, thus no page will be modified at that time.
Best,
Wei
From: David Hildenbrand <hidden> Date: 2020-02-05 09:05:56
On 05.02.20 10:00, Wang, Wei W wrote:
On Wednesday, February 5, 2020 4:57 PM, David Hildenbrand wrote:
quoted
quoted
quoted
Yes, I agree with you. Yet, I am thinking about one
(unlikely?impossible?) scenario. Can you refresh my brain why that
cannot happen (IOW, why we don't have to wait for the host to process
the request)?
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. After
that, it is done using that page for the next hour.
4. The host processes the request and clears the bit in the dirty bitmap.
5. The guest is being migrated by the host. The modified page is not
being migrated.
Whenever the guest modifies a page during migration, it will be
captured by the dirty logging and the hypervisor will send the dirtied the
page in the following round.
Please explain why the steps I outlined don't apply esp. in the last round.
Your general statement does not explain why this race can't happen.
The guest is stopped in the last round, thus no page will be modified at that time.
No, that does not answer my question. Because then, obviously the guest
can't do any hinting in the last round. I think I am missing something
important :)
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. The dirty
bit is set in the hypervisor.
4. The host processes the request and clears the bit in the dirty bitmap.
5. The guest is stopped and the last set of dirty pages is migrated. The
modified page is not being migrated (because not marked dirty).
Something between 3. and 4. has to guarantee that the page will still be
migrated, what guarantees that?
--
Thanks,
David / dhildenb
From: Wang, Wei W <hidden> Date: 2020-02-05 09:19:15
On Wednesday, February 5, 2020 5:06 PM, David Hildenbrand wrote:
No, that does not answer my question. Because then, obviously the guest
can't do any hinting in the last round. I think I am missing something
important :)
No problem, probably need more details here:
QEMU has a dirty bitmap which indicates all the dirty pages from the previous round.
KVM has a dirty bitmap which records what pages are modified in this round.
When a new round starts, QEMU syncs the bitmap from KVM (this round always
sends the pages dirtied from the previous round).
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. The dirty bit is
set in the hypervisor.
The bit will be set in KVM's bitmap, and will be synced to QEMU's bitmap when the next round starts.
4. The host processes the request and clears the bit in the dirty bitmap.
This clears the bit from the QEMU bitmap, and this page will not be sent in this round.
5. The guest is stopped and the last set of dirty pages is migrated. The
modified page is not being migrated (because not marked dirty).
When QEMU start the last round, it first syncs the bitmap from KVM, which includes the one set in step 3.
Then the modified page gets sent.
Best,
Wei
From: David Hildenbrand <hidden> Date: 2020-02-05 09:22:45
quoted
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. The dirty bit is
set in the hypervisor.
The bit will be set in KVM's bitmap, and will be synced to QEMU's bitmap when the next round starts.
quoted
4. The host processes the request and clears the bit in the dirty bitmap.
This clears the bit from the QEMU bitmap, and this page will not be sent in this round.
quoted
5. The guest is stopped and the last set of dirty pages is migrated. The
modified page is not being migrated (because not marked dirty).
When QEMU start the last round, it first syncs the bitmap from KVM, which includes the one set in step 3.
Then the modified page gets sent.
So, if you run a TCG guest and use it with free page reporting, the race
is possible? So the correctness depends on two dirty bitmaps in the
hypervisor and how they interact. wow this is fragile.
Thanks for the info :)
--
Thanks,
David / dhildenb
From: Wang, Wei W <hidden> Date: 2020-02-05 09:35:13
On Wednesday, February 5, 2020 5:23 PM, David Hildenbrand wrote:
So, if you run a TCG guest and use it with free page reporting, the race is
possible? So the correctness depends on two dirty bitmaps in the hypervisor
and how they interact. wow this is fragile.
Not sure how TCG tracks the dirty bits. But In whatever implementation, the hypervisor should have
already dealt with the race between he current round and the previous round dirty recording.
(the race isn't brought by this feature essentially)
Best,
Wei
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-05 09:35:41
On Wed, Feb 05, 2020 at 10:22:34AM +0100, David Hildenbrand wrote:
quoted
quoted
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. The dirty bit is
set in the hypervisor.
The bit will be set in KVM's bitmap, and will be synced to QEMU's bitmap when the next round starts.
quoted
4. The host processes the request and clears the bit in the dirty bitmap.
This clears the bit from the QEMU bitmap, and this page will not be sent in this round.
quoted
5. The guest is stopped and the last set of dirty pages is migrated. The
modified page is not being migrated (because not marked dirty).
When QEMU start the last round, it first syncs the bitmap from KVM, which includes the one set in step 3.
Then the modified page gets sent.
So, if you run a TCG guest and use it with free page reporting, the race
is possible?
I'd have to look at the implementation but the basic idea is not
kvm specific. The idea is that hypervisor can detect that 3 happened
after 1, by means of creating a copy of the dirty bitmap
when request is sent to the guest.
So the correctness depends on two dirty bitmaps in the
hypervisor and how they interact. wow this is fragile.
Thanks for the info :)
It's pretty fragile, and the annoying part is we do not
actually benefit from this at all since it all only triggers
in the shrinker corner case.
The original idea was that we can send any hint to hypervisor and reuse
the page immediately without waiting for hint to be seen. That seemed
worth having, as a means to minimize impact of hinting.
Then we dropped that and switched to OOM, and there not having
to wait also seemed like a worthwhile thing.
In the end we switched to shrinker where we can wait
if we like, and many guests never even hit the shrinker so we
have sacrificed robustness for nothing.
If we go back to OOM then at least it's justified ..
From: David Hildenbrand <hidden> Date: 2020-02-05 09:37:39
On 05.02.20 10:35, Wang, Wei W wrote:
On Wednesday, February 5, 2020 5:23 PM, David Hildenbrand wrote:
quoted
So, if you run a TCG guest and use it with free page reporting, the race is
possible? So the correctness depends on two dirty bitmaps in the hypervisor
and how they interact. wow this is fragile.
Not sure how TCG tracks the dirty bits. But In whatever implementation, the hypervisor should have
There is only a single bitmap for that purpose. (well, the one where KVM
syncs to)
already dealt with the race between he current round and the previous round dirty recording.
(the race isn't brought by this feature essentially)
It is guaranteed to work reliably without this feature as you only clear
what *has been migrated*, not what your guest thinks should not been
migrated at one point and decides differently at another point. The race
is bought forwards by this feature.
--
Thanks,
David / dhildenb
From: David Hildenbrand <hidden> Date: 2020-02-05 09:58:29
On 05.02.20 10:49, Wang, Wei W wrote:
On Wednesday, February 5, 2020 5:37 PM, David Hildenbrand wrote:
quoted
quoted
Not sure how TCG tracks the dirty bits. But In whatever
implementation, the hypervisor should have
There is only a single bitmap for that purpose. (well, the one where KVM
syncs to)
quoted
already dealt with the race between he current round and the previous
round dirty recording.
quoted
(the race isn't brought by this feature essentially)
It is guaranteed to work reliably without this feature as you only clear what
*has been migrated*,
Not "clear what has been migrated" (that skips nothing..)
Anyway, it's a hint used for optimization.
Yes, an optimization that might easily lead to data corruption when the
two bitmaps are either not in place or don't play along in that specific
way (and I suspect this is the case under TCG).
--
Thanks,
David / dhildenb
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-05 10:25:25
On Wed, Feb 05, 2020 at 10:58:14AM +0100, David Hildenbrand wrote:
On 05.02.20 10:49, Wang, Wei W wrote:
quoted
On Wednesday, February 5, 2020 5:37 PM, David Hildenbrand wrote:
quoted
quoted
Not sure how TCG tracks the dirty bits. But In whatever
implementation, the hypervisor should have
There is only a single bitmap for that purpose. (well, the one where KVM
syncs to)
quoted
already dealt with the race between he current round and the previous
round dirty recording.
quoted
(the race isn't brought by this feature essentially)
It is guaranteed to work reliably without this feature as you only clear what
*has been migrated*,
Not "clear what has been migrated" (that skips nothing..)
Anyway, it's a hint used for optimization.
Yes, an optimization that might easily lead to data corruption when the
two bitmaps are either not in place or don't play along in that specific
way (and I suspect this is the case under TCG).
So I checked and TCG has two copies too.
Each block has bmap used for migration and also dirty_memory
where pages are marked dirty. See cpu_physical_memory_sync_dirty_bitmap.
So from QEMU POV, there is a callback that tells balloon when it's safe
to request hints. As that affects the bitmap, that must not happen in
parallel with dirty bitmap handling. Sounds like a reasonable
limitation.
The hint can be useful outside migration, but in its current form
needs to then be non-destructive.
E.g. I can imaging userspace calling MADV_SOFT_OFFLINE on the hinted
memory.
Again a flag that tells guest it should wait until used
could be a reasonable expension. If we stick to the shrinker
it's actually implementable easily. With an OOM notifier - I'm not so
sure ...
And a big part of the problem is that after all this time the page
hinting interfaces are still undocumented. Quite sad really :(
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2020-02-05 10:27:56
On Wed, Feb 05, 2020 at 09:19:58AM +0100, David Hildenbrand wrote:
On 05.02.20 08:35, Nadav Amit wrote:
quoted
quoted
On Feb 3, 2020, at 2:50 PM, Nadav Amit [off-list ref] wrote:
quoted
On Feb 3, 2020, at 8:34 AM, David Hildenbrand [off-list ref] wrote:
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
quoted
On 29.01.20 20:11, Tyler Sanderson wrote:
quoted
On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
quoted
A primary advantage of virtio balloon over other memory reclaim
mechanisms is that it can pressure the guest's page cache into
shrinking.
quoted
However, since the balloon driver changed to using the shrinker
use case has become a bit more tricky. I'm wondering what the
intended
quoted
quoted
device implementation is.
When inflating the balloon against page cache (i.e. no free
memory
quoted
quoted
quoted
remains) vmscan.c will both shrink page cache, but also invoke
the
quoted
quoted
quoted
shrinkers -- including the balloon's shrinker. So the balloon
driver
quoted
quoted
quoted
allocates memory which requires reclaim, vmscan gets this memory
by
quoted
quoted
shrinking the balloon, and then the driver adds the memory back
to
quoted
the
quoted
quoted
balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Adding some information for the record (if someone googles this thread):
In the VMware balloon driver, the shrinker is disabled by default since we
encountered a performance degradation in testing. I tried to avoid rapid
inflation/shrinker-deflation cycles by adding a timeout, but apparently it
did not help in avoiding the performance regression.
Thanks for that info. To me that sounds like the shrinker is the wrong
approach to "auto-deflation". It's not just "some slab cache".
So as you pointed out yourself deflate on oom is really under-specified.
I would be very happy if you could take a stub at documenting what's
expected from guest and how it could be used.
Please copy the virtio TC when you do this as this is spec stuff.
From: David Hildenbrand <hidden> Date: 2020-02-05 10:42:19
On 05.02.20 11:25, Michael S. Tsirkin wrote:
On Wed, Feb 05, 2020 at 10:58:14AM +0100, David Hildenbrand wrote:
quoted
On 05.02.20 10:49, Wang, Wei W wrote:
quoted
On Wednesday, February 5, 2020 5:37 PM, David Hildenbrand wrote:
quoted
quoted
Not sure how TCG tracks the dirty bits. But In whatever
implementation, the hypervisor should have
There is only a single bitmap for that purpose. (well, the one where KVM
syncs to)
quoted
already dealt with the race between he current round and the previous
round dirty recording.
quoted
(the race isn't brought by this feature essentially)
It is guaranteed to work reliably without this feature as you only clear what
*has been migrated*,
Not "clear what has been migrated" (that skips nothing..)
Anyway, it's a hint used for optimization.
Yes, an optimization that might easily lead to data corruption when the
two bitmaps are either not in place or don't play along in that specific
way (and I suspect this is the case under TCG).
So I checked and TCG has two copies too.
Each block has bmap used for migration and also dirty_memory
where pages are marked dirty. See cpu_physical_memory_sync_dirty_bitmap.
qemu_guest_free_page_hint() works on block->bmap.
cpu_physical_memory_set_dirty_range() works on ram_list.dirty_memory[i].
So you are right - sorry for the false alarm and thanks for verifying :)
[...]
Again a flag that tells guest it should wait until used
could be a reasonable expension. If we stick to the shrinker
it's actually implementable easily. With an OOM notifier - I'm not so
sure ...
See my other mail. I think we should keep handling just as is and not
overcomplicate things (especially in our implementation as you noted)
Instead, maybe abstract the reporting feature.
And a big part of the problem is that after all this time the page
hinting interfaces are still undocumented. Quite sad really :(
Yes, that was the source of my confusion ... the double-bitmap thingy is
non-obvious. And anybody who wants to implement that interface in a
hypervisor has to be aware that the race I explained has to be avoided
using e.g., two bitmaps and the sync.
--
Thanks,
David / dhildenb
From: David Hildenbrand <hidden> Date: 2020-02-05 10:44:00
On 05.02.20 11:27, Michael S. Tsirkin wrote:
On Wed, Feb 05, 2020 at 09:19:58AM +0100, David Hildenbrand wrote:
quoted
On 05.02.20 08:35, Nadav Amit wrote:
quoted
quoted
On Feb 3, 2020, at 2:50 PM, Nadav Amit [off-list ref] wrote:
quoted
On Feb 3, 2020, at 8:34 AM, David Hildenbrand [off-list ref] wrote:
On 03.02.20 17:18, Alexander Duyck wrote:
quoted
On Mon, 2020-02-03 at 08:11 -0500, Michael S. Tsirkin wrote:
quoted
On Thu, Jan 30, 2020 at 11:59:46AM -0800, Tyler Sanderson wrote:
quoted
On Thu, Jan 30, 2020 at 7:31 AM Wang, Wei W [off-list ref] wrote:
On Thursday, January 30, 2020 11:03 PM, David Hildenbrand wrote:
quoted
On 29.01.20 20:11, Tyler Sanderson wrote:
quoted
On Wed, Jan 29, 2020 at 2:31 AM David Hildenbrand <david@redhat.com
<mailto:david@redhat.com>> wrote:
On 29.01.20 01:22, Tyler Sanderson via Virtualization wrote:
quoted
A primary advantage of virtio balloon over other memory reclaim
mechanisms is that it can pressure the guest's page cache into
shrinking.
quoted
However, since the balloon driver changed to using the shrinker
use case has become a bit more tricky. I'm wondering what the
intended
quoted
quoted
device implementation is.
When inflating the balloon against page cache (i.e. no free
memory
quoted
quoted
quoted
remains) vmscan.c will both shrink page cache, but also invoke
the
quoted
quoted
quoted
shrinkers -- including the balloon's shrinker. So the balloon
driver
quoted
quoted
quoted
allocates memory which requires reclaim, vmscan gets this memory
by
quoted
quoted
shrinking the balloon, and then the driver adds the memory back
to
quoted
the
quoted
quoted
balloon. Basically a busy no-op.
Per my understanding, the balloon allocation won’t invoke shrinker as
__GFP_DIRECT_RECLAIM isn't set, no?
I could be wrong about the mechanism, but the device sees lots of activity on
the deflate queue. The balloon is being shrunk. And this only starts once all
free memory is depleted and we're inflating into page cache.
So given this looks like a regression, maybe we should revert the
patch in question 71994620bb25 ("virtio_balloon: replace oom notifier with shrinker")
Besides, with VIRTIO_BALLOON_F_FREE_PAGE_HINT
shrinker also ignores VIRTIO_BALLOON_F_MUST_TELL_HOST which isn't nice
at all.
So it looks like all this rework introduced more issues than it
addressed ...
I also CC Alex Duyck for an opinion on this.
Alex, what do you use to put pressure on page cache?
I would say reverting probably makes sense. I'm not sure there is much
value to having a shrinker running deflation when you are actively trying
to increase the balloon. It would make more sense to wait until you are
actually about to start hitting oom.
I think the shrinker makes sense for free page hinting feature
(everything on free_page_list).
So instead of only reverting, I think we should split it up and always
register the shrinker for VIRTIO_BALLOON_F_FREE_PAGE_HINT and the OOM
notifier (as before) for VIRTIO_BALLOON_F_MUST_TELL_HOST.
(Of course, adapting what is being done in the shrinker and in the OOM
notifier)
David,
Please keep me posted. I decided to adapt the same solution as the virtio
balloon for the VMware balloon. If the verdict is that this is damaging and
the OOM notifier should be used instead, I will submit patches to move to
OOM notifier as well.
Adding some information for the record (if someone googles this thread):
In the VMware balloon driver, the shrinker is disabled by default since we
encountered a performance degradation in testing. I tried to avoid rapid
inflation/shrinker-deflation cycles by adding a timeout, but apparently it
did not help in avoiding the performance regression.
Thanks for that info. To me that sounds like the shrinker is the wrong
approach to "auto-deflation". It's not just "some slab cache".
So as you pointed out yourself deflate on oom is really under-specified.
I would be very happy if you could take a stub at documenting what's
expected from guest and how it could be used.
Please copy the virtio TC when you do this as this is spec stuff.
So, I'll first get the code back into the desired state, so at least we
have an agreement of how it should be, and then follow up with a spec
update.
Might take some time, though (plenty of stuff to do).
--
Thanks,
David / dhildenb
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
On Wed, Feb 5, 2020 at 1:00 AM Wang, Wei W [off-list ref] wrote:
On Wednesday, February 5, 2020 4:57 PM, David Hildenbrand wrote:
quoted
quoted
quoted
Yes, I agree with you. Yet, I am thinking about one
(unlikely?impossible?) scenario. Can you refresh my brain why that
cannot happen (IOW, why we don't have to wait for the host to process
the request)?
1. Guest allocates a page and sends it to the host.
2. Shrinker gets active and releases that page again.
3. Some user in the guest allocates and modifies that page. After
that, it is done using that page for the next hour.
4. The host processes the request and clears the bit in the dirty
bitmap.
quoted
quoted
quoted
5. The guest is being migrated by the host. The modified page is not
being migrated.
Whenever the guest modifies a page during migration, it will be
captured by the dirty logging and the hypervisor will send the dirtied
the
quoted
page in the following round.
Please explain why the steps I outlined don't apply esp. in the last
round.
quoted
Your general statement does not explain why this race can't happen.
The guest is stopped in the last round, thus no page will be modified at
that time.
Isn't the hint only useful during the *first* round?
After the first round if a page becomes free then we need to update the
copy at the migration destination, so freeing a page that previously had
contents should mark it dirty.
On Tue, Feb 4, 2020 at 10:57 PM Michael S. Tsirkin [off-list ref] wrote:
On Tue, Feb 04, 2020 at 03:58:51PM -0800, Tyler Sanderson wrote:
quoted
> >
> > 1. It is last-resort, which means the system has already
gone through
quoted
> > heroics to prevent OOM. Those heroic reclaim efforts
are expensive
quoted
> > and impact application performance.
>
> That's *exactly* what "deflate on OOM" suggests.
>
>
> It seems there are some use cases where "deflate on OOM" is
desired and
quoted
> others where "deflate on pressure" is desired.
> This suggests adding a new feature bit "DEFLATE_ON_PRESSURE" that
> registers the shrinker, and reverting DEFLATE_ON_OOM to use the OOM
> notifier callback.
>
> This lets users configure the balloon for their use case.
You want the old behavior back, so why should we introduce a new
one? Or
quoted
am I missing something? (you did want us to revert to old handling,
no?)
quoted
Reverting actually doesn't help me because this has been the behavior
since
quoted
Linux 4.19 which is already widely in use. So my device implementation
needs to
quoted
handle the shrinker behavior anyways. I started this conversation to ask
what
quoted
the intended device implementation was.
I think there are reasonable device implementations that would prefer the
shrinker behavior (it turns out that mine doesn't).
For example, an implementation that slowly inflates the balloon for the
purpose
quoted
of memory overcommit. It might leave the balloon inflated and expect any
memory
quoted
pressure (including page cache usage) to deflate the balloon as a way to
dynamically right-size the balloon.
So just to make sure we understand, what exactly does your
implementation do?
My implementation is for the purposes of opportunistic memory overcommit.
We always want to give balloon memory back to the guest rather than causing
an OOM, so we use DEFLATE_ON_OOM.
We leave the balloon at size 0 while monitoring memory statistics reported
on the stats queue. When we see there is an opportunity for significant
savings then we inflate the balloon to a desired size (possibly including
pressuring the page cache), and then immediately deflate back to size 0.
The host pages backing the guest pages are unbacked during the inflation
process, so the memory footprint of the guest is smaller after this
inflate/deflate cycle.
quoted
Two reasons I didn't go with the above implementation:
1. I need to support guests before Linux 4.19 which don't have the
shrinker
quoted
behavior.
2. Memory in the balloon does not appear as "available" in /proc/meminfo
even
quoted
though it is freeable. This is confusing to users, but isn't a deal
breaker.
quoted
If we added a DEFLATE_ON_PRESSURE feature bit that indicated shrinker API
support then that would resolve reason #1 (ideally we would backport the
bit to
quoted
4.19).
We could declare lack of pagecache pressure with DEFLATE_ON_OOM a
regression and backport the revert but not I think the new
DEFLATE_ON_PRESSURE.
To be clear, the page cache can still be pressured. When the balloon driver
allocates memory and causes reclaim, some of that memory comes from the
balloon (bad) but some of that comes from the page cache (good).
quoted
In any case, the shrinker behavior when pressuring page cache is more of
an
quoted
inefficiency than a bug. It's not clear to me that it necessitates
reverting.
quoted
If there were/are reasons to be on the shrinker interface then I think
those
quoted
carry similar weight as the problem itself.
I consider virtio-balloon to this very day a big hack. And I don't
see
quoted
it getting better with new config knobs. Having that said, the
technologies that are candidates to replace it (free page reporting,
taming the guest page cache, etc.) are still not ready - so we'll
have
quoted
to stick with it for now :( .
>
> I'm actually not sure how you would safely do memory overcommit
without
quoted
> DEFLATE_ON_OOM. So I think it unlocks a huge use case.
Using better suited technologies that are not ready yet (well, some
form
quoted
of free page reporting is available under IBM z already but in a
proprietary form) ;) Anyhow, I remember that DEFLATE_ON_OOM only
makes
quoted
it less likely to crash your guest, but not that you are safe to
squeeze
quoted
the last bit out of your guest VM.
Can you elaborate on the danger of DEFLATE_ON_OOM? I haven't seen any
problems
quoted
in testing but I'd really like to know about the dangers.
Is there a difference in safety between the OOM notifier callback and the
shrinker API?
It's not about dangers as such. It's just that when linux hits OOM
all kind of error paths are being hit, latent bugs start triggering,
latency goes up drastically.
Doesn't this suggest that the shrinker is preferable to the OOM notifier in
the case that we're actually OOMing (with DEFLATE_ON_OOM)?
From: Wang, Wei W <hidden> Date: 2020-02-06 09:30:23
Isn't the hint only useful during the first round?
After the first round if a page becomes free then we need to update the copy at the migration destination, so freeing a page that previously had contents should mark it dirty.
Nope. I think as long as it is a free page (no matter 1st or 2nd round), we don’t need to send it.
Best,
Wei