All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Wanpeng Li <liwp.linux@gmail.com>
Cc: trivial@kernel.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	Michal Hocko <mhocko@suse.cz>,
	Balbir Singh <bsingharora@gmail.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Tejun Heo <tj@kernel.org>, Li Zefan <lizefan@huawei.com>,
	Christoph Lameter <cl@linux-foundation.org>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Milton Miller <miltonm@bga.com>,
	Nishanth Aravamudan <nacc@us.ibm.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Jason Wessel <jason.wessel@windriver.com>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	David Howells <dhowells@redhat.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>, Andrew Morton <>
Subject: Re: [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error
Date: Fri, 15 Jun 2012 16:58:09 +0200	[thread overview]
Message-ID: <20120615145809.GA27816@cmpxchg.org> (raw)
In-Reply-To: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com>

On Fri, Jun 15, 2012 at 09:19:45PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> Since there are five lists in LRU cache, the array nr in get_scan_count
> should be:
> 
> nr[0] = anon inactive pages to scan; nr[1] = anon active pages to scan
> nr[2] = file inactive pages to scan; nr[3] = file active pages to scan
> 
> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> 
> ---
>  mm/vmscan.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index eeb3bc9..ed823df 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1567,7 +1567,8 @@ static int vmscan_swappiness(struct scan_control *sc)
>   * by looking at the fraction of the pages scanned we did rotate back
>   * onto the active list instead of evict.
>   *
> - * nr[0] = anon pages to scan; nr[1] = file pages to scan
> + * nr[0] = anon inactive pages to scan; nr[1] = anon active pages to scan
> + * nr[2] = file inactive pages to scan; nr[3] = file active pages to scan
>   */

Does including this in the comment have any merit in the first place?
We never access nr[0] or nr[1] etc. anywhere with magic numbers.  It's
a local function with one callsite, the passed array is declared and
accessed exclusively by what is defined in enum lru_list, where is the
point in repeating the enum items?.  I'd rather the next change to
this comment would be its removal.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Wanpeng Li <liwp.linux@gmail.com>
Cc: Christoph Lameter <cl@linux-foundation.org>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	linux-pci@vger.kernel.org,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	David Howells <dhowells@redhat.com>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Larry Woodman <lwoodman@redhat.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Gavin Shan <shangw@linux.vnet.ibm.com>,
	x86@kernel.org, Hugh Dickins <hughd@google.com>,
	Ingo Molnar <mingo@redhat.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Nishanth Aravamudan <nacc@us.ibm.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Mel Gorman <mel@csn.ul.ie>, David Rientjes <rientjes@google.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Bjorn Helgaas <bhelgaas@google.com>,
	cgroups@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Michal Hocko <mhocko@suse.cz>,
	trivial@kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Milton Miller <miltonm@bga.com>,
	Minchan Kim <minchan@kernel.org>, Li Zefan <lizefan@huawei.com>,
	Jason Wessel <jason.wessel@windriver.com>,
	Tejun Heo <tj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error
Date: Fri, 15 Jun 2012 16:58:09 +0200	[thread overview]
Message-ID: <20120615145809.GA27816@cmpxchg.org> (raw)
In-Reply-To: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com>

On Fri, Jun 15, 2012 at 09:19:45PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> Since there are five lists in LRU cache, the array nr in get_scan_count
> should be:
> 
> nr[0] = anon inactive pages to scan; nr[1] = anon active pages to scan
> nr[2] = file inactive pages to scan; nr[3] = file active pages to scan
> 
> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> 
> ---
>  mm/vmscan.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index eeb3bc9..ed823df 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1567,7 +1567,8 @@ static int vmscan_swappiness(struct scan_control *sc)
>   * by looking at the fraction of the pages scanned we did rotate back
>   * onto the active list instead of evict.
>   *
> - * nr[0] = anon pages to scan; nr[1] = file pages to scan
> + * nr[0] = anon inactive pages to scan; nr[1] = anon active pages to scan
> + * nr[2] = file inactive pages to scan; nr[3] = file active pages to scan
>   */

Does including this in the comment have any merit in the first place?
We never access nr[0] or nr[1] etc. anywhere with magic numbers.  It's
a local function with one callsite, the passed array is declared and
accessed exclusively by what is defined in enum lru_list, where is the
point in repeating the enum items?.  I'd rather the next change to
this comment would be its removal.

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Wanpeng Li <liwp.linux@gmail.com>
Cc: trivial@kernel.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
	Michal Hocko <mhocko@suse.cz>,
	Balbir Singh <bsingharora@gmail.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Tejun Heo <tj@kernel.org>, Li Zefan <lizefan@huawei.com>,
	Christoph Lameter <cl@linux-foundation.org>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Milton Miller <miltonm@bga.com>,
	Nishanth Aravamudan <nacc@us.ibm.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Jason Wessel <jason.wessel@windriver.com>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	David Howells <dhowells@redhat.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mel@csn.ul.ie>, Minchan Kim <minchan@kernel.org>,
	Gavin Shan <shangw@linux.vnet.ibm.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Larry Woodman <lwoodman@redhat.com>,
	Hugh Dickins <hughd@google.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, linux-mm@kvack.org,
	cgroups@vger.kernel.org
Subject: Re: [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error
Date: Fri, 15 Jun 2012 16:58:09 +0200	[thread overview]
Message-ID: <20120615145809.GA27816@cmpxchg.org> (raw)
In-Reply-To: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com>

On Fri, Jun 15, 2012 at 09:19:45PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <liwp@linux.vnet.ibm.com>
> 
> Since there are five lists in LRU cache, the array nr in get_scan_count
> should be:
> 
> nr[0] = anon inactive pages to scan; nr[1] = anon active pages to scan
> nr[2] = file inactive pages to scan; nr[3] = file active pages to scan
> 
> Signed-off-by: Wanpeng Li <liwp.linux@gmail.com>
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Acked-by: Minchan Kim <minchan@kernel.org>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> 
> ---
>  mm/vmscan.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index eeb3bc9..ed823df 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1567,7 +1567,8 @@ static int vmscan_swappiness(struct scan_control *sc)
>   * by looking at the fraction of the pages scanned we did rotate back
>   * onto the active list instead of evict.
>   *
> - * nr[0] = anon pages to scan; nr[1] = file pages to scan
> + * nr[0] = anon inactive pages to scan; nr[1] = anon active pages to scan
> + * nr[2] = file inactive pages to scan; nr[3] = file active pages to scan
>   */

Does including this in the comment have any merit in the first place?
We never access nr[0] or nr[1] etc. anywhere with magic numbers.  It's
a local function with one callsite, the passed array is declared and
accessed exclusively by what is defined in enum lru_list, where is the
point in repeating the enum items?.  I'd rather the next change to
this comment would be its removal.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-06-15 14:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-15 13:19 [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error Wanpeng Li
2012-06-15 13:19 ` Wanpeng Li
2012-06-15 13:19 ` Wanpeng Li
2012-06-15 14:58 ` Johannes Weiner [this message]
2012-06-15 14:58   ` Johannes Weiner
2012-06-15 14:58   ` Johannes Weiner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120615145809.GA27816@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=benh@kernel.crashing.org \
    --cc=bhelgaas@google.com \
    --cc=bsingharora@gmail.com \
    --cc=cl@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jason.wessel@windriver.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=liwp.linux@gmail.com \
    --cc=lizefan@huawei.com \
    --cc=mhocko@suse.cz \
    --cc=miltonm@bga.com \
    --cc=mingo@redhat.com \
    --cc=nacc@us.ibm.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=sfr@canb.auug.org.au \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=trivial@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.