linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
@ 2009-12-28  4:46 Minchan Kim
  2009-12-28  4:47 ` KAMEZAWA Hiroyuki
  2009-12-29 20:08 ` Matt Mackall
  0 siblings, 2 replies; 10+ messages in thread
From: Minchan Kim @ 2009-12-28  4:46 UTC (permalink / raw)
  To: Andrew Morton
  Cc: lkml, linux-mm, Matt Mackall, KAMEZAWA Hiroyuki, Hugh Dickins


I am not sure we have to account zero page with file_rss. 
Hugh and Kame's new zero page doesn't do it. 
As side effect of this, we can prevent innocent process which have a lot
of zero page when OOM happens. 
(But I am not sure there is a process like this :)
So I think not file_rss counting is not bad. 

RSS counting zero page with file_rss helps any program using smaps?
If we have to keep the old behavior, I have to remake this patch. 

== CUT_HERE ==

Long time ago, We regards zero page as file_rss and
vm_normal_page doesn't return NULL.

But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
can return NULL in case of zero page. Also we don't count it with
file_rss any more.

Then, RSS and PSS can't be matched.
For consistency, Let's ignore zero page in smaps_pte_range.

CC: Matt Mackall <mpm@selenic.com>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 fs/proc/task_mmu.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 47c03f4..f277c4a 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -361,12 +361,11 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
 		if (!pte_present(ptent))
 			continue;
 
-		mss->resident += PAGE_SIZE;
-
 		page = vm_normal_page(vma, addr, ptent);
 		if (!page)
 			continue;
 
+		mss->resident += PAGE_SIZE;
 		/* Accumulate the size in pages that have been accessed. */
 		if (pte_young(ptent) || PageReferenced(page))
 			mss->referenced += PAGE_SIZE;
-- 
1.5.6.3



-- 
Kind regards,
Minchan Kim

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-28  4:46 [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps Minchan Kim
@ 2009-12-28  4:47 ` KAMEZAWA Hiroyuki
  2009-12-28  5:31   ` Minchan Kim
  2009-12-29 20:08 ` Matt Mackall
  1 sibling, 1 reply; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-12-28  4:47 UTC (permalink / raw)
  To: Minchan Kim; +Cc: Andrew Morton, lkml, linux-mm, Matt Mackall, Hugh Dickins

On Mon, 28 Dec 2009 13:46:19 +0900
Minchan Kim <minchan.kim@gmail.com> wrote:

> 
> I am not sure we have to account zero page with file_rss. 
> Hugh and Kame's new zero page doesn't do it. 
> As side effect of this, we can prevent innocent process which have a lot
> of zero page when OOM happens. 
> (But I am not sure there is a process like this :)
> So I think not file_rss counting is not bad. 
> 
> RSS counting zero page with file_rss helps any program using smaps?
> If we have to keep the old behavior, I have to remake this patch. 
> 
> == CUT_HERE ==
> 
> Long time ago, We regards zero page as file_rss and
> vm_normal_page doesn't return NULL.
> 
> But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
> can return NULL in case of zero page. Also we don't count it with
> file_rss any more.
> 
> Then, RSS and PSS can't be matched.
> For consistency, Let's ignore zero page in smaps_pte_range.
> 
> CC: Matt Mackall <mpm@selenic.com>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

BTW, how about counting ZERO page in smaps ? Ignoring them completely sounds
not very good.

Thanks,
-Kame

> ---
>  fs/proc/task_mmu.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 47c03f4..f277c4a 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -361,12 +361,11 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  		if (!pte_present(ptent))
>  			continue;
>  
> -		mss->resident += PAGE_SIZE;
> -
>  		page = vm_normal_page(vma, addr, ptent);
>  		if (!page)
>  			continue;
>  
> +		mss->resident += PAGE_SIZE;
>  		/* Accumulate the size in pages that have been accessed. */
>  		if (pte_young(ptent) || PageReferenced(page))
>  			mss->referenced += PAGE_SIZE;
> -- 
> 1.5.6.3
> 
> 
> 
> -- 
> Kind regards,
> Minchan Kim
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-28  4:47 ` KAMEZAWA Hiroyuki
@ 2009-12-28  5:31   ` Minchan Kim
  2009-12-28  5:43     ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 10+ messages in thread
From: Minchan Kim @ 2009-12-28  5:31 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Minchan Kim, Andrew Morton, lkml, linux-mm, Matt Mackall, Hugh Dickins

Hi, Kame.

On Mon, 28 Dec 2009 13:47:52 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Mon, 28 Dec 2009 13:46:19 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
> 
> > 
> > I am not sure we have to account zero page with file_rss. 
> > Hugh and Kame's new zero page doesn't do it. 
> > As side effect of this, we can prevent innocent process which have a lot
> > of zero page when OOM happens. 
> > (But I am not sure there is a process like this :)
> > So I think not file_rss counting is not bad. 
> > 
> > RSS counting zero page with file_rss helps any program using smaps?
> > If we have to keep the old behavior, I have to remake this patch. 
> > 
> > == CUT_HERE ==
> > 
> > Long time ago, We regards zero page as file_rss and
> > vm_normal_page doesn't return NULL.
> > 
> > But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
> > can return NULL in case of zero page. Also we don't count it with
> > file_rss any more.
> > 
> > Then, RSS and PSS can't be matched.
> > For consistency, Let's ignore zero page in smaps_pte_range.
> > 
> > CC: Matt Mackall <mpm@selenic.com>
> > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> 
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Thanks for ACK. :)

> 
> BTW, how about counting ZERO page in smaps? Ignoring them completely sounds
> not very good.

I am not use it is useful. 

zero page snapshot of ongoing process is useful?
Doesn't Admin need to know about zero page?
Let's admins use it well. If we remove zero page again?
How many are applications use smaps? 
Did we have a problem without it?

When I think of it, there are too many qeustions. 
Most important thing to add new statistics is just need of customer. 

Frankly speaking, I don't have good scenario of using zero page.
Do you have any scenario it is valueable?

> 
> Thanks,
> -Kame



-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-28  5:31   ` Minchan Kim
@ 2009-12-28  5:43     ` KAMEZAWA Hiroyuki
  2009-12-28  9:59       ` Minchan Kim
  0 siblings, 1 reply; 10+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-12-28  5:43 UTC (permalink / raw)
  To: Minchan Kim; +Cc: Andrew Morton, lkml, linux-mm, Matt Mackall, Hugh Dickins

On Mon, 28 Dec 2009 14:31:54 +0900
Minchan Kim <minchan.kim@gmail.com> wrote:
> 
> > BTW, how about counting ZERO page in smaps? Ignoring them completely sounds
> > not very good.
> 
> I am not use it is useful. 
> 
> zero page snapshot of ongoing process is useful?
> Doesn't Admin need to know about zero page?
> Let's admins use it well. If we remove zero page again?
> How many are applications use smaps? 
> Did we have a problem without it?
> 
My concern is that hiding indormation which was exported before.
No more than that and no strong demand.


> When I think of it, there are too many qeustions. 
> Most important thing to add new statistics is just need of customer. 
> 
> Frankly speaking, I don't have good scenario of using zero page.
> Do you have any scenario it is valueable?
> 
read before write ? maybe sometimes happens.

For example. current glibc's calloc() avoids memset() if the pages are
dropped by MADVISE (without unmap). 

Before starting zero-page works, I checked "questions" in lkml and
found some reports that some applications start to go OOM after zero-page
removal.

For me, I know one of my customer's application depends on behavior of
zero page (on RHEL5). So, I tried to add again it before RHEL6 because
I think removal of zero-page corrupts compatibility.

Thanks,
-Kame


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-28  5:43     ` KAMEZAWA Hiroyuki
@ 2009-12-28  9:59       ` Minchan Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Minchan Kim @ 2009-12-28  9:59 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Andrew Morton, lkml, linux-mm, Matt Mackall, Hugh Dickins

On Mon, Dec 28, 2009 at 2:43 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Mon, 28 Dec 2009 14:31:54 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
>>
>> > BTW, how about counting ZERO page in smaps? Ignoring them completely sounds
>> > not very good.
>>
>> I am not use it is useful.
>>
>> zero page snapshot of ongoing process is useful?
>> Doesn't Admin need to know about zero page?
>> Let's admins use it well. If we remove zero page again?
>> How many are applications use smaps?
>> Did we have a problem without it?
>>
> My concern is that hiding indormation which was exported before.
> No more than that and no strong demand.
>
>
>> When I think of it, there are too many qeustions.
>> Most important thing to add new statistics is just need of customer.
>>
>> Frankly speaking, I don't have good scenario of using zero page.
>> Do you have any scenario it is valueable?
>>
> read before write ? maybe sometimes happens.
>
> For example. current glibc's calloc() avoids memset() if the pages are
> dropped by MADVISE (without unmap).
>
> Before starting zero-page works, I checked "questions" in lkml and
> found some reports that some applications start to go OOM after zero-page
> removal.
>
> For me, I know one of my customer's application depends on behavior of
> zero page (on RHEL5). So, I tried to add again it before RHEL6 because
> I think removal of zero-page corrupts compatibility.
>

Okay. I will repost the patch.

> Thanks,
> -Kame
>
>



-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-28  4:46 [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps Minchan Kim
  2009-12-28  4:47 ` KAMEZAWA Hiroyuki
@ 2009-12-29 20:08 ` Matt Mackall
  2009-12-30  1:33   ` Minchan Kim
  1 sibling, 1 reply; 10+ messages in thread
From: Matt Mackall @ 2009-12-29 20:08 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, lkml, linux-mm, KAMEZAWA Hiroyuki, Hugh Dickins

On Mon, 2009-12-28 at 13:46 +0900, Minchan Kim wrote:
> I am not sure we have to account zero page with file_rss. 
> Hugh and Kame's new zero page doesn't do it. 
> As side effect of this, we can prevent innocent process which have a lot
> of zero page when OOM happens. 
> (But I am not sure there is a process like this :)
> So I think not file_rss counting is not bad. 
> 
> RSS counting zero page with file_rss helps any program using smaps?
> If we have to keep the old behavior, I have to remake this patch. 
> 
> == CUT_HERE ==
> 
> Long time ago, We regards zero page as file_rss and
> vm_normal_page doesn't return NULL.
> 
> But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
> can return NULL in case of zero page. Also we don't count it with
> file_rss any more.
> 
> Then, RSS and PSS can't be matched.
> For consistency, Let's ignore zero page in smaps_pte_range.
> 

Not counting the zero page in RSS is fine with me. But will this patch
make the total from smaps agree with get_mm_rss()?

Regarding OOM handling: arguably RSS should play no role in OOM as it's
practically meaningless in a shared memory system. If we were instead
used per-process unshared pages as the metric (aka USS), we'd have a
much better notion of how much memory an OOM kill would recover.
Unfortunately, that's not trivial to track as the accounting on COW
operations is not lightweight.

> CC: Matt Mackall <mpm@selenic.com>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> ---
>  fs/proc/task_mmu.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 47c03f4..f277c4a 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -361,12 +361,11 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>  		if (!pte_present(ptent))
>  			continue;
>  
> -		mss->resident += PAGE_SIZE;
> -
>  		page = vm_normal_page(vma, addr, ptent);
>  		if (!page)
>  			continue;
>  
> +		mss->resident += PAGE_SIZE;
>  		/* Accumulate the size in pages that have been accessed. */
>  		if (pte_young(ptent) || PageReferenced(page))
>  			mss->referenced += PAGE_SIZE;
> -- 
> 1.5.6.3
> 
> 
> 



-- 
http://selenic.com : development and support for Mercurial and Linux



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-29 20:08 ` Matt Mackall
@ 2009-12-30  1:33   ` Minchan Kim
  2009-12-30  3:11     ` Matt Mackall
  2009-12-30 16:19     ` Hugh Dickins
  0 siblings, 2 replies; 10+ messages in thread
From: Minchan Kim @ 2009-12-30  1:33 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Minchan Kim, Andrew Morton, lkml, linux-mm, KAMEZAWA Hiroyuki,
	Hugh Dickins

Hi, Matt. 

On Tue, 29 Dec 2009 14:08:59 -0600
Matt Mackall <mpm@selenic.com> wrote:

> On Mon, 2009-12-28 at 13:46 +0900, Minchan Kim wrote:
> > I am not sure we have to account zero page with file_rss. 
> > Hugh and Kame's new zero page doesn't do it. 
> > As side effect of this, we can prevent innocent process which have a lot
> > of zero page when OOM happens. 
> > (But I am not sure there is a process like this :)
> > So I think not file_rss counting is not bad. 
> > 
> > RSS counting zero page with file_rss helps any program using smaps?
> > If we have to keep the old behavior, I have to remake this patch. 
> > 
> > == CUT_HERE ==
> > 
> > Long time ago, We regards zero page as file_rss and
> > vm_normal_page doesn't return NULL.
> > 
> > But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
> > can return NULL in case of zero page. Also we don't count it with
> > file_rss any more.
> > 
> > Then, RSS and PSS can't be matched.
> > For consistency, Let's ignore zero page in smaps_pte_range.
> > 
> 
> Not counting the zero page in RSS is fine with me. But will this patch
> make the total from smaps agree with get_mm_rss()?

Yes. Anon page fault handler also don't count zero page any more, now. 
Nonetheless, smaps counts it with resident. 

It's point of this patch. 

But I reposted both anon fault handler and here counts it as file_rss
as compatibility with old zero page counting.
Pz, Look at that. :)

> 
> Regarding OOM handling: arguably RSS should play no role in OOM as it's
> practically meaningless in a shared memory system. If we were instead

It's very arguable issue for us that OOM depens on RSS.

> used per-process unshared pages as the metric (aka USS), we'd have a
> much better notion of how much memory an OOM kill would recover.
> Unfortunately, that's not trivial to track as the accounting on COW
> operations is not lightweight.

I think we can approximate it with the size of VM_SHARED vma of process
when VM calculate badness. 
What do you think about it?

Thanks for good idea, Matt. 

> 
> > CC: Matt Mackall <mpm@selenic.com>
> > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > ---
> >  fs/proc/task_mmu.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index 47c03f4..f277c4a 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -361,12 +361,11 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> >  		if (!pte_present(ptent))
> >  			continue;
> >  
> > -		mss->resident += PAGE_SIZE;
> > -
> >  		page = vm_normal_page(vma, addr, ptent);
> >  		if (!page)
> >  			continue;
> >  
> > +		mss->resident += PAGE_SIZE;
> >  		/* Accumulate the size in pages that have been accessed. */
> >  		if (pte_young(ptent) || PageReferenced(page))
> >  			mss->referenced += PAGE_SIZE;
> > -- 
> > 1.5.6.3
> > 
> > 
> > 
> 
> 
> 
> -- 
> http://selenic.com : development and support for Mercurial and Linux
> 
> 


-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-30  1:33   ` Minchan Kim
@ 2009-12-30  3:11     ` Matt Mackall
  2009-12-30 16:19     ` Hugh Dickins
  1 sibling, 0 replies; 10+ messages in thread
From: Matt Mackall @ 2009-12-30  3:11 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, lkml, linux-mm, KAMEZAWA Hiroyuki, Hugh Dickins

On Wed, 2009-12-30 at 10:33 +0900, Minchan Kim wrote:
> Hi, Matt. 
> 
> On Tue, 29 Dec 2009 14:08:59 -0600
> Matt Mackall <mpm@selenic.com> wrote:
> 
> > On Mon, 2009-12-28 at 13:46 +0900, Minchan Kim wrote:
> > > I am not sure we have to account zero page with file_rss. 
> > > Hugh and Kame's new zero page doesn't do it. 
> > > As side effect of this, we can prevent innocent process which have a lot
> > > of zero page when OOM happens. 
> > > (But I am not sure there is a process like this :)
> > > So I think not file_rss counting is not bad. 
> > > 
> > > RSS counting zero page with file_rss helps any program using smaps?
> > > If we have to keep the old behavior, I have to remake this patch. 
> > > 
> > > == CUT_HERE ==
> > > 
> > > Long time ago, We regards zero page as file_rss and
> > > vm_normal_page doesn't return NULL.
> > > 
> > > But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
> > > can return NULL in case of zero page. Also we don't count it with
> > > file_rss any more.
> > > 
> > > Then, RSS and PSS can't be matched.
> > > For consistency, Let's ignore zero page in smaps_pte_range.
> > > 
> > 
> > Not counting the zero page in RSS is fine with me. But will this patch
> > make the total from smaps agree with get_mm_rss()?
> 
> Yes. Anon page fault handler also don't count zero page any more, now. 
> Nonetheless, smaps counts it with resident. 

Ok, great.

Acked-by: Matt Mackall <mpm@selenic.com>

-- 
http://selenic.com : development and support for Mercurial and Linux



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-30  1:33   ` Minchan Kim
  2009-12-30  3:11     ` Matt Mackall
@ 2009-12-30 16:19     ` Hugh Dickins
  2009-12-31  2:47       ` Minchan Kim
  1 sibling, 1 reply; 10+ messages in thread
From: Hugh Dickins @ 2009-12-30 16:19 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Matt Mackall, Andrew Morton, lkml, linux-mm, KAMEZAWA Hiroyuki

On Wed, 30 Dec 2009, Minchan Kim wrote:
> On Tue, 29 Dec 2009 14:08:59 -0600
> Matt Mackall <mpm@selenic.com> wrote:
> > On Mon, 2009-12-28 at 13:46 +0900, Minchan Kim wrote:
> > > I am not sure we have to account zero page with file_rss. 
> > > Hugh and Kame's new zero page doesn't do it. 
> > > As side effect of this, we can prevent innocent process which have a lot
> > > of zero page when OOM happens. 
> > > (But I am not sure there is a process like this :)
> > > So I think not file_rss counting is not bad. 
> > > 
> > > RSS counting zero page with file_rss helps any program using smaps?
> > > If we have to keep the old behavior, I have to remake this patch. 
> > > 
> > > == CUT_HERE ==
> > > 
> > > Long time ago, We regards zero page as file_rss and
> > > vm_normal_page doesn't return NULL.
> > > 
> > > But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
> > > can return NULL in case of zero page. Also we don't count it with
> > > file_rss any more.
> > > 
> > > Then, RSS and PSS can't be matched.
> > > For consistency, Let's ignore zero page in smaps_pte_range.
> > > 
> > 
> > Not counting the zero page in RSS is fine with me. But will this patch
> > make the total from smaps agree with get_mm_rss()?
> 
> Yes. Anon page fault handler also don't count zero page any more, now. 
> Nonetheless, smaps counts it with resident. 
> 
> It's point of this patch. 
> 
> But I reposted both anon fault handler and here counts it as file_rss
> as compatibility with old zero page counting.
> Pz, Look at that. :)

I am getting confused between your different patches in this area,
heading in different directions, not increments in the same series.
But I think this is the one to which, like Matt, I'll say

Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

> 
> > 
> > Regarding OOM handling: arguably RSS should play no role in OOM as it's
> > practically meaningless in a shared memory system. If we were instead
> 
> It's very arguable issue for us that OOM depens on RSS.
> 
> > used per-process unshared pages as the metric (aka USS), we'd have a
> > much better notion of how much memory an OOM kill would recover.
> > Unfortunately, that's not trivial to track as the accounting on COW
> > operations is not lightweight.
> 
> I think we can approximate it with the size of VM_SHARED vma of process
> when VM calculate badness. 
> What do you think about it?

Sounds like it'll end up even harder to understand than by size or by rss.

> 
> Thanks for good idea, Matt. 
> 
> > 
> > > CC: Matt Mackall <mpm@selenic.com>
> > > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > > ---
> > >  fs/proc/task_mmu.c |    3 +--
> > >  1 files changed, 1 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > > index 47c03f4..f277c4a 100644
> > > --- a/fs/proc/task_mmu.c
> > > +++ b/fs/proc/task_mmu.c
> > > @@ -361,12 +361,11 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
> > >  		if (!pte_present(ptent))
> > >  			continue;
> > >  
> > > -		mss->resident += PAGE_SIZE;
> > > -
> > >  		page = vm_normal_page(vma, addr, ptent);
> > >  		if (!page)
> > >  			continue;
> > >  
> > > +		mss->resident += PAGE_SIZE;
> > >  		/* Accumulate the size in pages that have been accessed. */
> > >  		if (pte_young(ptent) || PageReferenced(page))
> > >  			mss->referenced += PAGE_SIZE;
> > > -- 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps
  2009-12-30 16:19     ` Hugh Dickins
@ 2009-12-31  2:47       ` Minchan Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Minchan Kim @ 2009-12-31  2:47 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Matt Mackall, Andrew Morton, lkml, linux-mm, KAMEZAWA Hiroyuki

On Thu, Dec 31, 2009 at 1:19 AM, Hugh Dickins
<hugh.dickins@tiscali.co.uk> wrote:
> On Wed, 30 Dec 2009, Minchan Kim wrote:
>> On Tue, 29 Dec 2009 14:08:59 -0600
>> Matt Mackall <mpm@selenic.com> wrote:
>> > On Mon, 2009-12-28 at 13:46 +0900, Minchan Kim wrote:
>> > > I am not sure we have to account zero page with file_rss.
>> > > Hugh and Kame's new zero page doesn't do it.
>> > > As side effect of this, we can prevent innocent process which have a lot
>> > > of zero page when OOM happens.
>> > > (But I am not sure there is a process like this :)
>> > > So I think not file_rss counting is not bad.
>> > >
>> > > RSS counting zero page with file_rss helps any program using smaps?
>> > > If we have to keep the old behavior, I have to remake this patch.
>> > >
>> > > == CUT_HERE ==
>> > >
>> > > Long time ago, We regards zero page as file_rss and
>> > > vm_normal_page doesn't return NULL.
>> > >
>> > > But now, we reinstated ZERO_PAGE and vm_normal_page's implementation
>> > > can return NULL in case of zero page. Also we don't count it with
>> > > file_rss any more.
>> > >
>> > > Then, RSS and PSS can't be matched.
>> > > For consistency, Let's ignore zero page in smaps_pte_range.
>> > >
>> >
>> > Not counting the zero page in RSS is fine with me. But will this patch
>> > make the total from smaps agree with get_mm_rss()?
>>
>> Yes. Anon page fault handler also don't count zero page any more, now.
>> Nonetheless, smaps counts it with resident.
>>
>> It's point of this patch.
>>
>> But I reposted both anon fault handler and here counts it as file_rss
>> as compatibility with old zero page counting.
>> Pz, Look at that. :)
>
> I am getting confused between your different patches in this area,
> heading in different directions, not increments in the same series.
> But I think this is the one to which, like Matt, I'll say
>
> Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

Thanks for ACK, Hugh.
It's my old version.
This patch can be changed according to account zero page as file_rss or not.
Anyway, We need consistency regardless of it.

>
>>
>> >
>> > Regarding OOM handling: arguably RSS should play no role in OOM as it's
>> > practically meaningless in a shared memory system. If we were instead
>>
>> It's very arguable issue for us that OOM depens on RSS.
>>
>> > used per-process unshared pages as the metric (aka USS), we'd have a
>> > much better notion of how much memory an OOM kill would recover.
>> > Unfortunately, that's not trivial to track as the accounting on COW
>> > operations is not lightweight.
>>
>> I think we can approximate it with the size of VM_SHARED vma of process
>> when VM calculate badness.
>> What do you think about it?
>
> Sounds like it'll end up even harder to understand than by size or by rss.

Yes. If we settle down OOM issue, I will repost this issue. :)

>
>>
>> Thanks for good idea, Matt.
>>
>> >
>> > > CC: Matt Mackall <mpm@selenic.com>
>> > > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
>> > > ---
>> > >  fs/proc/task_mmu.c |    3 +--
>> > >  1 files changed, 1 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> > > index 47c03f4..f277c4a 100644
>> > > --- a/fs/proc/task_mmu.c
>> > > +++ b/fs/proc/task_mmu.c
>> > > @@ -361,12 +361,11 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
>> > >           if (!pte_present(ptent))
>> > >                   continue;
>> > >
>> > > -         mss->resident += PAGE_SIZE;
>> > > -
>> > >           page = vm_normal_page(vma, addr, ptent);
>> > >           if (!page)
>> > >                   continue;
>> > >
>> > > +         mss->resident += PAGE_SIZE;
>> > >           /* Accumulate the size in pages that have been accessed. */
>> > >           if (pte_young(ptent) || PageReferenced(page))
>> > >                   mss->referenced += PAGE_SIZE;
>> > > --
>



-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-12-31  2:47 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-28  4:46 [PATCH -mmotm-2009-12-10-17-19] Fix wrong rss count of smaps Minchan Kim
2009-12-28  4:47 ` KAMEZAWA Hiroyuki
2009-12-28  5:31   ` Minchan Kim
2009-12-28  5:43     ` KAMEZAWA Hiroyuki
2009-12-28  9:59       ` Minchan Kim
2009-12-29 20:08 ` Matt Mackall
2009-12-30  1:33   ` Minchan Kim
2009-12-30  3:11     ` Matt Mackall
2009-12-30 16:19     ` Hugh Dickins
2009-12-31  2:47       ` Minchan Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).