linux-kernel - Re: [PATCH v5 4/4] add profile information for invalidated page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110218231732.GC13246@csn.ul.ie>
Date:	Fri, 18 Feb 2011 23:17:33 +0000
From:	Mel Gorman <mel@....ul.ie>
To:	Minchan Kim <minchan.kim@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-mm <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Steven Barrett <damentz@...uorix.net>,
	Ben Gamari <bgamari.foss@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Wu Fengguang <fengguang.wu@...el.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Nick Piggin <npiggin@...nel.dk>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Balbir Singh <balbir@...ux.vnet.ibm.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Subject: Re: [PATCH v5 4/4] add profile information for invalidated page

On Sat, Feb 19, 2011 at 07:07:01AM +0900, Minchan Kim wrote:
> Hi Mel,
> 
> On Sat, Feb 19, 2011 at 1:58 AM, Mel Gorman <mel@....ul.ie> wrote:
> > On Fri, Feb 18, 2011 at 12:08:22AM +0900, Minchan Kim wrote:
> >> This patch adds profile information about invalidated page reclaim.
> >> It's just for profiling for test so it is never for merging.
> >>
> >> Acked-by: Rik van Riel <riel@...hat.com>
> >> Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> >> Cc: Wu Fengguang <fengguang.wu@...el.com>
> >> Cc: Johannes Weiner <hannes@...xchg.org>
> >> Cc: Nick Piggin <npiggin@...nel.dk>
> >> Cc: Mel Gorman <mel@....ul.ie>
> >> Signed-off-by: Minchan Kim <minchan.kim@...il.com>
> >> ---
> >>  include/linux/vmstat.h |    4 ++--
> >>  mm/swap.c              |    3 +++
> >>  mm/vmstat.c            |    3 +++
> >>  3 files changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> >> index 833e676..c38ad95 100644
> >> --- a/include/linux/vmstat.h
> >> +++ b/include/linux/vmstat.h
> >> @@ -30,8 +30,8 @@
> >>
> >>  enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
> >>               FOR_ALL_ZONES(PGALLOC),
> >> -             PGFREE, PGACTIVATE, PGDEACTIVATE,
> >> -             PGFAULT, PGMAJFAULT,
> >> +             PGFREE, PGACTIVATE, PGDEACTIVATE, PGINVALIDATE,
> >> +             PGRECLAIM, PGFAULT, PGMAJFAULT,
> >>               FOR_ALL_ZONES(PGREFILL),
> >>               FOR_ALL_ZONES(PGSTEAL),
> >>               FOR_ALL_ZONES(PGSCAN_KSWAPD),
> >> diff --git a/mm/swap.c b/mm/swap.c
> >> index 0a33714..980c17b 100644
> >> --- a/mm/swap.c
> >> +++ b/mm/swap.c
> >> @@ -397,6 +397,7 @@ static void lru_deactivate(struct page *page, struct zone *zone)
> >>                * is _really_ small and  it's non-critical problem.
> >>                */
> >>               SetPageReclaim(page);
> >> +             __count_vm_event(PGRECLAIM);
> >>       } else {
> >>               /*
> >>                * The page's writeback ends up during pagevec
> >
> > Is this name potentially misleading?
> >
> > Pages that are reclaimed are accounted for with _steal. It's not particularly
> > obvious but that's the name it was given. I'd worry that an administrator that
> > was not aware of *_steal would read pgreclaim as "pages that were reclaimed"
> > when this is not necessarily the case.
> >
> > Is there a better name for this? pginvalidate_deferred
> > or pginvalidate_delayed maybe?
> >
> 
> Yep. Your suggestion is fair enough. But as I said in description,
> It's just for testing for my profiling, not merging so I didn't care
> about it. I don't think we need new vmstat of pginvalidate.
> 

My bad. I was treating this piece of information as something we'd keep
around and did not read the introduction clearly enough. If it's just for
evaluation, the name does not matter as long as the reviewers know what it
is. The figures look good and I have no problem with the series. I didn't
ack the memcg parts but only because memcg is not an area I'm familiar enough
for my ack to proper meaning. If there are no other objections, I'd suggest
resubmitting minus this patch.

Thanks.

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/