On Thu, 17 Apr 2014 09:07:45 +0900 Minchan Kim <minc...@kernel.org> wrote:
> Hi Vlastimil,
>
> Below just nitpicks.
It seems you were ignored ;)
> > {
> > struct page *page;
> > - unsigned long high_pfn, low_pfn, pfn, z_end_pfn;
> > + unsigned long pfn, low_pfn, next_free_pfn, z_end_pfn;
>
> Could you add comment for each variable?
>
> unsigned long pfn; /* scanning cursor */
> unsigned long low_pfn; /* lowest pfn free scanner is able to scan */
> unsigned long next_free_pfn; /* start pfn for scaning at next truen */
> unsigned long z_end_pfn; /* zone's end pfn */
>
>
> > @@ -688,11 +688,10 @@ static void isolate_freepages(struct zone *zone,
> > low_pfn = ALIGN(cc->migrate_pfn + 1, pageblock_nr_pages);
> >
> > /*
> > - * Take care that if the migration scanner is at the end of the zone
> > - * that the free scanner does not accidentally move to the next zone
> > - * in the next isolation cycle.
> > + * Seed the value for max(next_free_pfn, pfn) updates. If there are
> > + * none, the pfn < low_pfn check will kick in.
>
> "none" what? I'd like to clear more.
I did this:
--- a/mm/compaction.c~mm-compaction-cleanup-isolate_freepages-fix
+++ a/mm/compaction.c
@@ -662,7 +662,10 @@ static void isolate_freepages(struct zon
struct compact_control *cc)
{
struct page *page;
- unsigned long pfn, low_pfn, next_free_pfn, z_end_pfn;
+ unsigned long pfn; /* scanning cursor */
+ unsigned long low_pfn; /* lowest pfn scanner is able to scan */
+ unsigned long next_free_pfn; /* start pfn for scaning at next round */
+ unsigned long z_end_pfn; /* zone's end pfn */
int nr_freepages = cc->nr_freepages;
struct list_head *freelist = &cc->freepages;
@@ -679,8 +682,8 @@ static void isolate_freepages(struct zon
low_pfn = ALIGN(cc->migrate_pfn + 1, pageblock_nr_pages);
/*
- * Seed the value for max(next_free_pfn, pfn) updates. If there are
- * none, the pfn < low_pfn check will kick in.
+ * Seed the value for max(next_free_pfn, pfn) updates. If no pages are
+ * isolated, the pfn < low_pfn check will kick in.
*/
next_free_pfn = 0;
> > @@ -766,9 +765,9 @@ static void isolate_freepages(struct zone *zone,
> > * so that compact_finished() may detect this
> > */
> > if (pfn < low_pfn)
> > - cc->free_pfn = max(pfn, zone->zone_start_pfn);
> > - else
> > - cc->free_pfn = high_pfn;
> > + next_free_pfn = max(pfn, zone->zone_start_pfn);
>
> Why we need max operation?
> IOW, what's the problem if we do (next_free_pfn = pfn)?
An answer to this would be useful, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
