linux-kernel - Re: [PATCH 1/2] mm: Avoid putting a bad page back on the LRU v7

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] [day] [month] [year] [list]

Message-Id: <200807241044.35027.nickpiggin@yahoo.com.au>
Date:	Thu, 24 Jul 2008 10:44:34 +1000
From:	Nick Piggin <nickpiggin@...oo.com.au>
To:	Russ Anderson <rja@....com>
Cc:	mingo@...e.hu, tglx@...utronix.de, Tony Luck <tony.luck@...el.com>,
	linux-kernel@...r.kernel.org, linux-ia64@...r.kernel.org
Subject: Re: [PATCH 1/2] mm: Avoid putting a bad page back on the LRU v7

On Thursday 24 July 2008 08:13, Russ Anderson wrote:
> On Tue, Jul 22, 2008 at 12:42:10PM +1000, Nick Piggin wrote:
> > On Saturday 19 July 2008 06:36, Russ Anderson wrote:
> > > [PATCH 1/2] mm: Avoid putting a bad page back on the LRU v7
> > >
> > > Prevent a page with a physical memory error from being placed back
> > > on the LRU.  A new page flag (PG_memerror) is added if
> > > CONFIG_PAGEFLAGS_EXTENDED is defined.
> > >
> > > Signed-off-by: Russ Anderson <rja@....com>
> > > Reviewed-by: Christoph Lameter <clameter@....com>
> > >
> > > ---
> > >  include/linux/page-flags.h |   18 ++++++++++++++++--
> > >  mm/migrate.c               |   31 ++++++++++++++++++++++++++++++-
> > >  mm/page_alloc.c            |   13 ++++++++-----
> > >  3 files changed, 54 insertions(+), 8 deletions(-)
> > >
> > > Index: linux/mm/page_alloc.c
> > > ===================================================================
> > > --- linux.orig/mm/page_alloc.c	2008-07-18 15:15:48.000000000 -0500
> > > +++ linux/mm/page_alloc.c	2008-07-18 15:16:09.000000000 -0500
> > > @@ -602,10 +602,10 @@ static int prep_new_page(struct page *pa
> > >  		bad_page(page);
> > >
> > >  	/*
> > > -	 * For now, we report if PG_reserved was found set, but do not
> > > -	 * clear it, and do not allocate the page: as a safety net.
> > > +	 * For now, we report if PG_reserved or PG_memerror was found set,
> > > but +	 * do not clear it, and do not allocate the page: as a safety
> > > net. */
> > > -	if (PageReserved(page))
> > > +	if (PageReserved(page) || PageMemError(page))
> > >  		return 1;
> > >
> > >  	page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_reclaim |
> > > @@ -2475,8 +2475,11 @@ static void setup_zone_migrate_reserve(s
> > >  			continue;
> > >  		page = pfn_to_page(pfn);
> > >
> > > -		/* Blocks with reserved pages will never free, skip them. */
> > > -		if (PageReserved(page))
> > > +		/*
> > > +		 * Blocks with reserved pages or memory errors will never
> > > +		 * free, skip them.
> > > +		 */
> > > +		if (PageReserved(page) || PageMemError(page))
> > >  			continue;
> > >
> > >  		block_migratetype = get_pageblock_migratetype(page);
> >
> > I don't like adding more branches like this into fastpaths like this. It
> > would make a lot more sense to me if you just had some private module
> > that does the job of isolating the page from the lru and/or elevating
> > their refcount so that they do not get put back on freelists.
>
> That is how it works.  If PageMemError is set the migration code
> leaves the page with an elevated refcount.  The PageMemError() check
> was to avoid reallocating the page was an additional safty net.
> I'll pull the checks.

Ah, if it's that easy, great, then I have no problems with the patch.
Note that I have no problems with putting a VM_BUG_ON(PageMemError())
or something like that instead -- perhaps not quite the safty net you
would like. But helpful for developers and testers, and it acts as a
helpful little comment.

--
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/