Disk caches in VirtIO Balloon driver stats

PATCH In review Tomáš Golembiovský 3 participants 8 messages Dec 5, 2017 – Feb 20, 2018

lore

Share & tools

lore

Tomáš Golembiovský

author

2017-12-05 12:14 UTC #0 lore raw

v2 cover Disk caches in VirtIO Balloon driver stats

This is a QEMU counterpart to the patch for VirtIO balloon driver posted here:

https://lists.nongnu.org/archive/html/qemu-devel/2017-11/msg02106.html

Tomáš Golembiovský (1):
virtio-balloon: include statistics of disk/file caches

 hw/virtio/virtio-balloon.c                      | 1 +
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

Signature · 6 lines

2.15.1


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Tomáš Golembiovský

author

2017-12-05 12:14 UTC #1 lore raw

PATCH v2 1/1 virtio-balloon: include statistics of disk/file caches

Signed-off-by Tomáš Golembiovský <[email protected]>

---
 hw/virtio/virtio-balloon.c                      | 1 +
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

hw/virtio/virtio-balloon.c +1 -0

index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
    [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
    [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
    [VIRTIO_BALLOON_S_NR] = NULL
 };

include/standard-headers/linux/virtio_balloon.h +2 -1

index 9d06ccd066..7b0a41b8fc 100644
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */
+#define VIRTIO_BALLOON_S_NR       8
 
 /*
  * Memory statistics structure.

Signature · 6 lines

2.15.1


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Tomáš Golembiovský

author

2018-02-05 12:08 UTC #2 lore raw

Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

ping

Quoted 35 lines · On Tue, 5 Dec 2017 13:14:46 +0100 Tomáš Golembiovský <[email protected]>…

Signed-off-by Tomáš Golembiovský <[email protected]>

---
 hw/virtio/virtio-balloon.c                      | 1 +
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

hw/virtio/virtio-balloon.c +1 -0

index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
    [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
    [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
    [VIRTIO_BALLOON_S_NR] = NULL
 };

include/standard-headers/linux/virtio_balloon.h +2 -1

index 9d06ccd066..7b0a41b8fc 100644
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */
+#define VIRTIO_BALLOON_S_NR       8
 
 /*
  * Memory statistics structure.

Signature · 1 line

2.15.1

Signature · 1 line

Tomáš Golembiovský <[email protected]>

Jonathan Helman

2018-02-13 20:29 UTC #3 lore raw

Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

Quoted 5 lines · Tomáš Golembiovský

ping
On Tue, 5 Dec 2017 13:14:46 +0100
Tomáš Golembiovský <[email protected]> wrote:

It would be good to include the corresponding upstream kernel change in the commit message. This would be similar to a previous change: https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg05555.html

Quoted 18 lines

Signed-off-by Tomáš Golembiovský <[email protected]>

---
  hw/virtio/virtio-balloon.c                      | 1 +
  include/standard-headers/linux/virtio_balloon.h | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)

hw/virtio/virtio-balloon.c +1 -0

index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
     [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
     [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
     [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
     [VIRTIO_BALLOON_S_NR] = NULL
  };

Quoted 10 lines

include/standard-headers/linux/virtio_balloon.h struct virtio_balloon_config { · line 55

Quoted 10 lines

include/standard-headers/linux/virtio_balloon.h +1 -1

index 9d06ccd066..7b0a41b8fc 100644
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */

I've been wondering, VIRTIO_BALLOON_S_AVAIL is not in the virtio spec (see Section 5.5.6.3). It seems like this header file needs to be in sync with the virtio spec in order to make this change.

I have a similar change to add a new statistic and was wondering this.

Quoted 9 lines

Quoted 7 lines
include/standard-headers/linux/virtio_balloon.h +1 -0
+#define VIRTIO_BALLOON_S_NR       8
 
  /*
   * Memory statistics structure.
Signature · 1 line
2.15.1

You need to add your new stat to the list of stats in docs/virtio-balloon-stats.txt.

Jon

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Michael S. Tsirkin

2018-02-13 22:08 UTC #4 lore raw

Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

Quoted 5 lines · Jonathan Helman

Quoted 2 lines · Tomáš Golembiovský
ping

Quoted 7 lines

Quoted 3 lines
On Tue, 5 Dec 2017 13:14:46 +0100
Tomáš Golembiovský <[email protected]> wrote:
It would be good to include the corresponding upstream kernel change in the commit message. This would be similar to a previous change:
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg05555.html

good idea, but this has been merged by now.

Quoted 32 lines

Quoted 19 lines
Quoted 19 lines
Signed-off-by Tomáš Golembiovský <[email protected]>
---
  hw/virtio/virtio-balloon.c                      | 1 +
  include/standard-headers/linux/virtio_balloon.h | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)
hw/virtio/virtio-balloon.c +1 -0
index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
     [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
     [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
     [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
     [VIRTIO_BALLOON_S_NR] = NULL
  };
include/standard-headers/linux/virtio_balloon.h +0 -0
index 9d06ccd066..7b0a41b8fc 100644
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h struct virtio_balloon_config { · line 55
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h +1 -1
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */
I've been wondering, VIRTIO_BALLOON_S_AVAIL is not in the virtio spec (see Section 5.5.6.3). It seems like this header file needs to be in sync with the virtio spec in order to make this change.

Quoted 1 line

I have a similar change to add a new statistic and was wondering this.

Absolutely. Tomáš?

Quoted 9 lines

Quoted 8 lines
Quoted 6 lines
include/standard-headers/linux/virtio_balloon.h +1 -0
+#define VIRTIO_BALLOON_S_NR       8
  /*
   * Memory statistics structure.
Signature · 1 line
2.15.1

Quoted 4 lines

You need to add your new stat to the list of stats in docs/virtio-balloon-stats.txt.
Jon

Can't hurt, I agree.

Signature · 5 lines

MST

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Tomáš Golembiovský

author

2018-02-19 13:11 UTC #5 lore raw

Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

Quoted 10 lines · On Wed, 14 Feb 2018 00:07:53 +0200 "Michael S. Tsirkin" <[email protected]> wrote:

Quoted 9 lines · Jonathan Helman
Quoted 5 lines · Tomáš Golembiovský
ping
On Tue, 5 Dec 2017 13:14:46 +0100
Tomáš Golembiovský <[email protected]> wrote:

Quoted 5 lines

Quoted 3 lines
It would be good to include the corresponding upstream kernel change in the commit message. This would be similar to a previous change:
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg05555.html
good idea, but this has been merged by now.

Maybe next time, sorry.

Quoted 32 lines

Quoted 32 lines
Quoted 19 lines
Quoted 19 lines
Signed-off-by Tomáš Golembiovský <[email protected]>
---
  hw/virtio/virtio-balloon.c                      | 1 +
  include/standard-headers/linux/virtio_balloon.h | 3 ++-
  2 files changed, 3 insertions(+), 1 deletion(-)
hw/virtio/virtio-balloon.c +1 -0
index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
     [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
     [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
     [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
     [VIRTIO_BALLOON_S_NR] = NULL
  };
include/standard-headers/linux/virtio_balloon.h +0 -0
index 9d06ccd066..7b0a41b8fc 100644
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h struct virtio_balloon_config { · line 55
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h +1 -1
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */  
I've been wondering, VIRTIO_BALLOON_S_AVAIL is not in the virtio spec (see Section 5.5.6.3). It seems like this header file needs to be in sync with the virtio spec in order to make this change.

Quoted 3 lines

Quoted 1 line
I have a similar change to add a new statistic and was wondering this.
Absolutely. Tomáš?

Sent. I also included description for the VIRTIO_BALLOON_S_AVAIL that was missing.

Quoted 9 lines

Quoted 9 lines
Quoted 8 lines
Quoted 6 lines
include/standard-headers/linux/virtio_balloon.h +1 -0
+#define VIRTIO_BALLOON_S_NR       8
  /*
   * Memory statistics structure.
Signature · 1 line
2.15.1

Quoted 6 lines

Quoted 4 lines
You need to add your new stat to the list of stats in docs/virtio-balloon-stats.txt.
Jon
Can't hurt, I agree.

Sent.

Tomas

Signature · 5 lines

Tomáš Golembiovský <[email protected]>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Jonathan Helman

2018-02-20 05:11 UTC #6 lore raw

Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

Quoted 74 lines · Tomáš Golembiovský

Quoted 10 lines · On Wed, 14 Feb 2018 00:07:53 +0200 "Michael S. Tsirkin" <[email protected]> wrote:
Quoted 9 lines · Jonathan Helman
Quoted 5 lines · Tomáš Golembiovský
ping
On Tue, 5 Dec 2017 13:14:46 +0100
Tomáš Golembiovský <[email protected]> wrote:
Quoted 5 lines
Quoted 3 lines
It would be good to include the corresponding upstream kernel change in the commit message. This would be similar to a previous change:
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg05555.html
good idea, but this has been merged by now.
Maybe next time, sorry.
Quoted 32 lines
Quoted 32 lines
Quoted 19 lines
Quoted 19 lines
Signed-off-by Tomáš Golembiovský <[email protected]>
---
 hw/virtio/virtio-balloon.c                      | 1 +
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)
hw/virtio/virtio-balloon.c +1 -0
index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
    [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
    [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
    [VIRTIO_BALLOON_S_NR] = NULL
 };
include/standard-headers/linux/virtio_balloon.h +0 -0
index 9d06ccd066..7b0a41b8fc 100644
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h struct virtio_balloon_config { · line 55
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h +1 -1
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */  
I've been wondering, VIRTIO_BALLOON_S_AVAIL is not in the virtio spec (see Section 5.5.6.3). It seems like this header file needs to be in sync with the virtio spec in order to make this change.
Quoted 3 lines
Quoted 1 line
I have a similar change to add a new statistic and was wondering this.
Absolutely. Tomáš?
Sent. I also included description for the VIRTIO_BALLOON_S_AVAIL that was missing.
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
Quoted 6 lines
include/standard-headers/linux/virtio_balloon.h +1 -0
+#define VIRTIO_BALLOON_S_NR       8
 /*
  * Memory statistics structure.
Signature · 1 line
2.15.1

Quoted 9 lines

Quoted 6 lines
Quoted 4 lines
You need to add your new stat to the list of stats in docs/virtio-balloon-stats.txt.
Jon
Can't hurt, I agree.
Sent.

Awesome, thank you.

Do you also plan to make the commensurate changes in libvirt, or did I just miss the patch?

Jon

Quoted 4 lines

Tomas
Signature · 1 line
Tomáš Golembiovský <[email protected]>

Tomáš Golembiovský

author

2018-02-20 11:15 UTC #7 lore raw

Re: [Qemu-devel] [PATCH v2 1/1] virtio-balloon: include statistics of disk/file caches

Quoted 78 lines · On Mon, 19 Feb 2018 21:11:27 -0800 Jonathan Helman…

Quoted 78 lines · Tomáš Golembiovský
Quoted 10 lines · On Wed, 14 Feb 2018 00:07:53 +0200 "Michael S. Tsirkin" <[email protected]> wrote:
Quoted 9 lines · Jonathan Helman
Quoted 5 lines · Tomáš Golembiovský
ping
On Tue, 5 Dec 2017 13:14:46 +0100
Tomáš Golembiovský <[email protected]> wrote:
Quoted 5 lines
Quoted 3 lines
It would be good to include the corresponding upstream kernel change in the commit message. This would be similar to a previous change:
https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg05555.html
good idea, but this has been merged by now.
Maybe next time, sorry.
Quoted 32 lines
Quoted 32 lines
Quoted 19 lines
Quoted 19 lines
Signed-off-by Tomáš Golembiovský <[email protected]>
---
 hw/virtio/virtio-balloon.c                      | 1 +
 include/standard-headers/linux/virtio_balloon.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)
hw/virtio/virtio-balloon.c +1 -0
index 37cde38982..8141326a51 100644
@@ -50,6 +50,7 @@ static const char *balloon_stat_names[] = {
    [VIRTIO_BALLOON_S_MEMFREE] = "stat-free-memory",
    [VIRTIO_BALLOON_S_MEMTOT] = "stat-total-memory",
    [VIRTIO_BALLOON_S_AVAIL] = "stat-available-memory",
+   [VIRTIO_BALLOON_S_CACHES] = "stat-disk-caches",
    [VIRTIO_BALLOON_S_NR] = NULL
 };
include/standard-headers/linux/virtio_balloon.h +0 -0
index 9d06ccd066..7b0a41b8fc 100644
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h struct virtio_balloon_config { · line 55
Quoted 8 lines
include/standard-headers/linux/virtio_balloon.h +1 -1
@@ -52,7 +52,8 @@ 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_CACHES   7   /* Disk caches */    
I've been wondering, VIRTIO_BALLOON_S_AVAIL is not in the virtio spec (see Section 5.5.6.3). It seems like this header file needs to be in sync with the virtio spec in order to make this change.
Quoted 3 lines
Quoted 1 line
I have a similar change to add a new statistic and was wondering this.
Absolutely. Tomáš?
Sent. I also included description for the VIRTIO_BALLOON_S_AVAIL that was missing.
Quoted 14 lines
Quoted 13 lines
Quoted 8 lines
Quoted 6 lines
include/standard-headers/linux/virtio_balloon.h +1 -0
+#define VIRTIO_BALLOON_S_NR       8
 /*
  * Memory statistics structure.
Signature · 1 line
2.15.1
You need to add your new stat to the list of stats in docs/virtio-balloon-stats.txt.
Jon

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Can't hurt, I agree.
Sent.
Awesome, thank you.
Do you also plan to make the commensurate changes in libvirt, or did I just miss the patch?

Yes, I do. I just didn't get to it yet.

Tomas

Quoted 8 lines

Jon
Quoted 4 lines
Tomas
Signature · 1 line
Tomáš Golembiovský <[email protected]>  

Signature · 1 line

Tomáš Golembiovský <[email protected]>