Re: [PATCH v3 2/8] mm, compaction: remove redundant watermark check in compact_finished()

From: Joonsoo Kim
Date: Wed Mar 15 2017 - 21:29:05 EST


Hello,

On Tue, Mar 07, 2017 at 02:15:39PM +0100, Vlastimil Babka wrote:
> When detecting whether compaction has succeeded in forming a high-order page,
> __compact_finished() employs a watermark check, followed by an own search for
> a suitable page in the freelists. This is not ideal for two reasons:
>
> - The watermark check also searches high-order freelists, but has a less strict
> criteria wrt fallback. It's therefore redundant and waste of cycles. This was
> different in the past when high-order watermark check attempted to apply
> reserves to high-order pages.

Although it looks redundant now, I don't like removal of the watermark
check here. Criteria in watermark check would be changed to more strict
later and we would easily miss to apply it on compaction side if the
watermark check is removed.

>
> - The watermark check might actually fail due to lack of order-0 pages.
> Compaction can't help with that, so there's no point in continuing because of
> that. It's possible that high-order page still exists and it terminates.

If lack of order-0 pages is the reason for stopping compaction, we
need to insert the watermark check for order-0 to break the compaction
instead of removing it. Am I missing something?

Thanks.