From mboxrd@z Thu Jan 1 00:00:00 1970 From: Johannes Weiner Subject: Re: [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error Date: Fri, 15 Jun 2012 16:58:09 +0200 Message-ID: <20120615145809.GA27816@cmpxchg.org> References: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com> Mime-Version: 1.0 Return-path: Content-Disposition: inline In-Reply-To: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com> Sender: owner-linux-mm@kvack.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Wanpeng Li Cc: trivial@kernel.org, Benjamin Herrenschmidt , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Bjorn Helgaas , Michal Hocko , Balbir Singh , KAMEZAWA Hiroyuki , Tejun Heo , Li Zefan , Christoph Lameter , Paul Gortmaker , Jesse Barnes , Milton Miller , Nishanth Aravamudan , Stephen Rothwell , Peter Zijlstra , Jason Wessel , Jan Kiszka , David Howells , Srikar Dronamraju , Andrew Morton <> On Fri, Jun 15, 2012 at 09:19:45PM +0800, Wanpeng Li wrote: > From: Wanpeng Li > > 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 > Acked-by: KOSAKI Motohiro > Acked-by: Minchan Kim > Reviewed-by: Rik van Riel > > --- > 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: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zene.cmpxchg.org (zene.cmpxchg.org [IPv6:2a01:238:4224:fa00:ca1f:9ef3:caee:a2bd]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id B0B73B70D3 for ; Sat, 16 Jun 2012 02:17:21 +1000 (EST) Date: Fri, 15 Jun 2012 16:58:09 +0200 From: Johannes Weiner To: Wanpeng Li Subject: Re: [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error Message-ID: <20120615145809.GA27816@cmpxchg.org> References: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com> Cc: Christoph Lameter , Srikar Dronamraju , linux-pci@vger.kernel.org, Jesse Barnes , David Howells , Paul Gortmaker , "H. Peter Anvin" , Larry Woodman , Andrea Arcangeli , Stephen Rothwell , Gavin Shan , x86@kernel.org, Hugh Dickins , Ingo Molnar , KOSAKI Motohiro , Jan Kiszka , Nishanth Aravamudan , Peter Zijlstra , Mel Gorman , David Rientjes , Al Viro , Bjorn Helgaas , cgroups@vger.kernel.org, Thomas Gleixner , KAMEZAWA Hiroyuki , Michal Hocko , trivial@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Milton Miller , Minchan Kim , Li Zefan , Jason Wessel , Tejun Heo , Andrew Morton , linuxppc-dev@lists.ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, Jun 15, 2012 at 09:19:45PM +0800, Wanpeng Li wrote: > From: Wanpeng Li > > 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 > Acked-by: KOSAKI Motohiro > Acked-by: Minchan Kim > Reviewed-by: Rik van Riel > > --- > 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx207.postini.com [74.125.245.207]) by kanga.kvack.org (Postfix) with SMTP id 13AF26B0068 for ; Fri, 15 Jun 2012 11:02:27 -0400 (EDT) Date: Fri, 15 Jun 2012 16:58:09 +0200 From: Johannes Weiner Subject: Re: [PATCH 6/7][TRIVIAL][resend] mm: cleanup page reclaim comment error Message-ID: <20120615145809.GA27816@cmpxchg.org> References: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1339766387-7740-1-git-send-email-liwp.linux@gmail.com> Sender: owner-linux-mm@kvack.org List-ID: To: Wanpeng Li Cc: trivial@kernel.org, Benjamin Herrenschmidt , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Bjorn Helgaas , Michal Hocko , Balbir Singh , KAMEZAWA Hiroyuki , Tejun Heo , Li Zefan , Christoph Lameter , Paul Gortmaker , Jesse Barnes , Milton Miller , Nishanth Aravamudan , Stephen Rothwell , Peter Zijlstra , Jason Wessel , Jan Kiszka , David Howells , Srikar Dronamraju , Andrew Morton , Mel Gorman , Minchan Kim , Gavin Shan , Al Viro , Andrea Arcangeli , David Rientjes , KOSAKI Motohiro , Larry Woodman , Hugh Dickins , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org On Fri, Jun 15, 2012 at 09:19:45PM +0800, Wanpeng Li wrote: > From: Wanpeng Li > > 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 > Acked-by: KOSAKI Motohiro > Acked-by: Minchan Kim > Reviewed-by: Rik van Riel > > --- > 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: email@kvack.org