Always account inflated memory as used for both cases - with and
without deflate on oom. Do not change total ram which can confuse
userspace and users.
Signed-off-by: Alexander Atanasov <[email protected]>
---
drivers/virtio/virtio_balloon.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 97d3b29cb9f1..fa6ddec45fc4 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -244,9 +244,6 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
- if (!virtio_has_feature(vb->vdev,
- VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
- adjust_managed_page_count(page, -1);
vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE;
}
@@ -265,9 +262,6 @@ static void release_pages_balloon(struct virtio_balloon *vb,
struct page *page, *next;
list_for_each_entry_safe(page, next, pages, lru) {
- if (!virtio_has_feature(vb->vdev,
- VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
- adjust_managed_page_count(page, 1);
list_del(&page->lru);
put_page(page); /* balloon reference */
}
@@ -750,12 +744,9 @@ static void report_free_page_func(struct work_struct *work)
static int virtio_balloon_debug_show(struct seq_file *f, void *offset)
{
struct virtio_balloon *vb = f->private;
- s64 num_pages = vb->num_pages << (VIRTIO_BALLOON_PFN_SHIFT - 10);
+ u64 num_pages = vb->num_pages << (VIRTIO_BALLOON_PFN_SHIFT - 10);
- if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
- num_pages = -num_pages;
-
- seq_printf(f, "inflated: %lld kB\n", num_pages);
+ seq_printf(f, "inflated: %llu kB\n", num_pages);
return 0;
}
--
2.25.1
On 26.07.22 16:10, Alexander Atanasov wrote:
> Always account inflated memory as used for both cases - with and
> without deflate on oom. Do not change total ram which can confuse
> userspace and users.
Sorry, but NAK.
This would affect existing users / user space / balloon stats. For example
HV just recently switch to properly using adjust_managed_page_count()
commit d1df458cbfdb0c3384c03c7fbcb1689bc02a746c
Author: Vitaly Kuznetsov <[email protected]>
Date: Wed Dec 2 17:12:45 2020 +0100
hv_balloon: do adjust_managed_page_count() when ballooning/un-ballooning
Unlike virtio_balloon/virtio_mem/xen balloon drivers, Hyper-V balloon driver
does not adjust managed pages count when ballooning/un-ballooning and this leads
to incorrect stats being reported, e.g. unexpected 'free' output.
Note, the calculation in post_status() seems to remain correct: ballooned out
pages are never 'available' and we manually add dm->num_pages_ballooned to
'commited'.
--
Thanks,
David / dhildenb
On Tue, Jul 26, 2022 at 02:10:47PM +0000, Alexander Atanasov wrote:
> Always account inflated memory as used for both cases - with and
> without deflate on oom. Do not change total ram which can confuse
> userspace and users.
>
> Signed-off-by: Alexander Atanasov <[email protected]>
I only noticed this patch accidentally since it looked like
part of discussion on an older patch.
Please do not do this, each version should be its own thread,
if you want to link to previous discussion provide a
cover letter and do it there.
> ---
> drivers/virtio/virtio_balloon.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 97d3b29cb9f1..fa6ddec45fc4 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -244,9 +244,6 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>
> set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> - if (!virtio_has_feature(vb->vdev,
> - VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> - adjust_managed_page_count(page, -1);
> vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
>
> @@ -265,9 +262,6 @@ static void release_pages_balloon(struct virtio_balloon *vb,
> struct page *page, *next;
>
> list_for_each_entry_safe(page, next, pages, lru) {
> - if (!virtio_has_feature(vb->vdev,
> - VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> - adjust_managed_page_count(page, 1);
> list_del(&page->lru);
> put_page(page); /* balloon reference */
> }
We adjusted total ram with balloon usage for many years. I don't think
we can just drop this accounting using "can confuse userspace" as a
justification - any userspace that's confused by this has been confused
since approximately forever.
> @@ -750,12 +744,9 @@ static void report_free_page_func(struct work_struct *work)
> static int virtio_balloon_debug_show(struct seq_file *f, void *offset)
> {
> struct virtio_balloon *vb = f->private;
> - s64 num_pages = vb->num_pages << (VIRTIO_BALLOON_PFN_SHIFT - 10);
> + u64 num_pages = vb->num_pages << (VIRTIO_BALLOON_PFN_SHIFT - 10);
>
> - if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> - num_pages = -num_pages;
> -
> - seq_printf(f, "inflated: %lld kB\n", num_pages);
> + seq_printf(f, "inflated: %llu kB\n", num_pages);
>
> return 0;
I don't much like it when patch 1 adds code only for patch 2 to drop it.
> }
> --
> 2.25.1