[PATCH] exit,stats: /* obey this comment */

[PATCH] exit,stats: /* obey this comment */

[Rik van Riel]

There is a helpful comment in do_exit() that states we sync the
mm's RSS info before statistics gathering.

The function that does the statistics gathering is called right
above that comment.

Change the code to obey the comment.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/exit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 22fcc05dec40..8a87bb43dbd0 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -711,10 +711,10 @@ void do_exit(long code)
 			current->comm, task_pid_nr(current),
 			preempt_count());
 
-	acct_update_integrals(tsk);
 	/* sync mm's RSS info before statistics gathering */
 	if (tsk->mm)
 		sync_mm_rss(tsk->mm);
+	acct_update_integrals(tsk);
 	group_dead = atomic_dec_and_test(&tsk->signal->live);
 	if (group_dead) {
 		hrtimer_cancel(&tsk->signal->real_timer);

Re: [PATCH] exit,stats: /* obey this comment */

[Andrew Morton]

On Thu, 23 Apr 2015 17:17:59 -0400 Rik van Riel <riel@redhat.com> wrote:

> There is a helpful comment in do_exit() that states we sync the
> mm's RSS info before statistics gathering.
> 
> The function that does the statistics gathering is called right
> above that comment.
> 
> Change the code to obey the comment.
> 
> ...
>
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -711,10 +711,10 @@ void do_exit(long code)
>  			current->comm, task_pid_nr(current),
>  			preempt_count());
>  
> -	acct_update_integrals(tsk);
>  	/* sync mm's RSS info before statistics gathering */
>  	if (tsk->mm)
>  		sync_mm_rss(tsk->mm);
> +	acct_update_integrals(tsk);
>  	group_dead = atomic_dec_and_test(&tsk->signal->live);
>  	if (group_dead) {
>  		hrtimer_cancel(&tsk->signal->real_timer);

I can't actually find anywhere where these counters are used in the
accounting code.

In fact the rss_stat counters don't get used for much at all, really -
non-essential stuff in the oom killer and a bit of procfs eyecandy. 
What have I missed.

The patch makes sense from a defensive POV, but perhaps that
comment is misleading.

Re: [PATCH] exit,stats: /* obey this comment */

[Rik van Riel]

On 04/23/2015 05:47 PM, Andrew Morton wrote:
> On Thu, 23 Apr 2015 17:17:59 -0400 Rik van Riel <riel@redhat.com> wrote:
> 
>> There is a helpful comment in do_exit() that states we sync the
>> mm's RSS info before statistics gathering.
>>
>> The function that does the statistics gathering is called right
>> above that comment.
>>
>> Change the code to obey the comment.
>>
>> ...
>>
>> --- a/kernel/exit.c
>> +++ b/kernel/exit.c
>> @@ -711,10 +711,10 @@ void do_exit(long code)
>>  			current->comm, task_pid_nr(current),
>>  			preempt_count());
>>  
>> -	acct_update_integrals(tsk);
>>  	/* sync mm's RSS info before statistics gathering */
>>  	if (tsk->mm)
>>  		sync_mm_rss(tsk->mm);
>> +	acct_update_integrals(tsk);
>>  	group_dead = atomic_dec_and_test(&tsk->signal->live);
>>  	if (group_dead) {
>>  		hrtimer_cancel(&tsk->signal->real_timer);
> 
> I can't actually find anywhere where these counters are used in the
> accounting code.

Don't the get reported to userspace in things like this?

$ /usr/bin/time ls /dev
...
0.00user 0.00system 0:00.00elapsed 33%CPU (0avgtext+0avgdata
2652maxresident)k
0inputs+0outputs (0major+135minor)pagefaults 0swaps

Though admittedly this thing only reports maxresident...

I think there is some tool that reports the avg resident rss,
but I cannot remember what it is.

-- 
All rights reversed