thr3ads.net - Linux Virtualization - [PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics [Oct 2017]

If this information is useful, please help other people find it:
Share via:

Tomáš Golembiovský

2017-Oct-31 12:20 UTC

[PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

ping

+Gil, +Amnon... could you maybe aid in reviewing the patch, please?


    Tomas

On Sun, 22 Oct 2017 20:05:57 +0200
Tom?? Golembiovsk? <tgolembi at redhat.com> wrote:
> On Thu, 19 Oct 2017 16:12:20 +0300
> "Michael S. Tsirkin" <mst at redhat.com> 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? <tgolembi at redhat.com>
> > 
> > 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
> 
> 
> > > ---
> > >  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? <tgolembi at redhat.com>

-- 
Tom?? Golembiovsk? <tgolembi at redhat.com>

Michael S. Tsirkin

2017-Oct-31 16:15 UTC

head link

[PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

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? <tgolembi at redhat.com> wrote:
> 
> > On Thu, 19 Oct 2017 16:12:20 +0300
> > "Michael S. Tsirkin" <mst at redhat.com> 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? <tgolembi at
redhat.com>
> > > 
> > > 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.

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? <tgolembi at redhat.com>
> 
> 
> -- 
> Tom?? Golembiovsk? <tgolembi at redhat.com>

Tomáš Golembiovský

2017-Nov-03 11:27 UTC

head link

[PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

On Tue, 31 Oct 2017 18:15:48 +0200
"Michael S. Tsirkin" <mst at redhat.com> 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? <tgolembi at redhat.com> wrote:
> > 
> > > On Thu, 19 Oct 2017 16:12:20 +0300
> > > "Michael S. Tsirkin" <mst at redhat.com> 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? <tgolembi at
redhat.com>
> > > > 
> > > > 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? <tgolembi at redhat.com>
> > 
> > 
> > -- 
> > Tom?? Golembiovsk? <tgolembi at redhat.com>

-- 
Tom?? Golembiovsk? <tgolembi at redhat.com>

Maybe Matching Threads

Search for more maybe matching threads

Linux Virtualization - Oct 2017 - [PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

[PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

[PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

[PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics

Maybe Matching Threads