All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Mel Gorman <mel@csn.ul.ie>
Cc: Rik van Riel <riel@redhat.com>,
	Sasha Levin <levinsasha928@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	jaschut@sandia.gov, minchan@kernel.org,
	kamezawa.hiroyu@jp.fujitsu.com, Dave Jones <davej@redhat.com>
Subject: Re: [PATCH -mm v2] mm: have order > 0 compaction start off where it left
Date: Tue, 3 Jul 2012 14:48:08 -0700	[thread overview]
Message-ID: <20120703144808.4daa4244.akpm@linux-foundation.org> (raw)
In-Reply-To: <20120703101024.GG13141@csn.ul.ie>

On Tue, 3 Jul 2012 11:10:24 +0100
Mel Gorman <mel@csn.ul.ie> wrote:

> > >>>>>+          if (cc->order>   0)
> > >>>>>+                  zone->compact_cached_free_pfn = high_pfn;
> > >>>>
> > >>>>Is high_pfn guaranteed to be aligned to pageblock_nr_pages here?  I
> > >>>>assume so, if lots of code in other places is correct but it's
> > >>>>unobvious from reading this function.
> > >>>
> > >>>Reading the code a few more times, I believe that it is
> > >>>indeed aligned to pageblock size.
> > >>
> > >>I'll slip this into -next for a while.
> > >>
> > >>--- a/mm/compaction.c~isolate_freepages-check-that-high_pfn-is-aligned-as-expected
> > >>+++ a/mm/compaction.c
> > >>@@ -456,6 +456,7 @@ static void isolate_freepages(struct zon
> > >>                 }
> > >>                 spin_unlock_irqrestore(&zone->lock, flags);
> > >>
> > >>+               WARN_ON_ONCE(high_pfn&  (pageblock_nr_pages - 1));
> > >>                 /*
> > >>                  * Record the highest PFN we isolated pages from. When next
> > >>                  * looking for free pages, the search will restart here as
> > >
> > >I've triggered the following with today's -next:
> > 
> > I've been staring at the migrate code for most of the afternoon,
> > and am not sure how this is triggered.
> > 
> 
> That warning is placed in isolate_freepages(). When the migration
> scanner and free scanner have almost met it is possible for high_pfn to
> be
> 
> cc->migrate_pfn + pageblock_nr_pages
> 
> and that is not necessarily pageblock aligned. Forcing it to be aligned
> raises the possibility that the free scanner moves to another zone. This
> is very unlikely but could happen if a high zone was very small.
> 
> I should have caught this when the warning was proposed :( IMO it's
> safe to just drop the warning.

The rest of this patch takes care to ensure that
->compact_cached_free_pfn is aligned to pageblock_nr_pages.  But it now
appears that this particular site will violate that.

What's up?  Do we need to fix this site, or do we remove all that
make-compact_cached_free_pfn-aligned code?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Mel Gorman <mel@csn.ul.ie>
Cc: Rik van Riel <riel@redhat.com>,
	Sasha Levin <levinsasha928@gmail.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	jaschut@sandia.gov, minchan@kernel.org,
	kamezawa.hiroyu@jp.fujitsu.com, Dave Jones <davej@redhat.com>
Subject: Re: [PATCH -mm v2] mm: have order > 0 compaction start off where it left
Date: Tue, 3 Jul 2012 14:48:08 -0700	[thread overview]
Message-ID: <20120703144808.4daa4244.akpm@linux-foundation.org> (raw)
In-Reply-To: <20120703101024.GG13141@csn.ul.ie>

On Tue, 3 Jul 2012 11:10:24 +0100
Mel Gorman <mel@csn.ul.ie> wrote:

> > >>>>>+          if (cc->order>   0)
> > >>>>>+                  zone->compact_cached_free_pfn = high_pfn;
> > >>>>
> > >>>>Is high_pfn guaranteed to be aligned to pageblock_nr_pages here?  I
> > >>>>assume so, if lots of code in other places is correct but it's
> > >>>>unobvious from reading this function.
> > >>>
> > >>>Reading the code a few more times, I believe that it is
> > >>>indeed aligned to pageblock size.
> > >>
> > >>I'll slip this into -next for a while.
> > >>
> > >>--- a/mm/compaction.c~isolate_freepages-check-that-high_pfn-is-aligned-as-expected
> > >>+++ a/mm/compaction.c
> > >>@@ -456,6 +456,7 @@ static void isolate_freepages(struct zon
> > >>                 }
> > >>                 spin_unlock_irqrestore(&zone->lock, flags);
> > >>
> > >>+               WARN_ON_ONCE(high_pfn&  (pageblock_nr_pages - 1));
> > >>                 /*
> > >>                  * Record the highest PFN we isolated pages from. When next
> > >>                  * looking for free pages, the search will restart here as
> > >
> > >I've triggered the following with today's -next:
> > 
> > I've been staring at the migrate code for most of the afternoon,
> > and am not sure how this is triggered.
> > 
> 
> That warning is placed in isolate_freepages(). When the migration
> scanner and free scanner have almost met it is possible for high_pfn to
> be
> 
> cc->migrate_pfn + pageblock_nr_pages
> 
> and that is not necessarily pageblock aligned. Forcing it to be aligned
> raises the possibility that the free scanner moves to another zone. This
> is very unlikely but could happen if a high zone was very small.
> 
> I should have caught this when the warning was proposed :( IMO it's
> safe to just drop the warning.

The rest of this patch takes care to ensure that
->compact_cached_free_pfn is aligned to pageblock_nr_pages.  But it now
appears that this particular site will violate that.

What's up?  Do we need to fix this site, or do we remove all that
make-compact_cached_free_pfn-aligned code?


  reply	other threads:[~2012-07-03 21:48 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-28 17:55 [PATCH -mm v2] mm: have order > 0 compaction start off where it left Rik van Riel
2012-06-28 17:55 ` Rik van Riel
2012-06-28 20:19 ` Jim Schutt
2012-06-28 20:19   ` Jim Schutt
2012-06-28 20:57   ` Rik van Riel
2012-06-28 20:57     ` Rik van Riel
2012-06-28 20:59 ` Andrew Morton
2012-06-28 20:59   ` Andrew Morton
2012-06-28 21:24   ` Rik van Riel
2012-06-28 21:24     ` Rik van Riel
2012-06-28 21:35     ` Andrew Morton
2012-06-28 21:35       ` Andrew Morton
2012-07-02 17:42       ` Sasha Levin
2012-07-02 17:42         ` Sasha Levin
2012-07-03  0:57         ` Rik van Riel
2012-07-03  0:57           ` Rik van Riel
2012-07-03  2:54           ` Minchan Kim
2012-07-03  2:54             ` Minchan Kim
2012-07-03 10:10           ` Mel Gorman
2012-07-03 10:10             ` Mel Gorman
2012-07-03 21:48             ` Andrew Morton [this message]
2012-07-03 21:48               ` Andrew Morton
2012-07-04  2:34               ` Minchan Kim
2012-07-04  2:34                 ` Minchan Kim
2012-07-04  7:42                 ` Andrew Morton
2012-07-04  7:42                   ` Andrew Morton
2012-07-04  8:01                   ` Minchan Kim
2012-07-04  8:01                     ` Minchan Kim
2012-07-11 20:18                     ` [PATCH -mm v3] " Rik van Riel
2012-07-11 20:18                       ` Rik van Riel
2012-07-12  2:26                       ` Minchan Kim
2012-07-12  2:26                         ` Minchan Kim
2012-07-04  9:57                   ` [PATCH -mm v2] " Mel Gorman
2012-07-04  9:57                     ` Mel Gorman
2012-06-28 23:27 ` Minchan Kim
2012-06-28 23:27   ` Minchan Kim
2012-07-03 14:59   ` Rik van Riel
2012-07-03 14:59     ` Rik van Riel
2012-07-04  2:28     ` Minchan Kim
2012-07-04  2:28       ` Minchan Kim
2012-07-04 10:08       ` Mel Gorman
2012-07-04 10:08         ` Mel Gorman
2012-07-03 20:13   ` [PATCH -mm] mm: minor fixes for compaction Rik van Riel
2012-07-03 20:13     ` Rik van Riel
2012-07-04  2:36     ` Minchan Kim
2012-07-04  2:36       ` Minchan Kim
2012-06-29 10:02 ` [PATCH -mm v2] mm: have order > 0 compaction start off where it left Mel Gorman
2012-06-29 10:02   ` Mel Gorman
2012-06-30  3:51 ` Kamezawa Hiroyuki
2012-06-30  3:51   ` Kamezawa Hiroyuki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120703144808.4daa4244.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=jaschut@sandia.gov \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    --cc=minchan@kernel.org \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.