* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-12 0:24 ` Jonathan Helman
0 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-12 0:24 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin
Cc: virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>> 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-12 0:24 ` Jonathan Helman
0 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-12 0:24 UTC (permalink / raw)
To: Jason Wang, Michael S. Tsirkin
Cc: Mihai Carabas, virtio-dev, linux-kernel, virtualization
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>> 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
>
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-12 0:24 ` Jonathan Helman
@ 2018-04-13 7:01 ` Jason Wang
-1 siblings, 0 replies; 41+ messages in thread
From: Jason Wang @ 2018-04-13 7:01 UTC (permalink / raw)
To: Jonathan Helman, Michael S. Tsirkin
Cc: virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com>
>>>>>>>> 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<jonathan.helman@oracle.com>
>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>> 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
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-13 7:01 ` Jason Wang
0 siblings, 0 replies; 41+ messages in thread
From: Jason Wang @ 2018-04-13 7:01 UTC (permalink / raw)
To: Jonathan Helman, Michael S. Tsirkin
Cc: virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com>
>>>>>>>> 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<jonathan.helman@oracle.com>
>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>> 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
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 7:01 ` Jason Wang
@ 2018-04-13 13:44 ` Michael S. Tsirkin
-1 siblings, 0 replies; 41+ messages in thread
From: Michael S. Tsirkin @ 2018-04-13 13:44 UTC (permalink / raw)
To: Jason Wang
Cc: Jonathan Helman, virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
> > > > > > > > > Reviewed-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > 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ý <tgolembi@redhat.com>
Date: Sun Nov 12 13:05:38 2017 +0100
virtio_balloon: include disk/file caches memory statistics
commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
Author: Igor Redko <redkoi@virtuozzo.com>
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.
--
MST
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-13 13:44 ` Michael S. Tsirkin
0 siblings, 0 replies; 41+ messages in thread
From: Michael S. Tsirkin @ 2018-04-13 13:44 UTC (permalink / raw)
To: Jason Wang
Cc: Jonathan Helman, virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
> > > > > > > > > Reviewed-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > 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ý <tgolembi@redhat.com>
Date: Sun Nov 12 13:05:38 2017 +0100
virtio_balloon: include disk/file caches memory statistics
commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
Author: Igor Redko <redkoi@virtuozzo.com>
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.
--
MST
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 13:44 ` Michael S. Tsirkin
@ 2018-04-13 17:10 ` Jonathan Helman
-1 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-13 17:10 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang
Cc: virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>> 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ý <tgolembi@redhat.com>
> Date: Sun Nov 12 13:05:38 2017 +0100
>
> virtio_balloon: include disk/file caches memory statistics
>
>
>
> commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
> Author: Igor Redko <redkoi@virtuozzo.com>
> 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
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-13 17:10 ` Jonathan Helman
0 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-13 17:10 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang
Cc: virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>> 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ý <tgolembi@redhat.com>
> Date: Sun Nov 12 13:05:38 2017 +0100
>
> virtio_balloon: include disk/file caches memory statistics
>
>
>
> commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
> Author: Igor Redko <redkoi@virtuozzo.com>
> 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
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 17:10 ` Jonathan Helman
@ 2018-04-13 21:51 ` Michael S. Tsirkin
-1 siblings, 0 replies; 41+ messages in thread
From: Michael S. Tsirkin @ 2018-04-13 21:51 UTC (permalink / raw)
To: Jonathan Helman
Cc: Jason Wang, virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
> > > > > > > > > > > Reviewed-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > > > 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ý <tgolembi@redhat.com>
> > Date: Sun Nov 12 13:05:38 2017 +0100
> >
> > virtio_balloon: include disk/file caches memory statistics
> >
> > commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
> > Author: Igor Redko <redkoi@virtuozzo.com>
> > 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
Whenever we update the headers.
--
MST
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-13 21:51 ` Michael S. Tsirkin
0 siblings, 0 replies; 41+ messages in thread
From: Michael S. Tsirkin @ 2018-04-13 21:51 UTC (permalink / raw)
To: Jonathan Helman
Cc: Jason Wang, virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
> > > > > > > > > > > Reviewed-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > > > 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ý <tgolembi@redhat.com>
> > Date: Sun Nov 12 13:05:38 2017 +0100
> >
> > virtio_balloon: include disk/file caches memory statistics
> >
> > commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
> > Author: Igor Redko <redkoi@virtuozzo.com>
> > 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
Whenever we update the headers.
--
MST
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 21:51 ` Michael S. Tsirkin
(?)
@ 2018-04-13 22:03 ` Jonathan Helman
-1 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-13 22:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>>>> 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ý <tgolembi@redhat.com>
>>> Date: Sun Nov 12 13:05:38 2017 +0100
>>>
>>> virtio_balloon: include disk/file caches memory statistics
>>>
>>> commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
>>> Author: Igor Redko <redkoi@virtuozzo.com>
>>> 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
>
> Whenever we update the headers.
>
Ah got it. Updating the headers without updating that string array
breaks things. Thanks.
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-13 22:03 ` Jonathan Helman
0 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-13 22:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, virtualization, linux-kernel, virtio-dev, Mihai Carabas
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>>>> 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ý <tgolembi@redhat.com>
>>> Date: Sun Nov 12 13:05:38 2017 +0100
>>>
>>> virtio_balloon: include disk/file caches memory statistics
>>>
>>> commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
>>> Author: Igor Redko <redkoi@virtuozzo.com>
>>> 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
>
> Whenever we update the headers.
>
Ah got it. Updating the headers without updating that string array
breaks things. Thanks.
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
@ 2018-04-13 22:03 ` Jonathan Helman
0 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-13 22:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mihai Carabas, virtio-dev, linux-kernel, virtualization
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>>>> 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ý <tgolembi@redhat.com>
>>> Date: Sun Nov 12 13:05:38 2017 +0100
>>>
>>> virtio_balloon: include disk/file caches memory statistics
>>>
>>> commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
>>> Author: Igor Redko <redkoi@virtuozzo.com>
>>> 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
>
> Whenever we update the headers.
>
Ah got it. Updating the headers without updating that string array
breaks things. Thanks.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 17:10 ` Jonathan Helman
(?)
(?)
@ 2018-04-13 21:51 ` Michael S. Tsirkin
-1 siblings, 0 replies; 41+ messages in thread
From: Michael S. Tsirkin @ 2018-04-13 21:51 UTC (permalink / raw)
To: Jonathan Helman; +Cc: Mihai Carabas, virtio-dev, linux-kernel, virtualization
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
> > > > > > > > > > > Reviewed-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > > > 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ý <tgolembi@redhat.com>
> > Date: Sun Nov 12 13:05:38 2017 +0100
> >
> > virtio_balloon: include disk/file caches memory statistics
> >
> > commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
> > Author: Igor Redko <redkoi@virtuozzo.com>
> > 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
Whenever we update the headers.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 13:44 ` Michael S. Tsirkin
(?)
(?)
@ 2018-04-13 17:10 ` Jonathan Helman
-1 siblings, 0 replies; 41+ messages in thread
From: Jonathan Helman @ 2018-04-13 17:10 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang
Cc: Mihai Carabas, virtio-dev, linux-kernel, virtualization
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
>>>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>>>> 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ý <tgolembi@redhat.com>
> Date: Sun Nov 12 13:05:38 2017 +0100
>
> virtio_balloon: include disk/file caches memory statistics
>
>
>
> commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
> Author: Igor Redko <redkoi@virtuozzo.com>
> 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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-13 7:01 ` Jason Wang
(?)
(?)
@ 2018-04-13 13:44 ` Michael S. Tsirkin
-1 siblings, 0 replies; 41+ messages in thread
From: Michael S. Tsirkin @ 2018-04-13 13:44 UTC (permalink / raw)
To: Jason Wang; +Cc: Mihai Carabas, virtio-dev, virtualization, linux-kernel
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<jasowang@redhat.com> 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<jonathan.helman@oracle.com>
> > > > > > > > > Reviewed-by: Jason Wang<jasowang@redhat.com>
> > > > > > > > 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ý <tgolembi@redhat.com>
Date: Sun Nov 12 13:05:38 2017 +0100
virtio_balloon: include disk/file caches memory statistics
commit 5057dcd0f1aaad57e07e728ba20a99e205c6b9de
Author: Igor Redko <redkoi@virtuozzo.com>
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.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [virtio-dev] Re: [PATCH v2] virtio_balloon: export hugetlb page allocation counts
2018-04-12 0:24 ` Jonathan Helman
` (2 preceding siblings ...)
(?)
@ 2018-04-13 7:01 ` Jason Wang
-1 siblings, 0 replies; 41+ messages in thread
From: Jason Wang @ 2018-04-13 7:01 UTC (permalink / raw)
To: Jonathan Helman, Michael S. Tsirkin
Cc: Mihai Carabas, virtio-dev, linux-kernel, virtualization
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<jasowang@redhat.com>
>>>>>>>> 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<jonathan.helman@oracle.com>
>>>>>>>> Reviewed-by: Jason Wang<jasowang@redhat.com>
>>>>>>> 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
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 41+ messages in thread