linux-kernel - Re: CMA broken in next-20120926

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: <201210091308.30306.b.zolnierkie@samsung.com>
Date:	Tue, 09 Oct 2012 13:08:30 +0200
From:	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Minchan Kim <minchan@...nel.org>,
	Thierry Reding <thierry.reding@...onic-design.de>,
	Peter Ujfalusi <peter.ujfalusi@...com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Michal Nazarewicz <mina86@...a86.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Subject: Re: CMA broken in next-20120926

On Tuesday 09 October 2012 12:11:43 Mel Gorman wrote:
> On Tue, Oct 09, 2012 at 10:40:10AM +0200, Bartlomiej Zolnierkiewicz wrote:
> > I also need following patch to make CONFIG_CMA=y && CONFIG_COMPACTION=y case
> > work:
> > 
> > From: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> > Subject: [PATCH] mm: compaction: cache if a pageblock was scanned and no pages were isolated - cma fix
> > 
> > Patch "mm: compaction: cache if a pageblock was scanned and no pages
> > were isolated" needs a following fix to successfully boot next-20121002
> > kernel (same with next-20121008) with CONFIG_CMA=y and CONFIG_COMPACTION=y
> > (with applied -fix1, -fix2, -fix3 patches from Mel Gorman and also with
> > cmatest module from Thierry Reding compiled in).
> > 
> 
> Why is it needed to make it boot? CMA should not care about the

It boots without Thierry's cmatest module but then fails on CMA
allocation attempt (I used out-of-tree /dev/cma_test interface to
generate CMA allocation request from user-space).

> PG_migrate_skip hint being set because it should always ignore it in
> alloc_contig_range() due to cc->ignore_skip_hint. It's not obvious to
> me why this fixes a boot failure and I wonder if it's papering over some
> underlying problem. Can you provide more details please?

I just compared CONFIG_COMPACTION=n and =y cases initially, figured
out the difference and did the change.  However on a closer look it
seems that {get,clear,set}_pageblock_skip() use incorrect bit ranges
(please compare to bit ranges used by {get,set}_pageblock_flags()
used for migration types) and can overwrite pageblock migratetype of
the next pageblock in the bitmap (I wonder how could this code ever
worked before?).

> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
> > ---
> >  mm/compaction.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Index: b/mm/compaction.c
> > ===================================================================
> > --- a/mm/compaction.c	2012-10-08 18:10:53.491679716 +0200
> > +++ b/mm/compaction.c	2012-10-08 18:11:33.615679713 +0200
> > @@ -117,7 +117,8 @@ static void update_pageblock_skip(struct
> >  			bool migrate_scanner)
> >  {
> >  	struct zone *zone = cc->zone;
> > -	if (!page)
> > +
> > +	if (!page || cc->ignore_skip_hint)
> >  		return;
> >  
> >  	if (!nr_isolated) {

The patch below also fixes the issue for me:

From: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: [PATCH] mm: compaction: fix bit ranges in {get,clear,set}_pageblock_skip() 

{get,clear,set}_pageblock_skip() use incorrect bit ranges (please compare
to bit ranges used by {get,set}_pageblock_flags() used for migration types)
and can overwrite pageblock migratetype of the next pageblock in the bitmap.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
---
 include/linux/pageblock-flags.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: b/include/linux/pageblock-flags.h
===================================================================
--- a/include/linux/pageblock-flags.h	2012-10-09 12:50:20.366340001 +0200
+++ b/include/linux/pageblock-flags.h	2012-10-09 12:50:31.794339996 +0200
@@ -71,13 +71,13 @@ void set_pageblock_flags_group(struct pa
 #ifdef CONFIG_COMPACTION
 #define get_pageblock_skip(page) \
 			get_pageblock_flags_group(page, PB_migrate_skip,     \
-							PB_migrate_skip + 1)
+							PB_migrate_skip)
 #define clear_pageblock_skip(page) \
 			set_pageblock_flags_group(page, 0, PB_migrate_skip,  \
-							PB_migrate_skip + 1)
+							PB_migrate_skip)
 #define set_pageblock_skip(page) \
 			set_pageblock_flags_group(page, 1, PB_migrate_skip,  \
-							PB_migrate_skip + 1)
+							PB_migrate_skip)
 #endif /* CONFIG_COMPACTION */
 
 #define get_pageblock_flags(page) \

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