From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail6.bemta7.messagelabs.com (mail6.bemta7.messagelabs.com [216.82.255.55]) by kanga.kvack.org (Postfix) with ESMTP id 7FB3E6B0022 for ; Tue, 17 May 2011 16:00:42 -0400 (EDT) Received: from hpaq3.eem.corp.google.com (hpaq3.eem.corp.google.com [172.25.149.3]) by smtp-out.google.com with ESMTP id p4HK0LoM019108 for ; Tue, 17 May 2011 13:00:22 -0700 Received: from qwk3 (qwk3.prod.google.com [10.241.195.131]) by hpaq3.eem.corp.google.com with ESMTP id p4HJxNjd000434 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for ; Tue, 17 May 2011 13:00:20 -0700 Received: by qwk3 with SMTP id 3so447694qwk.19 for ; Tue, 17 May 2011 13:00:14 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: Date: Tue, 17 May 2011 13:00:14 -0700 Message-ID: Subject: Re: [PATCH mmotm] add the pagefault count into memcg stats: shmem fix From: Ying Han Content-Type: multipart/alternative; boundary=0016360e3f5c6d6ab404a37e389b Sender: owner-linux-mm@kvack.org List-ID: To: Hugh Dickins Cc: Andrew Morton , KAMEZAWA Hiroyuki , KOSAKI Motohiro , Minchan Kim , Daisuke Nishimura , Balbir Singh , linux-mm@kvack.org --0016360e3f5c6d6ab404a37e389b Content-Type: text/plain; charset=ISO-8859-1 On Tue, May 17, 2011 at 11:24 AM, Hugh Dickins wrote: > mem_cgroup_count_vm_event() should update the PGMAJFAULT count for the > target mm, not for current mm (but of course they're usually the same). > > We don't know the target mm in shmem_getpage(), so do it at the outer > level in shmem_fault(); and it's easier to follow if we move the > count_vm_event(PGMAJFAULT) there too. > > Hah, it was using __count_vm_event() before, sneaking that update into > the unpreemptible section under info->lock: well, it comes to the same > on x86 at least, and I still think it's best to keep these together. > > Signed-off-by: Hugh Dickins > --- > > mm/shmem.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > --- mmotm/mm/shmem.c 2011-05-13 14:57:45.367884578 -0700 > +++ linux/mm/shmem.c 2011-05-17 10:27:19.901934756 -0700 > @@ -1293,14 +1293,10 @@ repeat: > swappage = lookup_swap_cache(swap); > if (!swappage) { > shmem_swp_unmap(entry); > + spin_unlock(&info->lock); > /* here we actually do the io */ > - if (type && !(*type & VM_FAULT_MAJOR)) { > - __count_vm_event(PGMAJFAULT); > - mem_cgroup_count_vm_event(current->mm, > - PGMAJFAULT); > + if (type) > *type |= VM_FAULT_MAJOR; > - } > - spin_unlock(&info->lock); > swappage = shmem_swapin(swap, gfp, info, idx); > if (!swappage) { > spin_lock(&info->lock); > @@ -1539,7 +1535,10 @@ static int shmem_fault(struct vm_area_st > error = shmem_getpage(inode, vmf->pgoff, &vmf->page, SGP_CACHE, > &ret); > if (error) > return ((error == -ENOMEM) ? VM_FAULT_OOM : > VM_FAULT_SIGBUS); > - > + if (ret & VM_FAULT_MAJOR) { > + count_vm_event(PGMAJFAULT); > + mem_cgroup_count_vm_event(vma->vm_mm, PGMAJFAULT); > + } > return ret | VM_FAULT_LOCKED; > } > > Thank you Hugh for the fix. Acked-by: Ying Han --Ying --0016360e3f5c6d6ab404a37e389b Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Tue, May 17, 2011 at 11:24 AM, Hugh D= ickins <hughd@goog= le.com> wrote:
mem_cgroup_count_vm_event() should update the PGMAJFAULT count for the
target mm, not for current mm (but of course they're usually the same).=

We don't know the target mm in shmem_getpage(), so do it at the outer level in shmem_fault(); and it's easier to follow if we move the
count_vm_event(PGMAJFAULT) there too.

Hah, it was using __count_vm_event() before, sneaking that update into
the unpreemptible section under info->lock: well, it comes to the same on x86 at least, and I still think it's best to keep these together.
Signed-off-by: Hugh Dickins <hughd@g= oogle.com>
---

=A0mm/shmem.c | =A0 13 ++++++-------
=A01 file changed, 6 insertions(+), 7 deletions(-)

--- mmotm/mm/shmem.c =A0 =A02011-05-13 14:57:45.367884578 -0700
+++ linux/mm/shmem.c =A0 =A02011-05-17 10:27:19.901934756 -0700
@@ -1293,14 +1293,10 @@ repeat:
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0swappage =3D lookup_swap_cache(swap);
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!swappage) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0shmem_swp_unmap(entry);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock(&info->loc= k);
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* here we actually do the = io */
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (type && !(*type &= amp; VM_FAULT_MAJOR)) {
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __count_vm_ev= ent(PGMAJFAULT);
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mem_cgroup_co= unt_vm_event(current->mm,
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 PGMAJFAULT);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (type)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*type |=3D = VM_FAULT_MAJOR;
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
- =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 spin_unlock(&info->loc= k);
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0swappage =3D shmem_swapin(s= wap, gfp, info, idx);
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!swappage) {
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0spin_lock(&= amp;info->lock);
@@ -1539,7 +1535,10 @@ static int shmem_fault(struct vm_area_st
=A0 =A0 =A0 =A0error =3D shmem_getpage(inode, vmf->pgoff, &vmf->= page, SGP_CACHE, &ret);
=A0 =A0 =A0 =A0if (error)
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return ((error =3D=3D -ENOMEM) ? VM_FAULT_O= OM : VM_FAULT_SIGBUS);
-
+ =A0 =A0 =A0 if (ret & VM_FAULT_MAJOR) {
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 count_vm_event(PGMAJFAULT);
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 mem_cgroup_count_vm_event(vma->vm_mm, PGMA= JFAULT);
+ =A0 =A0 =A0 }
=A0 =A0 =A0 =A0return ret | VM_FAULT_LOCKED;
=A0}

Thank you Hugh for the fix.
=A0
= Acked-by: Ying Han<yinghan@google.com>

--Ying
--0016360e3f5c6d6ab404a37e389b-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org