lkml.org 
[lkml]   [2008]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] getrusage: fill ru_maxrss value
On Tue, 2 Dec 2008 17:05:07 +0100
Oleg Nesterov <oleg@redhat.com> wrote:

> On 12/02, Jiri Pirko wrote:
> >
> > Based on the patch previously posted by Frank Mayhar:
> > http://kerneltrap.org/mailarchive/linux-kernel/2007/9/20/264772
> >
> > Changed to do not panic and to set the value properly. Also made some
> > modifications as Oleg suggested. maxrss value is set to pages as "time"
> > application converts it co KBs.
> >
> > This patch enables "time" application to show maxresident value
> > correctly.
>
> I believe the changelog could be a bit more descriptive ;)
> And I think the patch needs more CCs. (Hugh and Michael cc'ed).
>
> What about exec? Should we preserve signal_struct->maxrss, or should
> we reset it? I don't know what is the right behaviour, but imho it
> should be consistent with task_mem()/xacct_add_tsk(). And since
> bprm_mm_init() does not copy ->hiwater_xxx, perhaps we should reset.
> Or we should change the exec_mmap() path to preserve ->hiwater_xxx,
> I dunno.
>
> > Note that even without this patch applied there is a race between two
> > parallel update_hiwater_rss() callers.
> >
> > [...snip...]
> > @@ -1051,6 +1051,8 @@ NORET_TYPE void do_exit(long code)
> > if (tsk->mm) {
> > update_hiwater_rss(tsk->mm);
> > update_hiwater_vm(tsk->mm);
> > +
> > + tsk->signal->maxrss = tsk->mm->hiwater_rss;
>
> Yes. But still it is not good the patch adds new racy calls
> (k_getrusage() too). And in fact I think this code in do_exit()
> should die, and we can update signal->maxrss in exit_mm().
>
> Unless I missed something, the "if (tsk->mm) {}" code in
> do_exit() is not needed because xacct_add_tsk() is not right
> anyway. What we imho need is get_mm_hiwater_xxx(), can be
> also used by task_mm().
>
> I'll send the patch a bit later, then we can tweak this patch.
>
> Do you agree?
Okay.

Jirka
>
> (just in case, I don't mean that mm/ uses update_hiwater_xxx()
> "safely", afaics try_to_unmap can race with unmap_region
> for example, but still).
>
> Oleg.
>


\
 
 \ /
  Last update: 2008-12-02 17:55    [W:0.036 / U:0.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site