lkml.org 
[lkml]   [2008]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 1/2] mm: Avoid putting a bad page back on the LRU v7
    Date
    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@sgi.com>
    > > > Reviewed-by: Christoph Lameter <clameter@sgi.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.



    \
     
     \ /
      Last update: 2008-07-24 02:57    [W:0.058 / U:0.264 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site