From: KOSAKI Motohiro <[email protected]>
commit 2244b95a7b (zoned vm counters: basic ZVC (zoned vm counter)
implementation) broke protection column. It is a part of "pages"
attribute. but not it is showed after vmstats column.
This patch restores the right position.
<before>
pages free 3965
min 32
low 40
high 48
scanned 0
spanned 4080
present 3909
(snip)
numa_local 1
numa_other 0
nr_anon_transparent_hugepages 0
protection: (0, 3512, 7867, 7867)
<after>
pages free 3965
min 32
low 40
high 48
scanned 0
spanned 4080
present 3909
protection: (0, 3504, 7851, 7851)
nr_free_pages 3965
nr_inactive_anon 0
Cc: Christoph Lameter <[email protected]>
Signed-off-by: KOSAKI Motohiro <[email protected]>
---
mm/vmstat.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 1bbbbd9..9f5f2a9 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -987,19 +987,18 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
zone->pages_scanned,
zone->spanned_pages,
zone->present_pages);
-
- for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++)
- seq_printf(m, "\n %-12s %lu", vmstat_text[i],
- zone_page_state(zone, i));
-
seq_printf(m,
"\n protection: (%lu",
zone->lowmem_reserve[0]);
for (i = 1; i < ARRAY_SIZE(zone->lowmem_reserve); i++)
seq_printf(m, ", %lu", zone->lowmem_reserve[i]);
- seq_printf(m,
- ")"
- "\n pagesets");
+ seq_printf(m, ")");
+
+ for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++)
+ seq_printf(m, "\n %-12s %lu", vmstat_text[i],
+ zone_page_state(zone, i));
+
+ seq_printf(m, "\n pagesets");
for_each_online_cpu(i) {
struct per_cpu_pageset *pageset;
--
1.7.1
On Mon, 11 Jun 2012, [email protected] wrote:
> From: KOSAKI Motohiro <[email protected]>
>
> commit 2244b95a7b (zoned vm counters: basic ZVC (zoned vm counter)
> implementation) broke protection column. It is a part of "pages"
> attribute. but not it is showed after vmstats column.
>
> This patch restores the right position.
Well this reorders the output. vmstats are also counts of pages. I am not
sure what the difference is.
You are not worried about breaking something that may scan the zoneinfo
output with this change? Its been this way for 6 years and its likely that
tools expect the current layout.
On 6/11/2012 10:02 AM, Christoph Lameter wrote:
> On Mon, 11 Jun 2012, [email protected] wrote:
>
>> From: KOSAKI Motohiro <[email protected]>
>>
>> commit 2244b95a7b (zoned vm counters: basic ZVC (zoned vm counter)
>> implementation) broke protection column. It is a part of "pages"
>> attribute. but not it is showed after vmstats column.
>>
>> This patch restores the right position.
>
> Well this reorders the output. vmstats are also counts of pages. I am not
> sure what the difference is.
No. In this case, "pages" mean zone attribute. In the other hand, vmevent
is a statistics.
> You are not worried about breaking something that may scan the zoneinfo
> output with this change? Its been this way for 6 years and its likely that
> tools expect the current layout.
I don't worry about this. Because of, /proc/zoneinfo is cray machine unfrinedly
format and afaik no application uses it.
btw, I believe we should aim /sys/devices/system/node/<node-num>/zones new directory
and export zone infos as machine readable format.
On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
> On 6/11/2012 10:02 AM, Christoph Lameter wrote:
> > On Mon, 11 Jun 2012, [email protected] wrote:
> >
> >> From: KOSAKI Motohiro <[email protected]>
> >>
> >> commit 2244b95a7b (zoned vm counters: basic ZVC (zoned vm counter)
> >> implementation) broke protection column. It is a part of "pages"
> >> attribute. but not it is showed after vmstats column.
> >>
> >> This patch restores the right position.
> >
> > Well this reorders the output. vmstats are also counts of pages. I am not
> > sure what the difference is.
>
> No. In this case, "pages" mean zone attribute. In the other hand, vmevent
> is a statistics.
The vmevent countes are something different from the zone counters. Event
counters are indeed statistics only but the numbers here were intended
to be are actual counts of pages. Well some of them like the numa_XXX are
stats you are right. Those could be moved off the ZVCs and become event
counters.
> > You are not worried about breaking something that may scan the zoneinfo
> > output with this change? Its been this way for 6 years and its likely that
> > tools expect the current layout.
>
> I don't worry about this. Because of, /proc/zoneinfo is cray machine unfrinedly
> format and afaik no application uses it.
Cray? What does that have to do with it.
> btw, I believe we should aim /sys/devices/system/node/<node-num>/zones new directory
> and export zone infos as machine readable format.
Yes that would be a good thing.
On Mon, Jun 11, 2012 at 10:40 AM, Christoph Lameter <[email protected]> wrote:
> On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
>
>> On 6/11/2012 10:02 AM, Christoph Lameter wrote:
>> > On Mon, 11 Jun 2012, [email protected] wrote:
>> >
>> >> From: KOSAKI Motohiro <[email protected]>
>> >>
>> >> commit 2244b95a7b (zoned vm counters: basic ZVC (zoned vm counter)
>> >> implementation) broke protection column. It is a part of "pages"
>> >> attribute. but not it is showed after vmstats column.
>> >>
>> >> This patch restores the right position.
>> >
>> > Well this reorders the output. vmstats are also counts of pages. I am not
>> > sure what the difference is.
>>
>> No. In this case, "pages" mean zone attribute. In the other hand, vmevent
>> is a statistics.
>
> The vmevent countes are something different from the zone counters. Event
> counters are indeed statistics only but the numbers here were intended
> to be are actual counts of pages. Well some of them like the numa_XXX are
> stats you are right. Those could be moved off the ZVCs and become event
> counters.
>
>> > You are not worried about breaking something that may scan the zoneinfo
>> > output with this change? Its been this way for 6 years and its likely that
>> > tools expect the current layout.
>>
>> I don't worry about this. Because of, /proc/zoneinfo is cray machine unfrinedly
>> format and afaik no application uses it.
>
> Cray? What does that have to do with it.
sorry. s/cray/crazy/
>
>> btw, I believe we should aim /sys/devices/system/node/<node-num>/zones new directory
>> and export zone infos as machine readable format.
>
> Yes that would be a good thing.
On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
> > You are not worried about breaking something that may scan the zoneinfo
> > output with this change? Its been this way for 6 years and its likely that
> > tools expect the current layout.
>
> I don't worry about this. Because of, /proc/zoneinfo is cray machine unfrinedly
> format and afaik no application uses it.
>
We do, and I think it would be a shame to break anything parsing the way
that this file has been written for the past several years for something
as aesthetical as this.
On Mon, Jun 11, 2012 at 4:37 PM, David Rientjes <[email protected]> wrote:
> On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
>
>> > You are not worried about breaking something that may scan the zoneinfo
>> > output with this change? Its been this way for 6 years and its likely that
>> > tools expect the current layout.
>>
>> I don't worry about this. Because of, /proc/zoneinfo is cray machine unfrinedly
>> format and afaik no application uses it.
>>
>
> We do, and I think it would be a shame to break anything parsing the way
> that this file has been written for the past several years for something
> as aesthetical as this.
How do you parsing?
Several years, some one added ZVC stat. therefore, hardcoded line
number parsing never work anyway. And in the other hand, if you are
parsing, field
name, my patch doesn't break anything.
On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
> > We do, and I think it would be a shame to break anything parsing the way
> > that this file has been written for the past several years for something
> > as aesthetical as this.
>
> How do you parsing?
>
> Several years, some one added ZVC stat. therefore, hardcoded line
> number parsing never work anyway. And in the other hand, if you are
> parsing, field
> name, my patch doesn't break anything.
>
Yeah, your patch doesn't break me because I'm parsing by field name but I
feel it would be a shame for it to break anyone else that may not be doing
it that way. The set of users in the world who are parsing /proc/zoneinfo
who may or may not do crazy things is not fully represented on this
thread, so I don't feel it's worth it.
On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
> Several years, some one added ZVC stat. therefore, hardcoded line
You are talking to the "some one".... The aim at that point was not the
beauty of the output but the scaling of the counter operations. There was
no intention in placing things a certain way. I'd be fine with changes as
long as we are sure that they do not break anything.
(6/11/12 5:04 PM), Christoph Lameter wrote:
> On Mon, 11 Jun 2012, KOSAKI Motohiro wrote:
>
>> Several years, some one added ZVC stat. therefore, hardcoded line
>
> You are talking to the "some one".... The aim at that point was not the
> beauty of the output but the scaling of the counter operations. There was
> no intention in placing things a certain way. I'd be fine with changes as
> long as we are sure that they do not break anything.
Maybe my english was poor. I didn't talk about your change at last mail. I
talked about some new counters like nr_anon_transparent_hugepages. Hardcoded
linenuber assumption was break multiple times already. therefore, I don't
think line number change causes application breakage.