On Tue, Sep 11, 2012 at 04:34:55PM -0700, Andrew Morton wrote: > On Mon, 10 Sep 2012 09:18:30 +0800 > Shaohua Li <shli@xxxxxxxxxx> wrote: > > > isolate_migratepages_range() might isolate none pages, for example, when > > zone->lru_lock is contended and compaction is async. In this case, we should > > abort compaction, otherwise, compact_zone will run a useless loop and make > > zone->lru_lock is even contended. > > > > ... > > > > @@ -838,12 +838,14 @@ static unsigned long compact_zone_order( > > .migratetype = allocflags_to_migratetype(gfp_mask), > > .zone = zone, > > .sync = sync, > > - .contended = contended, > > }; > > INIT_LIST_HEAD(&cc.freepages); > > INIT_LIST_HEAD(&cc.migratepages); > > > > - return compact_zone(zone, &cc); > > + ret = compact_zone(zone, &cc); > > + if (contended) > > + *contended = cc.contended; > > + return ret; > > } > > > > From a quick read, `contended' is never NULL here. And defining the "contended" pointer can be null with some caller so the if is needed. The inner code was checking it before. This is also why we couldn't use *contended for the loop break bugfix, because contended could have been null at times. Now cc.contended is always available so we can use that to fix the bug and it's self documenting as well. > interface so that `contended' must be a valid pointer is a good change, > IMO - it results in simpler and faster code. Agreed. > Alas, try_to_compact_pages()'s kerneldoc altogether forgets to describe > this argument. Mel's > mm-compaction-capture-a-suitable-high-order-page-immediately-when-it-is-made-available.patch > adds a `pages' arg and forgets to document that as well. > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>