From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S936504AbcIGM65 (ORCPT <rfc822;w@1wt.eu>);
        Wed, 7 Sep 2016 08:58:57 -0400
Received: from mx1.redhat.com ([209.132.183.28]:52863 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S932705AbcIGM6z (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 7 Sep 2016 08:58:55 -0400
Date: Wed, 7 Sep 2016 14:58:07 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: robert.foss@collabora.com
Cc: corbet@lwn.net, akpm@linux-foundation.org, vbabka@suse.cz,
        hughd@google.com, mhocko@suse.com, koct9i@gmail.com,
        n-horiguchi@ah.jp.nec.com, kirill.shutemov@linux.intel.com,
        john.stultz@linaro.org, minchan@kernel.org,
        ross.zwisler@linux.intel.com, jmarchan@redhat.com, hannes@cmpxchg.org,
        keescook@chromium.org, viro@zeniv.linux.org.uk, mguzik@redhat.com,
        jdanis@google.com, calvinowens@fb.com, adobriyan@gmail.com,
        ebiederm@xmission.com, sonnyrao@chromium.org,
        seth.forshee@canonical.com, tixxdz@gmail.com,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        Ben Zhang <benzh@chromium.org>, Bryan Freed <bfreed@chromium.org>,
        Filipe Brandenburger <filbranden@chromium.org>,
        Jann Horn <jann@thejh.net>, Michal Hocko <mhocko@kernel.org>,
        linux-api@vger.kernel.org, Jacek Anaszewski <j.anaszewski@samsung.com>
Subject: Re: [PATCH v5 1/3] mm, proc: Implement /proc/<pid>/totmaps
Message-ID: <20160907125806.GA3849@redhat.com>
References: <1473106449-12847-1-git-send-email-robert.foss@collabora.com> <1473106449-12847-2-git-send-email-robert.foss@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1473106449-12847-2-git-send-email-robert.foss@collabora.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 07 Sep 2016 12:58:54 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 09/05, robert.foss@collabora.com wrote:
>
> @@ -2854,6 +2854,7 @@ static const struct pid_entry tgid_base_stuff[] = {
>  	REG("clear_refs", S_IWUSR, proc_clear_refs_operations),
>  	REG("smaps",      S_IRUGO, proc_pid_smaps_operations),
>  	REG("pagemap",    S_IRUSR, proc_pagemap_operations),
> +	REG("totmaps",    S_IRUGO, proc_totmaps_operations),

I must have missed something, but I fail to understand why this patch
is so complicated.

Just use ONE("totmaps", S_IRUGO, proc_totmaps_operations) ?

> +static int totmaps_proc_show(struct seq_file *m, void *data)
> +{
> +	struct proc_maps_private *priv = m->private;
> +	struct mm_struct *mm = priv->mm;
> +	struct vm_area_struct *vma;
> +	struct mem_size_stats mss_sum;
> +
> +	memset(&mss_sum, 0, sizeof(mss_sum));
> +	down_read(&mm->mmap_sem);
> +	hold_task_mempolicy(priv);
        ^^^^^^^^^^^^^^^^^^^^^^^^^
why?

> +	for (vma = mm->mmap; vma != priv->tail_vma; vma = vma->vm_next) {

Hmm. the usage of ->tail_vma looks just wrong. I guess the code should
work because it is NULL but still.

> +		struct mem_size_stats mss;
> +		struct mm_walk smaps_walk = {
> +			.pmd_entry = smaps_pte_range,
> +			.mm = vma->vm_mm,
> +			.private = &mss,
> +		};
> +
> +		if (vma->vm_mm && !is_vm_hugetlb_page(vma)) {
> +			memset(&mss, 0, sizeof(mss));
> +			walk_page_vma(vma, &smaps_walk);
> +			add_smaps_sum(&mss, &mss_sum);
> +		}
> +	}

Why? I mean, why not walk_page_range() ? You do not need this for-each-vma
loop at all? At least if you change this patch to use the ONE() helper, and
everything else looks unneeded in this case.

Oleg.