[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/1] virtio_balloon: include buffers and cach
From: |
Tomáš Golembiovský |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics |
Date: |
Fri, 3 Nov 2017 12:27:14 +0100 |
On Tue, 31 Oct 2017 18:15:48 +0200
"Michael S. Tsirkin" <address@hidden> wrote:
> On Tue, Oct 31, 2017 at 01:20:19PM +0100, Tomáš Golembiovský wrote:
> > ping
> >
> > +Gil, +Amnon... could you maybe aid in reviewing the patch, please?
> >
> >
> > Tomas
> >
> > On Sun, 22 Oct 2017 20:05:57 +0200
> > Tomáš Golembiovský <address@hidden> wrote:
> >
> > > On Thu, 19 Oct 2017 16:12:20 +0300
> > > "Michael S. Tsirkin" <address@hidden> wrote:
> > >
> > > > On Thu, Sep 21, 2017 at 02:55:41PM +0200, Tomáš Golembiovský wrote:
> > > > > Add a new fields, VIRTIO_BALLOON_S_BUFFERS and
> > > > > VIRTIO_BALLOON_S_CACHED,
> > > > > to virtio_balloon memory statistics protocol. The values correspond to
> > > > > 'Buffers' and 'Cached' in /proc/meminfo.
> > > > >
> > > > > To be able to compute the value of 'Cached' memory it is necessary to
> > > > > export total_swapcache_pages() to modules.
> > > > >
> > > > > Signed-off-by: Tomáš Golembiovský <address@hidden>
> > > >
> > > > Does 'Buffers' actually make sense? It's a temporary storage -
> > > > wouldn't it be significantly out of date by the time
> > > > host receives it?
> > >
> > > That would be best answered by somebody from kernel. But my personal
> > > opinion is that it would not be out of date. The amount of memory
> > > dedicated to Buffers does not seem to fluctuate too much.
> > >
> > > Tomas
> > >
>
> I would be inclined to say, just report
> global_node_page_state(NR_FILE_PAGES).
> Maybe subtract buffer ram.
>
> It's not clear host cares about the distinction,
> it's all memory that can shrink in response to
> memory pressure such as inflating the balloon.
So in procfs terms we'd be sending sum Cached+SwapCahced.
Martin, would that be good enough?
I wonder whether it would still make sense to send Buffers as a separate
value though. Maybe we should forget about having some granularity here
and just report all the disk caches as one value.
Tomas
>
> This statistic is portable as well I think, most guests have
> storage cache.
>
>
> > > > > ---
> > > > > drivers/virtio/virtio_balloon.c | 11 +++++++++++
> > > > > include/uapi/linux/virtio_balloon.h | 4 +++-
> > > > > mm/swap_state.c | 1 +
> > > > > 3 files changed, 15 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/virtio/virtio_balloon.c
> > > > > b/drivers/virtio/virtio_balloon.c
> > > > > index f0b3a0b9d42f..c2558ec47a62 100644
> > > > > --- a/drivers/virtio/virtio_balloon.c
> > > > > +++ b/drivers/virtio/virtio_balloon.c
> > > > > @@ -244,12 +244,19 @@ static unsigned int update_balloon_stats(struct
> > > > > virtio_balloon *vb)
> > > > > struct sysinfo i;
> > > > > unsigned int idx = 0;
> > > > > long available;
> > > > > + long cached;
> > > > >
> > > > > all_vm_events(events);
> > > > > si_meminfo(&i);
> > > > >
> > > > > available = si_mem_available();
> > > > >
> > > > > + cached = global_node_page_state(NR_FILE_PAGES) -
> > > > > + total_swapcache_pages() - i.bufferram;
> > > > > + if (cached < 0)
> > > > > + cached = 0;
> > > > > +
> > > > > +
> > > > > #ifdef CONFIG_VM_EVENT_COUNTERS
> > > > > update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
> > > > > pages_to_bytes(events[PSWPIN]));
> > > > > @@ -264,6 +271,10 @@ static unsigned int update_balloon_stats(struct
> > > > > virtio_balloon *vb)
> > > > > pages_to_bytes(i.totalram));
> > > > > update_stat(vb, idx++, VIRTIO_BALLOON_S_AVAIL,
> > > > > pages_to_bytes(available));
> > > > > + update_stat(vb, idx++, VIRTIO_BALLOON_S_BUFFERS,
> > > > > + pages_to_bytes(i.bufferram));
> > > > > + update_stat(vb, idx++, VIRTIO_BALLOON_S_CACHED,
> > > > > + pages_to_bytes(cached));
> > > > >
> > > > > return idx;
> > > > > }
> > > > > diff --git a/include/uapi/linux/virtio_balloon.h
> > > > > b/include/uapi/linux/virtio_balloon.h
> > > > > index 343d7ddefe04..d5dc8a56a497 100644
> > > > > --- a/include/uapi/linux/virtio_balloon.h
> > > > > +++ b/include/uapi/linux/virtio_balloon.h
> > > > > @@ -52,7 +52,9 @@ struct virtio_balloon_config {
> > > > > #define VIRTIO_BALLOON_S_MEMFREE 4 /* Total amount of free memory
> > > > > */
> > > > > #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
> > > > > #define VIRTIO_BALLOON_S_AVAIL 6 /* Available memory as in
> > > > > /proc */
> > > > > -#define VIRTIO_BALLOON_S_NR 7
> > > > > +#define VIRTIO_BALLOON_S_BUFFERS 7 /* Buffers memory as in /proc
> > > > > */
> > > > > +#define VIRTIO_BALLOON_S_CACHED 8 /* Cached memory as in /proc */
> > > > > +#define VIRTIO_BALLOON_S_NR 9
> > > > >
> > > > > /*
> > > > > * Memory statistics structure.
> > > > > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > > > > index 71ce2d1ccbf7..f3a4ff7d6c52 100644
> > > > > --- a/mm/swap_state.c
> > > > > +++ b/mm/swap_state.c
> > > > > @@ -95,6 +95,7 @@ unsigned long total_swapcache_pages(void)
> > > > > rcu_read_unlock();
> > > > > return ret;
> > > > > }
> > > > > +EXPORT_SYMBOL_GPL(total_swapcache_pages);
> > > > >
> > > > > static atomic_t swapin_readahead_hits = ATOMIC_INIT(4);
> > > >
> > > > Need an ack from MM crowd on that.
> > > >
> > > > > --
> > > > > 2.14.1
> > >
> > >
> > > --
> > > Tomáš Golembiovský <address@hidden>
> >
> >
> > --
> > Tomáš Golembiovský <address@hidden>
--
Tomáš Golembiovský <address@hidden>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics,
Tomáš Golembiovský <=