Re: [PATCH] virtio_balloon: add array of stat names
On 04/13/2018 03:07 PM, Michael S. Tsirkin wrote:
On Fri, Apr 13, 2018 at 11:53:31AM -0700, Jonathan Helman wrote:
On 04/13/2018 06:44 AM, Michael S. Tsirkin wrote:
Jason Wang points out that it's vary hard for users to build an array of
s/vary/very
stat names. The naive thing is to use VIRTIO_BALLOON_S_NR but that
breaks if we add more stats.
Let's add an array of reasonably readable names.
Thanks for doing this, this is goodness. Clients of this interface will now
only require a modification to their internal copy of the header file (if
they have one), rather than updating their string array as well.
Fixes: 6c64fe7f2 ("virtio_balloon: export hugetlb page allocation counts")
I think this is a tad bit confusing since the only way you'd know why this
patch "fixes" 6c64fe7f2 is by reading the LKML archives and finding Jason's
comment.
So
... add more stats as recently by commit 6c64fe7f2 ("virtio_balloon: export hugetlb
page allocation counts")
Sure. Thanks.
Jon
Cc: Jason Wang
Cc: Jonathan Helman , > Signed-off-by: Michael S. Tsirkin
Reviewed-by: Jonathan Helman
---
include/uapi/linux/virtio_balloon.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 9e02137..1477c17 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -64,6 +64,21 @@ struct virtio_balloon_config {
#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation
failures */
#define VIRTIO_BALLOON_S_NR 10
+#define VIRTIO_BALLOON_S_NAMES_WITH_PREFIX(VIRTIO_BALLOON_S_NAMES_prefix) { \
+ VIRTIO_BALLOON_S_NAMES_prefix "swap-in", \
+ VIRTIO_BALLOON_S_NAMES_prefix "swap-out", \
+ VIRTIO_BALLOON_S_NAMES_prefix "major-faults", \
+ VIRTIO_BALLOON_S_NAMES_prefix "minor-faults", \
+ VIRTIO_BALLOON_S_NAMES_prefix "free-memory", \
+ VIRTIO_BALLOON_S_NAMES_prefix "total-memory", \
+ VIRTIO_BALLOON_S_NAMES_prefix "available-memory", \
+ VIRTIO_BALLOON_S_NAMES_prefix "disk-caches", \
+ VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-allocations", \
+ VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-failures" \ > +}
+
+#define VIRTIO_BALLOON_S_NAMES VIRTIO_BALLOON_S_NAMES_WITH_PREFIX("")
+
/*
* Memory statistics structure.
* Driver fills an array of these structures and passes to device.
Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
On 04/13/2018 02:51 PM, Michael S. Tsirkin wrote:
On Fri, Apr 13, 2018 at 10:10:57AM -0700, Jonathan Helman wrote:
On 04/13/2018 06:44 AM, Michael S. Tsirkin wrote:
On Fri, Apr 13, 2018 at 03:01:11PM +0800, Jason Wang wrote:
On 2018年04月12日 08:24, Jonathan Helman wrote:
On 04/10/2018 08:12 PM, Jason Wang wrote:
On 2018年04月10日 05:11, Jonathan Helman wrote:
On 03/22/2018 07:38 PM, Jason Wang wrote:
On 2018年03月22日 11:10, Michael S. Tsirkin wrote:
On Thu, Mar 22, 2018 at 09:52:18AM +0800, Jason Wang wrote:
On 2018年03月20日 12:26, Jonathan Helman wrote:
On Mar 19, 2018, at 7:31 PM, Jason
Wang wrote:
On 2018年03月20日 06:14, Jonathan Helman wrote:
Export the number of successful and failed hugetlb page
allocations via the virtio balloon driver. These 2 counts
come directly from the vm_events HTLB_BUDDY_PGALLOC and
HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
Reviewed-by: Jason Wang
Thanks.
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git
a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int
update_balloon_stats(struct
virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++,
VIRTIO_BALLOON_S_MAJFLT,
events[PGMAJFAULT]);
update_stat(vb, idx++,
VIRTIO_BALLOON_S_MINFLT,
events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git
a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..40297a3 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#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_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC
8 /* Hugetlb page allocations */
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL
9 /* Hugetlb page allocation failures
*/
+#define VIRTIO_BALLOON_S_NR 10
/*
* Memory statistics structure.
Not for this patch, but it looks to me that
exporting such nr through uapi is fragile.
Sorry, can you explain what you mean here?
Jon
Spec said "Within an output buffer submitted to the
statsq, the device MUST
ignore entries with tag values that it does not
recognize". So exporting
VIRTIO_BALLOON_S_NR seems useless and device
implementation can not depend
on such number in uapi.
Thanks
Suggestions? I don't like to break build for people ...
Didn't have a good idea. But maybe we should keep
VIRTIO_BALLOON_S_NR unchanged, and add a comment here.
Thanks
I think Jason's comment is for a future patch. Didn't see this
patch get applied, so wondering if it could be.
Thanks,
Jon
Hi Jon:
Have you tested new driver with old qemu?
Yes, this testing scenario looks good. Thanks.
Jon
Hi Jon:
I mean e.g compiling qemu with new linux headers. E.g current qemu has:
static const char *balloon_stat_names[] = {
[VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
[VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
[VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
[VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
[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
};
I'm afraid it will be broken if VIRTIO_BALLOON_S_NR is 10.
Thanks
Well it is handy for sizing arrays and this isn't the first time we did
this:
commit 4d32029b8ddb7be4d1699c6d8e1675ff5476d149
Author: Tomáš Golembiovský
Date: Sun Nov 12 13:05:38 2017 +0100
virtio_balloon: include disk/file caches memory statistics
commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
Author: Igor Redko
Date: Thu Mar 17 14:19:08 2016 -0700
virtio_balloon: export 'available' memory to balloon statistics
how about we give QEMU a hand and just put the list of names in
the header?
I posted a patch like that, pls review.
Sorry, maybe I'm missing something. We have an internal copy of the header
file in qemu under include/standard-headers/linux/virtio_balloon.h, which
hw/virtio/virtio-balloon.c includes (
Re: [PATCH] virtio_balloon: add array of stat names
On 04/13/2018 06:44 AM, Michael S. Tsirkin wrote:
Jason Wang points out that it's vary hard for users to build an array of
s/vary/very
stat names. The naive thing is to use VIRTIO_BALLOON_S_NR but that
breaks if we add more stats.
Let's add an array of reasonably readable names.
Thanks for doing this, this is goodness. Clients of this interface will
now only require a modification to their internal copy of the header
file (if they have one), rather than updating their string array as well.
Fixes: 6c64fe7f2 ("virtio_balloon: export hugetlb page allocation counts")
I think this is a tad bit confusing since the only way you'd know why
this patch "fixes" 6c64fe7f2 is by reading the LKML archives and finding
Jason's comment.
Cc: Jason Wang
Cc: Jonathan Helman , > Signed-off-by: Michael S. Tsirkin
Reviewed-by: Jonathan Helman
---
include/uapi/linux/virtio_balloon.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 9e02137..1477c17 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -64,6 +64,21 @@ struct virtio_balloon_config {
#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation failures
*/
#define VIRTIO_BALLOON_S_NR 10
+#define VIRTIO_BALLOON_S_NAMES_WITH_PREFIX(VIRTIO_BALLOON_S_NAMES_prefix) { \
+ VIRTIO_BALLOON_S_NAMES_prefix "swap-in", \
+ VIRTIO_BALLOON_S_NAMES_prefix "swap-out", \
+ VIRTIO_BALLOON_S_NAMES_prefix "major-faults", \
+ VIRTIO_BALLOON_S_NAMES_prefix "minor-faults", \
+ VIRTIO_BALLOON_S_NAMES_prefix "free-memory", \
+ VIRTIO_BALLOON_S_NAMES_prefix "total-memory", \
+ VIRTIO_BALLOON_S_NAMES_prefix "available-memory", \
+ VIRTIO_BALLOON_S_NAMES_prefix "disk-caches", \
+ VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-allocations", \
+ VIRTIO_BALLOON_S_NAMES_prefix "hugetlb-failures" \ > +}
+
+#define VIRTIO_BALLOON_S_NAMES VIRTIO_BALLOON_S_NAMES_WITH_PREFIX("")
+
/*
* Memory statistics structure.
* Driver fills an array of these structures and passes to device.
Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
On 04/13/2018 06:44 AM, Michael S. Tsirkin wrote:
On Fri, Apr 13, 2018 at 03:01:11PM +0800, Jason Wang wrote:
On 2018年04月12日 08:24, Jonathan Helman wrote:
On 04/10/2018 08:12 PM, Jason Wang wrote:
On 2018年04月10日 05:11, Jonathan Helman wrote:
On 03/22/2018 07:38 PM, Jason Wang wrote:
On 2018年03月22日 11:10, Michael S. Tsirkin wrote:
On Thu, Mar 22, 2018 at 09:52:18AM +0800, Jason Wang wrote:
On 2018年03月20日 12:26, Jonathan Helman wrote:
On Mar 19, 2018, at 7:31 PM, Jason
Wang wrote:
On 2018年03月20日 06:14, Jonathan Helman wrote:
Export the number of successful and failed hugetlb page
allocations via the virtio balloon driver. These 2 counts
come directly from the vm_events HTLB_BUDDY_PGALLOC and
HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
Reviewed-by: Jason Wang
Thanks.
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git
a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int
update_balloon_stats(struct
virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++,
VIRTIO_BALLOON_S_MAJFLT,
events[PGMAJFAULT]);
update_stat(vb, idx++,
VIRTIO_BALLOON_S_MINFLT,
events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git
a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..40297a3 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#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_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC
8 /* Hugetlb page allocations */
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL
9 /* Hugetlb page allocation failures
*/
+#define VIRTIO_BALLOON_S_NR 10
/*
* Memory statistics structure.
Not for this patch, but it looks to me that
exporting such nr through uapi is fragile.
Sorry, can you explain what you mean here?
Jon
Spec said "Within an output buffer submitted to the
statsq, the device MUST
ignore entries with tag values that it does not
recognize". So exporting
VIRTIO_BALLOON_S_NR seems useless and device
implementation can not depend
on such number in uapi.
Thanks
Suggestions? I don't like to break build for people ...
Didn't have a good idea. But maybe we should keep
VIRTIO_BALLOON_S_NR unchanged, and add a comment here.
Thanks
I think Jason's comment is for a future patch. Didn't see this
patch get applied, so wondering if it could be.
Thanks,
Jon
Hi Jon:
Have you tested new driver with old qemu?
Yes, this testing scenario looks good. Thanks.
Jon
Hi Jon:
I mean e.g compiling qemu with new linux headers. E.g current qemu has:
static const char *balloon_stat_names[] = {
[VIRTIO_BALLOON_S_SWAP_IN] = "stat-swap-in",
[VIRTIO_BALLOON_S_SWAP_OUT] = "stat-swap-out",
[VIRTIO_BALLOON_S_MAJFLT] = "stat-major-faults",
[VIRTIO_BALLOON_S_MINFLT] = "stat-minor-faults",
[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
};
I'm afraid it will be broken if VIRTIO_BALLOON_S_NR is 10.
Thanks
Well it is handy for sizing arrays and this isn't the first time we did
this:
commit 4d32029b8ddb7be4d1699c6d8e1675ff5476d149
Author: Tomáš Golembiovský
Date: Sun Nov 12 13:05:38 2017 +0100
virtio_balloon: include disk/file caches memory statistics
commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
Author: Igor Redko
Date: Thu Mar 17 14:19:08 2016 -0700
virtio_balloon: export 'available' memory to balloon statistics
how about we give QEMU a hand and just put the list of names in
the header?
I posted a patch like that, pls review.
Sorry, maybe I'm missing something. We have an internal copy of the
header file in qemu under
include/standard-headers/linux/virtio_balloon.h, which
hw/virtio/virtio-balloon.c includes (through including
hw/virtio/virtio-balloon.h). I thought qemu would use internal header
files so that it could be compiled on any Linux kernel version. So when
would we ever have the issue Jason is discussing?
Thanks,
Jon
Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
On 04/10/2018 08:12 PM, Jason Wang wrote:
On 2018年04月10日 05:11, Jonathan Helman wrote:
On 03/22/2018 07:38 PM, Jason Wang wrote:
On 2018年03月22日 11:10, Michael S. Tsirkin wrote:
On Thu, Mar 22, 2018 at 09:52:18AM +0800, Jason Wang wrote:
On 2018年03月20日 12:26, Jonathan Helman wrote:
On Mar 19, 2018, at 7:31 PM, Jason Wang wrote:
On 2018年03月20日 06:14, Jonathan Helman wrote:
Export the number of successful and failed hugetlb page
allocations via the virtio balloon driver. These 2 counts
come directly from the vm_events HTLB_BUDDY_PGALLOC and
HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
Reviewed-by: Jason Wang
Thanks.
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int
update_balloon_stats(struct virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT,
events[PGMAJFAULT]);
update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT,
events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..40297a3 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#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_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Hugetlb page
allocations */
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page
allocation failures */
+#define VIRTIO_BALLOON_S_NR 10
/*
* Memory statistics structure.
Not for this patch, but it looks to me that exporting such nr
through uapi is fragile.
Sorry, can you explain what you mean here?
Jon
Spec said "Within an output buffer submitted to the statsq, the
device MUST
ignore entries with tag values that it does not recognize". So
exporting
VIRTIO_BALLOON_S_NR seems useless and device implementation can not
depend
on such number in uapi.
Thanks
Suggestions? I don't like to break build for people ...
Didn't have a good idea. But maybe we should keep VIRTIO_BALLOON_S_NR
unchanged, and add a comment here.
Thanks
I think Jason's comment is for a future patch. Didn't see this patch
get applied, so wondering if it could be.
Thanks,
Jon
Hi Jon:
Have you tested new driver with old qemu?
Yes, this testing scenario looks good. Thanks.
Jon
Thanks
Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
On 03/22/2018 07:38 PM, Jason Wang wrote:
On 2018年03月22日 11:10, Michael S. Tsirkin wrote:
On Thu, Mar 22, 2018 at 09:52:18AM +0800, Jason Wang wrote:
On 2018年03月20日 12:26, Jonathan Helman wrote:
On Mar 19, 2018, at 7:31 PM, Jason Wang wrote:
On 2018年03月20日 06:14, Jonathan Helman wrote:
Export the number of successful and failed hugetlb page
allocations via the virtio balloon driver. These 2 counts
come directly from the vm_events HTLB_BUDDY_PGALLOC and
HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
Reviewed-by: Jason Wang
Thanks.
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c
b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int
update_balloon_stats(struct virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT,
events[PGMAJFAULT]);
update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT,
events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..40297a3 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#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_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Hugetlb page
allocations */
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page
allocation failures */
+#define VIRTIO_BALLOON_S_NR 10
/*
* Memory statistics structure.
Not for this patch, but it looks to me that exporting such nr
through uapi is fragile.
Sorry, can you explain what you mean here?
Jon
Spec said "Within an output buffer submitted to the statsq, the
device MUST
ignore entries with tag values that it does not recognize". So exporting
VIRTIO_BALLOON_S_NR seems useless and device implementation can not
depend
on such number in uapi.
Thanks
Suggestions? I don't like to break build for people ...
Didn't have a good idea. But maybe we should keep VIRTIO_BALLOON_S_NR
unchanged, and add a comment here.
Thanks
I think Jason's comment is for a future patch. Didn't see this patch get
applied, so wondering if it could be.
Thanks,
Jon
Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
> On Mar 19, 2018, at 7:31 PM, Jason Wang wrote:
>
>
>
> On 2018年03月20日 06:14, Jonathan Helman wrote:
>> Export the number of successful and failed hugetlb page
>> allocations via the virtio balloon driver. These 2 counts
>> come directly from the vm_events HTLB_BUDDY_PGALLOC and
>> HTLB_BUDDY_PGALLOC_FAIL.
>>
>> Signed-off-by: Jonathan Helman
>
> Reviewed-by: Jason Wang
Thanks.
>
>> ---
>> drivers/virtio/virtio_balloon.c | 6 ++
>> include/uapi/linux/virtio_balloon.h | 4 +++-
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c
>> b/drivers/virtio/virtio_balloon.c
>> index dfe5684..6b237e3 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -272,6 +272,12 @@ static unsigned int update_balloon_stats(struct
>> virtio_balloon *vb)
>> pages_to_bytes(events[PSWPOUT]));
>> update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
>> update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
>> +#ifdef CONFIG_HUGETLB_PAGE
>> +update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
>> +events[HTLB_BUDDY_PGALLOC]);
>> +update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
>> +events[HTLB_BUDDY_PGALLOC_FAIL]);
>> +#endif
>> #endif
>> update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
>> pages_to_bytes(i.freeram));
>> diff --git a/include/uapi/linux/virtio_balloon.h
>> b/include/uapi/linux/virtio_balloon.h
>> index 4e8b830..40297a3 100644
>> --- a/include/uapi/linux/virtio_balloon.h
>> +++ b/include/uapi/linux/virtio_balloon.h
>> @@ -53,7 +53,9 @@ struct virtio_balloon_config {
>> #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
>> #define VIRTIO_BALLOON_S_AVAIL6 /* Available memory as in /proc */
>> #define VIRTIO_BALLOON_S_CACHES 7 /* Disk caches */
>> -#define VIRTIO_BALLOON_S_NR 8
>> +#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Hugetlb page allocations */
>> +#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation
>> failures */
>> +#define VIRTIO_BALLOON_S_NR 10
>>/*
>> * Memory statistics structure.
>
> Not for this patch, but it looks to me that exporting such nr through uapi is
> fragile.
Sorry, can you explain what you mean here?
Jon
>
> Thanks
[PATCH v2] virtio_balloon: export hugetlb page allocation counts
Export the number of successful and failed hugetlb page
allocations via the virtio balloon driver. These 2 counts
come directly from the vm_events HTLB_BUDDY_PGALLOC and
HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int update_balloon_stats(struct
virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..40297a3 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
#define VIRTIO_BALLOON_S_AVAIL6 /* Available memory as in /proc */
#define VIRTIO_BALLOON_S_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Hugetlb page allocations */
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation failures
*/
+#define VIRTIO_BALLOON_S_NR 10
/*
* Memory statistics structure.
--
1.8.3.1
Re: [PATCH] virtio_balloon: export huge page allocation statistics
On 02/27/2018 07:20 AM, Michael S. Tsirkin wrote:
On Fri, Feb 16, 2018 at 09:44:32PM -0800, Jonathan Helman wrote:
Export statistics for successful and failed huge page allocations
from the virtio balloon driver. These 2 stats come directly from
the vm_events HTLB_BUDDY_PGALLOC and HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
Any host/guest intergace changes need to be copied to
the virtio TC mailing list (subscriber-only, sorry about that):
virtio-...@lists.oasis-open.org
Sorry, will do on v2.
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int update_balloon_stats(struct
virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..e3e8071 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
#define VIRTIO_BALLOON_S_AVAIL6 /* Available memory as in /proc */
#define VIRTIO_BALLOON_S_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Number of htlb pgalloc successes
*/
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Number of htlb pgalloc failures */
+#define VIRTIO_BALLOON_S_NR 10
Can you clarify the comments pls? Eschew abbreviation.
Sure. How about the following instead? Hoping it makes things clearer,
without going over the 80 character limit ;)
#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Hugetlb page allocations */
#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Hugetlb page allocation
failures */
If this is better, I'll send out a v2.
Thanks,
Jon
/*
* Memory statistics structure.
--
1.8.3.1
[PATCH] virtio_balloon: export huge page allocation statistics
Export statistics for successful and failed huge page allocations
from the virtio balloon driver. These 2 stats come directly from
the vm_events HTLB_BUDDY_PGALLOC and HTLB_BUDDY_PGALLOC_FAIL.
Signed-off-by: Jonathan Helman
---
drivers/virtio/virtio_balloon.c | 6 ++
include/uapi/linux/virtio_balloon.h | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index dfe5684..6b237e3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -272,6 +272,12 @@ static unsigned int update_balloon_stats(struct
virtio_balloon *vb)
pages_to_bytes(events[PSWPOUT]));
update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+#ifdef CONFIG_HUGETLB_PAGE
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGALLOC,
+ events[HTLB_BUDDY_PGALLOC]);
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_HTLB_PGFAIL,
+ events[HTLB_BUDDY_PGALLOC_FAIL]);
+#endif
#endif
update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
pages_to_bytes(i.freeram));
diff --git a/include/uapi/linux/virtio_balloon.h
b/include/uapi/linux/virtio_balloon.h
index 4e8b830..e3e8071 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -53,7 +53,9 @@ struct virtio_balloon_config {
#define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
#define VIRTIO_BALLOON_S_AVAIL6 /* Available memory as in /proc */
#define VIRTIO_BALLOON_S_CACHES 7 /* Disk caches */
-#define VIRTIO_BALLOON_S_NR 8
+#define VIRTIO_BALLOON_S_HTLB_PGALLOC 8 /* Number of htlb pgalloc successes
*/
+#define VIRTIO_BALLOON_S_HTLB_PGFAIL 9 /* Number of htlb pgalloc failures */
+#define VIRTIO_BALLOON_S_NR 10
/*
* Memory statistics structure.
--
1.8.3.1
