On Thu, Jan 05, 2012 at 06:31:14PM +0800, Huang Shijie wrote:

quoted

Why?


Returning ISOLATE_SUCCESS means that we fall through. This means busy
work in migrate_pages(), updating list accounting and the list. It's
wasteful but is it functionally incorrect? What problem did you observe?

there may are many times the cc->migratepages is zero, but the
return value is ISOLATE_SUCCESS.

Ok, this is a reasonable assertion. How often will depend on a large
number of factors.

quoted

If this is simply a performance issue then minimally COMPACTBLOCKS

yes, My concern is the performance.

For future reference, please explain this in the changelog.

the comment of ISOLATE_NONE makes me confused.  :(

I see your confusion. The main difference between ISOLATE_NONE and
ISOLATE_SUCCESS is that scanning within the pageblock took place even
if no pages were isolated by the scan. Maybe it would be easier if
your patch clarified the meaning of the return values. Something like;

typedef enum {
        ISOLATE_ABORT,          /* Abort compaction now */
        ISOLATE_NONE,           /* No pages scanned, consider next pageblock */
        ISOLATE_SUCCESS,        /* Pages scanned and maybe isolated, migrate */
} isolate_migrate_t;

and then check if pages were really isolated on ISOLATE_SUCCESS?

If you think we should update the COMPACTBLOCK in this case, my
patch is wrong.

I think the overhead is unnecessary and worth fixing but because the
scanning took place, the COMPACTBLOCK counter should be bumped and
the tracepoint triggered.

quoted

still needs to be updated, we still want to see the tracepoint etc. To

ok.

Thanks.

-- 
Mel Gorman
SUSE Labs

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>