in isolate_migratepages() have check page in LRU twice, the next one
at _isolate_lru_page().
Signed-off-by: Figo.zhang <[email protected]>
---
mm/compaction.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 021a296..ac605cb 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -321,9 +321,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
continue;
}
- if (!PageLRU(page))
- continue;
-
/*
* PageLRU is set, and lru_lock excludes isolation,
* splitting and collapsing (collapsing has already
On Fri, May 06, 2011 at 07:32:46PM +0800, Figo.zhang wrote:
>
> in isolate_migratepages() have check page in LRU twice, the next one
> at _isolate_lru_page().
>
> Signed-off-by: Figo.zhang <[email protected]>
Not checking for PageLRU means that PageTransHuge() gets called
for each page. While the scanner is active and the lock released,
a transparent hugepage can be created and potentially we test
PageTransHuge() on a tail page. This will trigger a BUG if
CONFIG_DEBUG_VM is set.
Nacked-by: Mel Gorman <[email protected]>
>
> mm/compaction.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 021a296..ac605cb 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -321,9 +321,6 @@ static unsigned long isolate_migratepages(struct zone *zone,
> continue;
> }
>
> - if (!PageLRU(page))
> - continue;
> -
> /*
> * PageLRU is set, and lru_lock excludes isolation,
> * splitting and collapsing (collapsing has already
>
>
--
Mel Gorman
SUSE Labs
On Fri, May 06, 2011 at 07:32:46PM +0800, Figo.zhang wrote:
>
> in isolate_migratepages() have check page in LRU twice, the next one
> at _isolate_lru_page().
hugetlbfs or any other compound page won't have PageLRU set and they
may go away from under us leading to compound_order not being reliable
if we remove the PageLRU check before compound_order. So we need to
verify the page is in LRU before running compound_order safely. And if
we hold the lru_lock, the page won't be isolated under us, and we know
it's not going to get splitted either.
We might use compound_trans_order but that's only reliable if run on
the head page so it's not so reliable, and so far it's only used by
memory-failure to "diminish" the risk of races in reading the compound
order, but it's not the best having to use compound_trans_order (and
memory-failure remains unsafe w.r.t to hugetlbfs being released during
hwpoisoning, so compound_trans_order might have to be improved for
it).
On Fri, May 06, 2011 at 02:09:55PM +0100, Mel Gorman wrote:
> On Fri, May 06, 2011 at 07:32:46PM +0800, Figo.zhang wrote:
> >
> > in isolate_migratepages() have check page in LRU twice, the next one
> > at _isolate_lru_page().
> >
> > Signed-off-by: Figo.zhang <[email protected]>
>
> Not checking for PageLRU means that PageTransHuge() gets called
> for each page. While the scanner is active and the lock released,
> a transparent hugepage can be created and potentially we test
> PageTransHuge() on a tail page. This will trigger a BUG if
> CONFIG_DEBUG_VM is set.
Agreed. The compound_order also would become unsafe even if it was
initially an head page (if it's a compound page not in lru). And
compound_trans_order isn't a solution either because we need to be
head for it to be safe like you said, better not having to use
compound_trans_order.