* [PATCH] xen/balloon: export balloon hotplug page stats to sysfs
@ 2014-03-20 12:56 Wei Liu
2014-03-20 13:21 ` David Vrabel
2014-03-20 19:38 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 6+ messages in thread
From: Wei Liu @ 2014-03-20 12:56 UTC (permalink / raw)
To: xen-devel; +Cc: Boris Ostrovsky, Wei Liu, David Vrabel
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
drivers/xen/xen-balloon.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c
index e555845..2d4c5a8 100644
--- a/drivers/xen/xen-balloon.c
+++ b/drivers/xen/xen-balloon.c
@@ -129,6 +129,11 @@ module_exit(balloon_exit);
BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
+#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
+BALLOON_SHOW(hotplug_kb, "%lu\n", PAGES2KB(balloon_stats.hotplug_pages));
+BALLOON_SHOW(balloon_hotplug_kb, "%lu\n",
+ PAGES2KB(balloon_stats.balloon_hotplug));
+#endif
static DEVICE_ULONG_ATTR(schedule_delay, 0444, balloon_stats.schedule_delay);
static DEVICE_ULONG_ATTR(max_schedule_delay, 0644, balloon_stats.max_schedule_delay);
@@ -206,6 +211,10 @@ static struct attribute *balloon_info_attrs[] = {
&dev_attr_current_kb.attr,
&dev_attr_low_kb.attr,
&dev_attr_high_kb.attr,
+#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
+ &dev_attr_hotplug_kb.attr,
+ &dev_attr_balloon_hotplug_kb.attr,
+#endif
NULL
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/balloon: export balloon hotplug page stats to sysfs
2014-03-20 12:56 [PATCH] xen/balloon: export balloon hotplug page stats to sysfs Wei Liu
@ 2014-03-20 13:21 ` David Vrabel
2014-03-20 14:01 ` Wei Liu
2014-03-20 19:38 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 6+ messages in thread
From: David Vrabel @ 2014-03-20 13:21 UTC (permalink / raw)
To: Wei Liu; +Cc: Boris Ostrovsky, xen-devel
On 20/03/14 12:56, Wei Liu wrote:
> --- a/drivers/xen/xen-balloon.c
> +++ b/drivers/xen/xen-balloon.c
> @@ -129,6 +129,11 @@ module_exit(balloon_exit);
> BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
> BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
> BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> +BALLOON_SHOW(hotplug_kb, "%lu\n", PAGES2KB(balloon_stats.hotplug_pages));
> +BALLOON_SHOW(balloon_hotplug_kb, "%lu\n",
> + PAGES2KB(balloon_stats.balloon_hotplug));
> +#endif
Extending the kernel's ABI requires justification. How are these values
useful for userspace tools? If it's just for debug purposes, using
debugfs would be preferred.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/balloon: export balloon hotplug page stats to sysfs
2014-03-20 13:21 ` David Vrabel
@ 2014-03-20 14:01 ` Wei Liu
2014-03-21 13:02 ` David Vrabel
0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2014-03-20 14:01 UTC (permalink / raw)
To: David Vrabel; +Cc: Boris Ostrovsky, Wei Liu, xen-devel
On Thu, Mar 20, 2014 at 01:21:17PM +0000, David Vrabel wrote:
> On 20/03/14 12:56, Wei Liu wrote:
> > --- a/drivers/xen/xen-balloon.c
> > +++ b/drivers/xen/xen-balloon.c
> > @@ -129,6 +129,11 @@ module_exit(balloon_exit);
> > BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
> > BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
> > BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
> > +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> > +BALLOON_SHOW(hotplug_kb, "%lu\n", PAGES2KB(balloon_stats.hotplug_pages));
> > +BALLOON_SHOW(balloon_hotplug_kb, "%lu\n",
> > + PAGES2KB(balloon_stats.balloon_hotplug));
> > +#endif
>
> Extending the kernel's ABI requires justification. How are these values
> useful for userspace tools? If it's just for debug purposes, using
> debugfs would be preferred.
>
My original thought is that userspace tools have the right to know how
many pages balloon driver has. Missing the hotplug pages is not very
desirable. But I have yet known any tools that require balloon hotplug
page stat -- after all they probably don't know there's such a thing is
nothing is exported. Also note that these BALLOON_SHOW fields are
read-only, so exporting more stats won't in any way affect system
behavior.
If you prefer debugfs I can switch to use that.
Wei.
> David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/balloon: export balloon hotplug page stats to sysfs
2014-03-20 12:56 [PATCH] xen/balloon: export balloon hotplug page stats to sysfs Wei Liu
2014-03-20 13:21 ` David Vrabel
@ 2014-03-20 19:38 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-03-20 19:38 UTC (permalink / raw)
To: Wei Liu; +Cc: Boris Ostrovsky, David Vrabel, xen-devel
On Thu, Mar 20, 2014 at 12:56:53PM +0000, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Konrad Wilk <konrad.wilk@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> drivers/xen/xen-balloon.c | 9 +++++++++
If you are doing that you also need to put information in
Documentation/sysfs/abi/something/somewhere.
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c
> index e555845..2d4c5a8 100644
> --- a/drivers/xen/xen-balloon.c
> +++ b/drivers/xen/xen-balloon.c
> @@ -129,6 +129,11 @@ module_exit(balloon_exit);
> BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
> BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
> BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> +BALLOON_SHOW(hotplug_kb, "%lu\n", PAGES2KB(balloon_stats.hotplug_pages));
> +BALLOON_SHOW(balloon_hotplug_kb, "%lu\n",
> + PAGES2KB(balloon_stats.balloon_hotplug));
> +#endif
>
> static DEVICE_ULONG_ATTR(schedule_delay, 0444, balloon_stats.schedule_delay);
> static DEVICE_ULONG_ATTR(max_schedule_delay, 0644, balloon_stats.max_schedule_delay);
> @@ -206,6 +211,10 @@ static struct attribute *balloon_info_attrs[] = {
> &dev_attr_current_kb.attr,
> &dev_attr_low_kb.attr,
> &dev_attr_high_kb.attr,
> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> + &dev_attr_hotplug_kb.attr,
> + &dev_attr_balloon_hotplug_kb.attr,
> +#endif
> NULL
> };
>
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/balloon: export balloon hotplug page stats to sysfs
2014-03-20 14:01 ` Wei Liu
@ 2014-03-21 13:02 ` David Vrabel
2014-03-21 13:34 ` Wei Liu
0 siblings, 1 reply; 6+ messages in thread
From: David Vrabel @ 2014-03-21 13:02 UTC (permalink / raw)
To: Wei Liu; +Cc: Boris Ostrovsky, xen-devel
On 20/03/14 14:01, Wei Liu wrote:
> On Thu, Mar 20, 2014 at 01:21:17PM +0000, David Vrabel wrote:
>> On 20/03/14 12:56, Wei Liu wrote:
>>> --- a/drivers/xen/xen-balloon.c
>>> +++ b/drivers/xen/xen-balloon.c
>>> @@ -129,6 +129,11 @@ module_exit(balloon_exit);
>>> BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
>>> BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
>>> BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
>>> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
>>> +BALLOON_SHOW(hotplug_kb, "%lu\n", PAGES2KB(balloon_stats.hotplug_pages));
>>> +BALLOON_SHOW(balloon_hotplug_kb, "%lu\n",
>>> + PAGES2KB(balloon_stats.balloon_hotplug));
>>> +#endif
>>
>> Extending the kernel's ABI requires justification. How are these values
>> useful for userspace tools? If it's just for debug purposes, using
>> debugfs would be preferred.
>>
>
> My original thought is that userspace tools have the right to know how
> many pages balloon driver has. Missing the hotplug pages is not very
> desirable.
What tools needs the value and how will it use it to make sensible
decisions?
If there isn't an answer and it just for diagnostics/debug then debugfs
is the right place.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/balloon: export balloon hotplug page stats to sysfs
2014-03-21 13:02 ` David Vrabel
@ 2014-03-21 13:34 ` Wei Liu
0 siblings, 0 replies; 6+ messages in thread
From: Wei Liu @ 2014-03-21 13:34 UTC (permalink / raw)
To: David Vrabel; +Cc: Boris Ostrovsky, Wei Liu, xen-devel
On Fri, Mar 21, 2014 at 01:02:26PM +0000, David Vrabel wrote:
> On 20/03/14 14:01, Wei Liu wrote:
> > On Thu, Mar 20, 2014 at 01:21:17PM +0000, David Vrabel wrote:
> >> On 20/03/14 12:56, Wei Liu wrote:
> >>> --- a/drivers/xen/xen-balloon.c
> >>> +++ b/drivers/xen/xen-balloon.c
> >>> @@ -129,6 +129,11 @@ module_exit(balloon_exit);
> >>> BALLOON_SHOW(current_kb, "%lu\n", PAGES2KB(balloon_stats.current_pages));
> >>> BALLOON_SHOW(low_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_low));
> >>> BALLOON_SHOW(high_kb, "%lu\n", PAGES2KB(balloon_stats.balloon_high));
> >>> +#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
> >>> +BALLOON_SHOW(hotplug_kb, "%lu\n", PAGES2KB(balloon_stats.hotplug_pages));
> >>> +BALLOON_SHOW(balloon_hotplug_kb, "%lu\n",
> >>> + PAGES2KB(balloon_stats.balloon_hotplug));
> >>> +#endif
> >>
> >> Extending the kernel's ABI requires justification. How are these values
> >> useful for userspace tools? If it's just for debug purposes, using
> >> debugfs would be preferred.
> >>
> >
> > My original thought is that userspace tools have the right to know how
> > many pages balloon driver has. Missing the hotplug pages is not very
> > desirable.
>
> What tools needs the value and how will it use it to make sensible
> decisions?
>
I see this issue the other way around. Without this information tools
cannot make correct decision, because it cannot possibly know there even
exists hotplug pages in balloon driver. Adding this is not for the sake of
existing tool, but for the sake of correctness. If you think userspace
will never have the need to know this information then debugfs it is.
> If there isn't an answer and it just for diagnostics/debug then debugfs
> is the right place.
>
> David
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-21 13:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-20 12:56 [PATCH] xen/balloon: export balloon hotplug page stats to sysfs Wei Liu
2014-03-20 13:21 ` David Vrabel
2014-03-20 14:01 ` Wei Liu
2014-03-21 13:02 ` David Vrabel
2014-03-21 13:34 ` Wei Liu
2014-03-20 19:38 ` Konrad Rzeszutek Wilk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.