2008-06-05 23:28:16

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 0/3 v2] per-task-delay-accounting: add memory reclaim delay

Hi.

This is v2 of accounting memory reclaim patch series.
Thanks to Kosaki-san, Kamezawa-san, Andrew for comments and advice!
These patches were fixed about the following.

Against: next-20080605

1) Change Log

o Add accounting memory reclaim delay from memcgroup.
For accounting both global and cgroup memory reclaim,
accounting point was moved from try_to_free_pages() to do_try_to_free_pages.

o Drop the patch regarding /proc export for memory reclaim delay.
Because it seems that two separate ways to report are not necessary,
this patch series supports only NETLINK and doesn't add a field to /proc/<pid>/stat.


2) Confirm the fix regarding memcgroup.

o Previous patch can't catch memory reclaim delay from memcgroup.

$ echo 10M > /cgroups/0/memory.limit_in_bytes

$ ls -s test.dat
500496 test.dat

$ time tar cvf test.tar test.dat
real 0m21.957s
user 0m0.032s
sys 0m2.348s

$ ./delayget -d -p <pid>
CPU count real total virtual total delay total
2441 2288143000 2438256954 22371958
IO count delay total
2444 18745251314
SWAP count delay total
0 0
RECLAIM count delay total
0 0

o Current patch can catch memory reclaim delay from memcgroup.

$ echo 10M > /cgroups/0/memory.limit_in_bytes

$ ls -s test.dat
500496 test.dat

$ time tar cvf test.tar test.dat
real 0m22.563s
user 0m0.028s
sys 0m2.440s

$ ./delayget -d -p <pid>
CPU count real total virtual total delay total
2640 2456153500 2478353004 28366219
IO count delay total
2628 19894214188
SWAP count delay total
0 0
RECLAIM count delay total
6600 10682486085


2008-06-05 23:31:23

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 1/3 v2] per-task-delay-accounting: add memory reclaim delay

Sometimes, application responses become bad under heavy memory load.
Applications take a bit time to reclaim memory.
The statistics, how long memory reclaim takes, will be useful to
measure memory usage.

This patch adds accounting memory reclaim to per-task-delay-accounting
for accounting the time of do_try_to_free_pages().

<i.e>

- When System is under low memory load,
memory reclaim may not occur.

$ free
total used free shared buffers cached
Mem: 8197800 1577300 6620500 0 4808 1516724
-/+ buffers/cache: 55768 8142032
Swap: 16386292 0 16386292

$ vmstat 1
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
0 0 0 5069748 10612 3014060 0 0 0 0 3 26 0 0 100 0
0 0 0 5069748 10612 3014060 0 0 0 0 4 22 0 0 100 0
0 0 0 5069748 10612 3014060 0 0 0 0 3 18 0 0 100 0

Measure the time of tar command.

$ ls -s test.dat
1501472 test.dat

$ time tar cvf test.tar test.dat
real 0m13.388s
user 0m0.116s
sys 0m5.304s

$ ./delayget -d -p <pid>
CPU count real total virtual total delay total
428 5528345500 5477116080 62749891
IO count delay total
338 8078977189
SWAP count delay total
0 0
RECLAIM count delay total
0 0

- When system is under heavy memory load
memory reclaim may occur.

$ vmstat 1
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
0 0 7159032 49724 1812 3012 0 0 0 0 3 24 0 0 100 0
0 0 7159032 49724 1812 3012 0 0 0 0 4 24 0 0 100 0
0 0 7159032 49848 1812 3012 0 0 0 0 3 22 0 0 100 0

In this case, one process uses more 8G memory
by execution of malloc() and memset().

$ time tar cvf test.tar test.dat
real 1m38.563s <- increased by 85 sec
user 0m0.140s
sys 0m7.060s

$ ./delayget -d -p <pid>
CPU count real total virtual total delay total
9021 7140446250 7315277975 923201824
IO count delay total
8965 90466349669
SWAP count delay total
3 21036367
RECLAIM count delay total
740 61011951153

In the later case, the value of RECLAIM is increasing.
So, taskstats can show how much memory reclaim influences TAT.

Signed-off-by: Keika Kobayashi <[email protected]>
---
include/linux/delayacct.h | 19 +++++++++++++++++++
include/linux/sched.h | 4 ++++
kernel/delayacct.c | 13 +++++++++++++
mm/vmscan.c | 5 +++++
4 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/include/linux/delayacct.h b/include/linux/delayacct.h
index ab94bc0..f352f06 100644
--- a/include/linux/delayacct.h
+++ b/include/linux/delayacct.h
@@ -39,6 +39,8 @@ extern void __delayacct_blkio_start(void);
extern void __delayacct_blkio_end(void);
extern int __delayacct_add_tsk(struct taskstats *, struct task_struct *);
extern __u64 __delayacct_blkio_ticks(struct task_struct *);
+extern void __delayacct_freepages_start(void);
+extern void __delayacct_freepages_end(void);

static inline int delayacct_is_task_waiting_on_io(struct task_struct *p)
{
@@ -107,6 +109,18 @@ static inline __u64 delayacct_blkio_ticks(struct task_struct *tsk)
return 0;
}

+static inline void delayacct_freepages_start(void)
+{
+ if (current->delays)
+ __delayacct_freepages_start();
+}
+
+static inline void delayacct_freepages_end(void)
+{
+ if (current->delays)
+ __delayacct_freepages_end();
+}
+
#else
static inline void delayacct_set_flag(int flag)
{}
@@ -129,6 +143,11 @@ static inline __u64 delayacct_blkio_ticks(struct task_struct *tsk)
{ return 0; }
static inline int delayacct_is_task_waiting_on_io(struct task_struct *p)
{ return 0; }
+static inline void delayacct_freepages_start(void)
+{}
+static inline void delayacct_freepages_end(void)
+{}
+
#endif /* CONFIG_TASK_DELAY_ACCT */

#endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 3e05e54..e6c557c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -666,6 +666,10 @@ struct task_delay_info {
/* io operations performed */
u32 swapin_count; /* total count of the number of swapin block */
/* io operations performed */
+
+ struct timespec freepages_start, freepages_end;
+ u64 freepages_delay; /* wait for memory reclaim */
+ u32 freepages_count; /* total count of memory reclaim */
};
#endif /* CONFIG_TASK_DELAY_ACCT */

diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 10e43fd..84b6782 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -165,3 +165,16 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
return ret;
}

+void __delayacct_freepages_start(void)
+{
+ delayacct_start(&current->delays->freepages_start);
+}
+
+void __delayacct_freepages_end(void)
+{
+ delayacct_end(&current->delays->freepages_start,
+ &current->delays->freepages_end,
+ &current->delays->freepages_delay,
+ &current->delays->freepages_count);
+}
+
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 9a29901..a756076 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -38,6 +38,7 @@
#include <linux/kthread.h>
#include <linux/freezer.h>
#include <linux/memcontrol.h>
+#include <linux/delayacct.h>

#include <asm/tlbflush.h>
#include <asm/div64.h>
@@ -1316,6 +1317,8 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
struct zone *zone;
enum zone_type high_zoneidx = gfp_zone(sc->gfp_mask);

+ delayacct_freepages_start();
+
if (scan_global_lru(sc))
count_vm_event(ALLOCSTALL);
/*
@@ -1396,6 +1399,8 @@ out:
} else
mem_cgroup_record_reclaim_priority(sc->mem_cgroup, priority);

+ delayacct_freepages_end();
+
return ret;
}

--
1.5.0.6

2008-06-05 23:32:31

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 2/3 v2] per-task-delay-accounting: update taskstats for memory reclaim delay

Add members for memory reclaim delay to taskstats,
and accumulate them in __delayacct_add_tsk() .

Signed-off-by: Keika Kobayashi <[email protected]>
---
include/linux/taskstats.h | 4 ++++
kernel/delayacct.c | 3 +++
2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/linux/taskstats.h b/include/linux/taskstats.h
index 5d69c07..87aae21 100644
--- a/include/linux/taskstats.h
+++ b/include/linux/taskstats.h
@@ -81,6 +81,10 @@ struct taskstats {
__u64 swapin_count;
__u64 swapin_delay_total;

+ /* Delay waiting for memory reclaim */
+ __u64 freepages_count;
+ __u64 freepages_delay_total;
+
/* cpu "wall-clock" running time
* On some architectures, value will adjust for cpu time stolen
* from the kernel in involuntary waits due to virtualization.
diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 84b6782..b3179da 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -145,8 +145,11 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
d->blkio_delay_total = (tmp < d->blkio_delay_total) ? 0 : tmp;
tmp = d->swapin_delay_total + tsk->delays->swapin_delay;
d->swapin_delay_total = (tmp < d->swapin_delay_total) ? 0 : tmp;
+ tmp = d->freepages_delay_total + tsk->delays->freepages_delay;
+ d->freepages_delay_total = (tmp < d->freepages_delay_total) ? 0 : tmp;
d->blkio_count += tsk->delays->blkio_count;
d->swapin_count += tsk->delays->swapin_count;
+ d->freepages_count += tsk->delays->freepages_count;
spin_unlock_irqrestore(&tsk->delays->lock, flags);

done:
--
1.5.0.6

2008-06-05 23:33:50

by Keika Kobayashi

[permalink] [raw]
Subject: [PATCH 3/3 v2] per-task-delay-accounting: update document and getdelays.c for memory reclaim

Update document and make getdelays.c show delay accounting for memory reclaim.

For making a distinction between "swapping in pages" and "memory reclaim"
in getdelays.c, MEM is changed to SWAP.

Signed-off-by: Keika Kobayashi <[email protected]>
---
Documentation/accounting/delay-accounting.txt | 11 ++++++++---
Documentation/accounting/getdelays.c | 8 ++++++--
2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Documentation/accounting/delay-accounting.txt b/Documentation/accounting/delay-accounting.txt
index 1443cd7..8a12f07 100644
--- a/Documentation/accounting/delay-accounting.txt
+++ b/Documentation/accounting/delay-accounting.txt
@@ -11,6 +11,7 @@ the delays experienced by a task while
a) waiting for a CPU (while being runnable)
b) completion of synchronous block I/O initiated by the task
c) swapping in pages
+d) memory reclaim

and makes these statistics available to userspace through
the taskstats interface.
@@ -41,7 +42,7 @@ this structure. See
include/linux/taskstats.h
for a description of the fields pertaining to delay accounting.
It will generally be in the form of counters returning the cumulative
-delay seen for cpu, sync block I/O, swapin etc.
+delay seen for cpu, sync block I/O, swapin, memory reclaim etc.

Taking the difference of two successive readings of a given
counter (say cpu_delay_total) for a task will give the delay
@@ -94,7 +95,9 @@ CPU count real total virtual total delay total
7876 92005750 100000000 24001500
IO count delay total
0 0
-MEM count delay total
+SWAP count delay total
+ 0 0
+RECLAIM count delay total
0 0

Get delays seen in executing a given simple command
@@ -108,5 +111,7 @@ CPU count real total virtual total delay total
6 4000250 4000000 0
IO count delay total
0 0
-MEM count delay total
+SWAP count delay total
+ 0 0
+RECLAIM count delay total
0 0
diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
index 40121b5..3f7755f 100644
--- a/Documentation/accounting/getdelays.c
+++ b/Documentation/accounting/getdelays.c
@@ -196,14 +196,18 @@ void print_delayacct(struct taskstats *t)
" %15llu%15llu%15llu%15llu\n"
"IO %15s%15s\n"
" %15llu%15llu\n"
- "MEM %15s%15s\n"
+ "SWAP %15s%15s\n"
+ " %15llu%15llu\n"
+ "RECLAIM %12s%15s\n"
" %15llu%15llu\n",
"count", "real total", "virtual total", "delay total",
t->cpu_count, t->cpu_run_real_total, t->cpu_run_virtual_total,
t->cpu_delay_total,
"count", "delay total",
t->blkio_count, t->blkio_delay_total,
- "count", "delay total", t->swapin_count, t->swapin_delay_total);
+ "count", "delay total", t->swapin_count, t->swapin_delay_total,
+ "count", "delay total",
+ t->freepages_count, t->freepages_delay_total);
}

void task_context_switch_counts(struct taskstats *t)
--
1.5.0.6

2008-06-06 02:20:11

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] per-task-delay-accounting: add memory reclaim delay

Keika Kobayashi wrote:
> Hi.
>
> This is v2 of accounting memory reclaim patch series.
> Thanks to Kosaki-san, Kamezawa-san, Andrew for comments and advice!
> These patches were fixed about the following.
>
> Against: next-20080605
>
> 1) Change Log
>
> o Add accounting memory reclaim delay from memcgroup.
> For accounting both global and cgroup memory reclaim,
> accounting point was moved from try_to_free_pages() to do_try_to_free_pages.
>
> o Drop the patch regarding /proc export for memory reclaim delay.
> Because it seems that two separate ways to report are not necessary,
> this patch series supports only NETLINK and doesn't add a field to /proc/<pid>/stat.
>
>
> 2) Confirm the fix regarding memcgroup.
>
> o Previous patch can't catch memory reclaim delay from memcgroup.
>
> $ echo 10M > /cgroups/0/memory.limit_in_bytes
>
> $ ls -s test.dat
> 500496 test.dat
>
> $ time tar cvf test.tar test.dat
> real 0m21.957s
> user 0m0.032s
> sys 0m2.348s
>
> $ ./delayget -d -p <pid>
> CPU count real total virtual total delay total
> 2441 2288143000 2438256954 22371958
> IO count delay total
> 2444 18745251314
> SWAP count delay total
> 0 0
> RECLAIM count delay total
> 0 0
>
> o Current patch can catch memory reclaim delay from memcgroup.
>
> $ echo 10M > /cgroups/0/memory.limit_in_bytes
>
> $ ls -s test.dat
> 500496 test.dat
>
> $ time tar cvf test.tar test.dat
> real 0m22.563s
> user 0m0.028s
> sys 0m2.440s
>
> $ ./delayget -d -p <pid>
> CPU count real total virtual total delay total
> 2640 2456153500 2478353004 28366219
> IO count delay total
> 2628 19894214188
> SWAP count delay total
> 0 0
> RECLAIM count delay total
> 6600 10682486085
>

Looks interesting, this data is for the whole system or memcgroup? If it is for
memcgroup, we should be using cgroupstats.

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-06-06 03:01:17

by Kamezawa Hiroyuki

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] per-task-delay-accounting: add memory reclaim delay

On Thu, 5 Jun 2008 16:27:59 -0700
Keika Kobayashi <[email protected]> wrote:

> Hi.
>
> This is v2 of accounting memory reclaim patch series.
> Thanks to Kosaki-san, Kamezawa-san, Andrew for comments and advice!
> These patches were fixed about the following.
>
> Against: next-20080605
>
> 1) Change Log
>
> o Add accounting memory reclaim delay from memcgroup.
> For accounting both global and cgroup memory reclaim,
> accounting point was moved from try_to_free_pages() to do_try_to_free_pages.
>
> o Drop the patch regarding /proc export for memory reclaim delay.
> Because it seems that two separate ways to report are not necessary,
> this patch series supports only NETLINK and doesn't add a field to /proc/<pid>/stat.
>
>

Thanks, I like this set.

Acked-by: KAMEZAWA Hiroyuki <[email protected]>




> 2) Confirm the fix regarding memcgroup.
>
> o Previous patch can't catch memory reclaim delay from memcgroup.
>
> $ echo 10M > /cgroups/0/memory.limit_in_bytes
>
> $ ls -s test.dat
> 500496 test.dat
>
> $ time tar cvf test.tar test.dat
> real 0m21.957s
> user 0m0.032s
> sys 0m2.348s
>
> $ ./delayget -d -p <pid>
> CPU count real total virtual total delay total
> 2441 2288143000 2438256954 22371958
> IO count delay total
> 2444 18745251314
> SWAP count delay total
> 0 0
> RECLAIM count delay total
> 0 0
>
> o Current patch can catch memory reclaim delay from memcgroup.
>
> $ echo 10M > /cgroups/0/memory.limit_in_bytes
>
> $ ls -s test.dat
> 500496 test.dat
>
> $ time tar cvf test.tar test.dat
> real 0m22.563s
> user 0m0.028s
> sys 0m2.440s
>
> $ ./delayget -d -p <pid>
> CPU count real total virtual total delay total
> 2640 2456153500 2478353004 28366219
> IO count delay total
> 2628 19894214188
> SWAP count delay total
> 0 0
> RECLAIM count delay total
> 6600 10682486085
>

2008-06-06 03:15:51

by Kamezawa Hiroyuki

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] per-task-delay-accounting: add memory reclaim delay

On Fri, 06 Jun 2008 07:48:25 +0530
Balbir Singh <[email protected]> wrote:

> Keika Kobayashi wrote:
> > Hi.
> >
> > This is v2 of accounting memory reclaim patch series.
> > Thanks to Kosaki-san, Kamezawa-san, Andrew for comments and advice!
> > These patches were fixed about the following.
> >
> > Against: next-20080605
> >
> > 1) Change Log
> >
> > o Add accounting memory reclaim delay from memcgroup.
> > For accounting both global and cgroup memory reclaim,
> > accounting point was moved from try_to_free_pages() to do_try_to_free_pages.
> >
> > o Drop the patch regarding /proc export for memory reclaim delay.
> > Because it seems that two separate ways to report are not necessary,
> > this patch series supports only NETLINK and doesn't add a field to /proc/<pid>/stat.
> >
> >
> > 2) Confirm the fix regarding memcgroup.
> >
> > o Previous patch can't catch memory reclaim delay from memcgroup.
> >
> > $ echo 10M > /cgroups/0/memory.limit_in_bytes
> >
> > $ ls -s test.dat
> > 500496 test.dat
> >
> > $ time tar cvf test.tar test.dat
> > real 0m21.957s
> > user 0m0.032s
> > sys 0m2.348s
> >
> > $ ./delayget -d -p <pid>
> > CPU count real total virtual total delay total
> > 2441 2288143000 2438256954 22371958
> > IO count delay total
> > 2444 18745251314
> > SWAP count delay total
> > 0 0
> > RECLAIM count delay total
> > 0 0
> >
> > o Current patch can catch memory reclaim delay from memcgroup.
> >
> > $ echo 10M > /cgroups/0/memory.limit_in_bytes
> >
> > $ ls -s test.dat
> > 500496 test.dat
> >
> > $ time tar cvf test.tar test.dat
> > real 0m22.563s
> > user 0m0.028s
> > sys 0m2.440s
> >
> > $ ./delayget -d -p <pid>
> > CPU count real total virtual total delay total
> > 2640 2456153500 2478353004 28366219
> > IO count delay total
> > 2628 19894214188
> > SWAP count delay total
> > 0 0
> > RECLAIM count delay total
> > 6600 10682486085
> >
>
> Looks interesting, this data is for the whole system or memcgroup? If it is for
> memcgroup, we should be using cgroupstats.
>
This patch itself is for per-task-stat. And it seems cgroup_stat doesn't handle
delay accounting now. It just counts # of processes in some status.

Thanks,
-Kame




2008-06-06 03:21:40

by Keika Kobayashi

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] per-task-delay-accounting: add memory reclaim delay

On Fri, 06 Jun 2008 07:48:25 +0530
Balbir Singh <[email protected]> wrote:

> > $ ./delayget -d -p <pid>
> > CPU count real total virtual total delay total
> > 2640 2456153500 2478353004 28366219
> > IO count delay total
> > 2628 19894214188
> > SWAP count delay total
> > 0 0
> > RECLAIM count delay total
> > 6600 10682486085
> >
>
> Looks interesting, this data is for the whole system or memcgroup? If it is for
> memcgroup, we should be using cgroupstats.

Thanks for your comment.
This accounting, which is named "RECLAIM", is global and memcgroup reclaim delay
and this data is value per task.

Unfortunately, I'm not sure what the whole system means.
Could you tell me your point?

2008-06-06 03:42:54

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 0/3 v2] per-task-delay-accounting: add memory reclaim delay

Keika Kobayashi wrote:
> On Fri, 06 Jun 2008 07:48:25 +0530
> Balbir Singh <[email protected]> wrote:
>
>>> $ ./delayget -d -p <pid>
>>> CPU count real total virtual total delay total
>>> 2640 2456153500 2478353004 28366219
>>> IO count delay total
>>> 2628 19894214188
>>> SWAP count delay total
>>> 0 0
>>> RECLAIM count delay total
>>> 6600 10682486085
>>>
>> Looks interesting, this data is for the whole system or memcgroup? If it is for
>> memcgroup, we should be using cgroupstats.
>
> Thanks for your comment.
> This accounting, which is named "RECLAIM", is global and memcgroup reclaim delay
> and this data is value per task.
>
> Unfortunately, I'm not sure what the whole system means.
> Could you tell me your point?

By whole system, I meant global reclaim.

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-06-06 03:52:52

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] per-task-delay-accounting: add memory reclaim delay

Keika Kobayashi wrote:
> Sometimes, application responses become bad under heavy memory load.
> Applications take a bit time to reclaim memory.
> The statistics, how long memory reclaim takes, will be useful to
> measure memory usage.
>
> This patch adds accounting memory reclaim to per-task-delay-accounting
> for accounting the time of do_try_to_free_pages().
>

Looks good to me

Acked-by: Balbir Singh <[email protected]>

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-06-06 04:15:59

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] per-task-delay-accounting: update taskstats for memory reclaim delay

Keika Kobayashi wrote:
> Add members for memory reclaim delay to taskstats,
> and accumulate them in __delayacct_add_tsk() .
>
> Signed-off-by: Keika Kobayashi <[email protected]>

Two suggested changes

1. Please add all fields at the end (otherwise we risk breaking compatibility)
2. please also update Documentation/accounting/taskstats-struct.txt

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-06-06 04:16:43

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH 3/3 v2] per-task-delay-accounting: update document and getdelays.c for memory reclaim

Keika Kobayashi wrote:
> Update document and make getdelays.c show delay accounting for memory reclaim.
>
> For making a distinction between "swapping in pages" and "memory reclaim"
> in getdelays.c, MEM is changed to SWAP.
>
> Signed-off-by: Keika Kobayashi <[email protected]>

Looks good

Acked-by: Balbir Singh <[email protected]>

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2008-06-06 21:58:21

by Hiroshi Shimamoto

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] per-task-delay-accounting: update taskstats for memory reclaim delay

Balbir Singh wrote:
> Keika Kobayashi wrote:
>> Add members for memory reclaim delay to taskstats,
>> and accumulate them in __delayacct_add_tsk() .
>>
>> Signed-off-by: Keika Kobayashi <[email protected]>
>
> Two suggested changes
>
> 1. Please add all fields at the end (otherwise we risk breaking compatibility)
> 2. please also update Documentation/accounting/taskstats-struct.txt
>
And update TASKSTATS_VERSION, right?

Thanks,
Hiroshi Shimamoto

2008-06-07 00:24:38

by Keika Kobayashi

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] per-task-delay-accounting: update taskstats for memory reclaim delay

Add members for memory reclaim delay to taskstats,
and accumulate them in __delayacct_add_tsk() .

Signed-off-by: Keika Kobayashi <[email protected]>
---

Balbir Singh <[email protected]> wrote:

> Two suggested changes
>
> 1. Please add all fields at the end (otherwise we risk breaking compatibility)
> 2. please also update Documentation/accounting/taskstats-struct.txt
>

Thanks for your suggestion.
Update version is here.

In taskstats-struct.txt, there was not a discription for "Time accounting for SMT machines".
This patch was made after the following patch had been applied.
http://lkml.org/lkml/2008/6/6/436 .

Documentation/accounting/taskstats-struct.txt | 7 +++++++
include/linux/taskstats.h | 6 +++++-
kernel/delayacct.c | 3 +++
3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/accounting/taskstats-struct.txt b/Documentation/accounting/taskstats-struct.txt
index cd784f4..b988d11 100644
--- a/Documentation/accounting/taskstats-struct.txt
+++ b/Documentation/accounting/taskstats-struct.txt
@@ -26,6 +26,8 @@ There are three different groups of fields in the struct taskstats:

5) Time accounting for SMT machines

+6) Extended delay accounting fields for memory reclaim
+
Future extension should add fields to the end of the taskstats struct, and
should not change the relative position of each field within the struct.

@@ -170,4 +172,9 @@ struct taskstats {
__u64 ac_utimescaled; /* utime scaled on frequency etc */
__u64 ac_stimescaled; /* stime scaled on frequency etc */
__u64 cpu_scaled_run_real_total; /* scaled cpu_run_real_total */
+
+6) Extended delay accounting fields for memory reclaim
+ /* Delay waiting for memory reclaim */
+ __u64 freepages_count;
+ __u64 freepages_delay_total;
}
diff --git a/include/linux/taskstats.h b/include/linux/taskstats.h
index 5d69c07..18269e9 100644
--- a/include/linux/taskstats.h
+++ b/include/linux/taskstats.h
@@ -31,7 +31,7 @@
*/


-#define TASKSTATS_VERSION 6
+#define TASKSTATS_VERSION 7
#define TS_COMM_LEN 32 /* should be >= TASK_COMM_LEN
* in linux/sched.h */

@@ -157,6 +157,10 @@ struct taskstats {
__u64 ac_utimescaled; /* utime scaled on frequency etc */
__u64 ac_stimescaled; /* stime scaled on frequency etc */
__u64 cpu_scaled_run_real_total; /* scaled cpu_run_real_total */
+
+ /* Delay waiting for memory reclaim */
+ __u64 freepages_count;
+ __u64 freepages_delay_total;
};


diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 84b6782..b3179da 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -145,8 +145,11 @@ int __delayacct_add_tsk(struct taskstats *d, struct task_struct *tsk)
d->blkio_delay_total = (tmp < d->blkio_delay_total) ? 0 : tmp;
tmp = d->swapin_delay_total + tsk->delays->swapin_delay;
d->swapin_delay_total = (tmp < d->swapin_delay_total) ? 0 : tmp;
+ tmp = d->freepages_delay_total + tsk->delays->freepages_delay;
+ d->freepages_delay_total = (tmp < d->freepages_delay_total) ? 0 : tmp;
d->blkio_count += tsk->delays->blkio_count;
d->swapin_count += tsk->delays->swapin_count;
+ d->freepages_count += tsk->delays->freepages_count;
spin_unlock_irqrestore(&tsk->delays->lock, flags);

done:
--
1.5.0.6

2008-06-07 04:40:31

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] per-task-delay-accounting: add memory reclaim delay

> Sometimes, application responses become bad under heavy memory load.
> Applications take a bit time to reclaim memory.
> The statistics, how long memory reclaim takes, will be useful to
> measure memory usage.
>
> This patch adds accounting memory reclaim to per-task-delay-accounting
> for accounting the time of do_try_to_free_pages().

looks good to me.

Acked-by: KOSAKI Motohiro <[email protected]>


2008-06-07 04:49:33

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] per-task-delay-accounting: update taskstats for memory reclaim delay

> Add members for memory reclaim delay to taskstats,
> and accumulate them in __delayacct_add_tsk() .
>
> Signed-off-by: Keika Kobayashi <[email protected]>
> ---
>
> Balbir Singh <[email protected]> wrote:
>
> > Two suggested changes
> >
> > 1. Please add all fields at the end (otherwise we risk breaking compatibility)
> > 2. please also update Documentation/accounting/taskstats-struct.txt
> >
>
> Thanks for your suggestion.
> Update version is here.
>
> In taskstats-struct.txt, there was not a discription for "Time accounting for SMT machines".
> This patch was made after the following patch had been applied.
> http://lkml.org/lkml/2008/6/6/436 .

The code of this patch looks good to me.
but this change log isn't so good.

Do you want remain email bare discssion on git log?

2008-06-07 04:50:24

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH 3/3 v2] per-task-delay-accounting: update document and getdelays.c for memory reclaim

> Update document and make getdelays.c show delay accounting for memory reclaim.
>
> For making a distinction between "swapping in pages" and "memory reclaim"
> in getdelays.c, MEM is changed to SWAP.
>
> Signed-off-by: Keika Kobayashi <[email protected]>

looks good to me.

but I don't know s/MEM/SWAP/ renaming is good or bad.
You should hear delayacct developer opinion. IMHO.




2008-06-09 16:27:05

by Keika Kobayashi

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] per-task-delay-accounting: update taskstats for memory reclaim delay

On Sat, 07 Jun 2008 13:44:07 +0900
KOSAKI Motohiro <[email protected]> wrote:

> The code of this patch looks good to me.
> but this change log isn't so good.
>
> Do you want remain email bare discssion on git log?
>

Thanks for your comment.

But I confirmed this patch did not remain
"email bare discssion" on git log.