Re: [PATCH mmotm] add the pagefault count into memcg stats: shmem fix

[Ying Han <yinghan@google.com>]

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

On Tue, May 17, 2011 at 11:24 AM, Hugh Dickins <hughd@google.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@google.com>
> ---
>
>  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<yinghan@google.com>

--Ying

[-- Attachment #2: Type: text/html, Size: 3366 bytes --]