On Wed 03-02-16 15:10:57, David Rientjes wrote:
> On Wed, 3 Feb 2016, Michal Hocko wrote:
>
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 8e345126d73e..b87acdca2a41 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -420,6 +420,7 @@ static struct task_struct *oom_reaper_th;
> > static struct task_struct *task_to_reap;
> > static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
> >
> > +#define K(x) ((x) << (PAGE_SHIFT-10))
> > static bool __oom_reap_task(struct task_struct *tsk)
> > {
> > struct mmu_gather tlb;
> > @@ -476,6 +477,11 @@ static bool __oom_reap_task(struct task_struct *tsk)
> > }
> > }
> > tlb_finish_mmu(&tlb, 0, -1);
> > + pr_info("oom_reaper: reaped process :%d (%s) anon-rss:%lukB,
> > file-rss:%lukB, shmem-rss:%lulB\n",
> > + task_pid_nr(tsk), tsk->comm,
> > + K(get_mm_counter(mm, MM_ANONPAGES)),
> > + K(get_mm_counter(mm, MM_FILEPAGES)),
> > + K(get_mm_counter(mm, MM_SHMEMPAGES)));
> > up_read(&mm->mmap_sem);
> >
> > /*
>
> This is a bit misleading, it would appear that the rss values are what was
> reaped when in fact they represent just the values of the mm being reaped.
> We have already printed these values as an artifact in the kernel log.
Yes and the idea was to provide the after state to compare before and
after. That's why I have kept the similar format. Just dropped the
virtual memory size because that doesn't make any sense in this context
now.
> I think it would be helpful to show anon-rss after reaping, however, so we
> can compare to the previous anon-rss that was reported. And, I agree that
> leaving behind a message in the kernel log that reaping has been
> successful is worthwhile. So this line should just show what anon-rss is
> after reaping and make it clear that this is not the memory reaped.
Does
"oom_reaper: reaped process %d (%s) current memory anon-rss:%lukB,
file-rss:%lukB, shmem-rss:%lukB "
sound any better?
--
Michal Hocko
SUSE Labs
