Re: [PATCH v3 13/17] mm, compaction: use correct watermark when checking allocation success

[Vlastimil Babka]

On 07/06/2016 07:47 AM, Joonsoo Kim wrote:
> On Fri, Jun 24, 2016 at 11:54:33AM +0200, Vlastimil Babka wrote:
> > The __compact_finished() function uses low watermark in a check that has to
> > pass if the direct compaction is to finish and allocation should succeed. This
> > is too pessimistic, as the allocation will typically use min watermark. It may
> > happen that during compaction, we drop below the low watermark (due to parallel
> > activity), but still form the target high-order page. By checking against low
> > watermark, we might needlessly continue compaction.
> >
> > Similarly, __compaction_suitable() uses low watermark in a check whether
> > allocation can succeed without compaction. Again, this is unnecessarily
> > pessimistic.
> >
> > After this patch, these check will use direct compactor's alloc_flags to
> > determine the watermark, which is effectively the min watermark.
> >
> > Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
> > Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> > ---
> >  mm/compaction.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/compaction.c b/mm/compaction.c
> > index 76897850c3c2..371760a85085 100644
> > --- a/mm/compaction.c
> > +++ b/mm/compaction.c
> > @@ -1320,7 +1320,7 @@ static enum compact_result __compact_finished(struct zone *zone, struct compact_
> >  		return COMPACT_CONTINUE;
> >
> >  	/* Compaction run is not finished if the watermark is not met */
> > -	watermark = low_wmark_pages(zone);
> > +	watermark = zone->watermark[cc->alloc_flags & ALLOC_WMARK_MASK];
>
> finish condition is changed. We have two more watermark checks in
> try_to_compact_pages() and kcompactd_do_work() and they should be
> changed too.

Ugh, I've completely missed them. Thanks for catching this, hopefully 
fixing that will improve the results.