Re: [PATCH v3 12/17] mm, compaction: more reliably increase direct compaction priority

[Vlastimil Babka]

On 07/06/2016 07:39 AM, Joonsoo Kim wrote:
> On Fri, Jun 24, 2016 at 11:54:32AM +0200, Vlastimil Babka wrote:
>> During reclaim/compaction loop, compaction priority can be increased by the
>> should_compact_retry() function, but the current code is not optimal. Priority
>> is only increased when compaction_failed() is true, which means that compaction
>> has scanned the whole zone. This may not happen even after multiple attempts
>> with the lower priority due to parallel activity, so we might needlessly
>> struggle on the lower priority and possibly run out of compaction retry
>> attempts in the process.
>>
>> We can remove these corner cases by increasing compaction priority regardless
>> of compaction_failed(). Examining further the compaction result can be
>> postponed only after reaching the highest priority. This is a simple solution
>> and we don't need to worry about reaching the highest priority "too soon" here,
>> because hen should_compact_retry() is called it means that the system is
>> already struggling and the allocation is supposed to either try as hard as
>> possible, or it cannot fail at all. There's not much point staying at lower
>> priorities with heuristics that may result in only partial compaction.
>> Also we now count compaction retries only after reaching the highest priority.
> 
> I'm not sure that this patch is safe. Deferring and skip-bit in
> compaction is highly related to reclaim/compaction. Just ignoring them and (almost)
> unconditionally increasing compaction priority will result in less
> reclaim and less success rate on compaction.

I don't see why less reclaim? Reclaim is always attempted before
compaction and compaction priority doesn't affect it. And as long as
reclaim wants to retry, should_compact_retry() isn't even called, so the
priority stays. I wanted to change that in v1, but Michal suggested I
shouldn't.

> And, as a necessarily, it
> would trigger OOM more frequently.

OOM is only allowed for costly orders. If reclaim itself doesn't want to
retry for non-costly orders anymore, and we finally start calling
should_compact_retry(), then I guess the system is really struggling
already and eventual OOM wouldn't be premature?

> It would not be your fault. This patch is reasonable in current
> situation. It just makes current things more deterministic
> although I dislike that current things and this patch would amplify
> those problem.
> 
> Thanks.
>