On Wed 10-08-16 11:12:19, Vlastimil Babka wrote: > Joonsoo has reminded me that in a later patch changing watermark checks > throughout compaction I forgot to update checks in try_to_compact_pages() and > compactd_do_work(). Closer inspection however shows that they are redundant > now > that compact_zone() reliably reports success with COMPACT_SUCCESS, as they > just > repeat (a subset) of checks that have just passed. So instead of checking > watermarks again, just test the return value.
the less watermark checks we do the better because they just increase a probability of subtle and hard to explain corner cases. > Also remove the stray "bool success" variable from kcompactd_do_work(). > > Reported-by: Joonsoo Kim <[email protected]> > Signed-off-by: Vlastimil Babka <[email protected]> Acked-by: Michal Hocko <[email protected]> > --- > mm/compaction.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index c355bf0d8599..a144f58f7193 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1698,9 +1698,8 @@ enum compact_result try_to_compact_pages(gfp_t > gfp_mask, unsigned int order, > alloc_flags, ac_classzone_idx(ac)); > rc = max(status, rc); > > - /* If a normal allocation would succeed, stop compacting */ > - if (zone_watermark_ok(zone, order, low_wmark_pages(zone), > - ac_classzone_idx(ac), alloc_flags)) { > + /* The allocation should succeed, stop compacting */ > + if (status == COMPACT_SUCCESS) { > /* > * We think the allocation will succeed in this zone, > * but it is not certain, hence the false. The caller > @@ -1873,8 +1872,6 @@ static void kcompactd_do_work(pg_data_t *pgdat) > .ignore_skip_hint = true, > > }; > - bool success = false; > - > trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order, > cc.classzone_idx); > count_vm_event(KCOMPACTD_WAKE); > @@ -1903,9 +1900,7 @@ static void kcompactd_do_work(pg_data_t *pgdat) > return; > status = compact_zone(zone, &cc); > > - if (zone_watermark_ok(zone, cc.order, low_wmark_pages(zone), > - cc.classzone_idx, 0)) { > - success = true; > + if (status == COMPACT_SUCCESS) { > compaction_defer_reset(zone, cc.order, false); > } else if (status == COMPACT_PARTIAL_SKIPPED || status == > COMPACT_COMPLETE) { > /* > -- > 2.9.2 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to [email protected]. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"[email protected]";> [email protected] </a> -- Michal Hocko SUSE Labs
