Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1400825 > unrolled thread

Re: [RFC 12/13] mm, compaction: more reliably increase direct compaction priority

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC 12/13] mm, compaction: more reliably increase direct  compaction priority Michal Hocko <mhocko@kernel.org> - 2016-05-13 16:20 +0200
    Re: [RFC 12/13] mm, compaction: more reliably increase direct  compaction priority Vlastimil Babka <vbabka@suse.cz> - 2016-05-16 09:40 +0200
      Re: [RFC 12/13] mm, compaction: more reliably increase direct  compaction priority Michal Hocko <mhocko@kernel.org> - 2016-05-16 10:20 +0200
        Re: [RFC 12/13] mm, compaction: more reliably increase direct  compaction priority Vlastimil Babka <vbabka@suse.cz> - 2016-05-16 11:30 +0200
          Re: [RFC 12/13] mm, compaction: more reliably increase direct  compaction priority Michal Hocko <mhocko@kernel.org> - 2016-05-16 12:00 +0200

#1400825 — Re: [RFC 12/13] mm, compaction: more reliably increase direct compaction priority

On Tue 10-05-16 09:36:02, 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 for
> several reasons:
> 
> - 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.

OK, I can see that this can be changed if we have a guarantee that at
least one full round is guaranteed. Which seems to be the case for the
lowest priority.

> 
> - should_compact_retry() is only called when should_reclaim_retry() returns
>   false. This means that compaction priority cannot get increased as long
>   as reclaim makes sufficient progress. Theoretically, reclaim should stop
>   retrying for high-order allocations as long as the high-order page doesn't
>   exist but due to races, this may result in spurious retries when the
>   high-order page momentarily does exist.

This is intentional behavior and I would like to preserve it if it is
possible. For higher order pages should_reclaim_retry retries as long
as there are some eligible high order pages present which are just hidden
by the watermark check. So this is mostly to get us over watermarks to
start carrying about fragmentation. If we race there then nothing really
terrible should happen and we should eventually converge to a terminal
state.

Does this make sense to you?
-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]

#1401340

On 05/13/2016 04:15 PM, Michal Hocko wrote:
> On Tue 10-05-16 09:36:02, Vlastimil Babka wrote:
>>
>> - should_compact_retry() is only called when should_reclaim_retry() returns
>>    false. This means that compaction priority cannot get increased as long
>>    as reclaim makes sufficient progress. Theoretically, reclaim should stop
>>    retrying for high-order allocations as long as the high-order page doesn't
>>    exist but due to races, this may result in spurious retries when the
>>    high-order page momentarily does exist.
>
> This is intentional behavior and I would like to preserve it if it is
> possible. For higher order pages should_reclaim_retry retries as long
> as there are some eligible high order pages present which are just hidden
> by the watermark check. So this is mostly to get us over watermarks to
> start carrying about fragmentation. If we race there then nothing really
> terrible should happen and we should eventually converge to a terminal
> state.
>
> Does this make sense to you?

Yeah it should work, my only worry was that this may get subtly wrong 
(as experience shows us) and due to e.g. slightly different watermark 
checks and/or a corner-case zone such as ZONE_DMA, 
should_reclaim_retry() would keep returning true, even if reclaim 
couldn't/wouldn't help anything. Then compaction would be needlessly 
kept at ineffective priority.

Also my understanding of the initial compaction priorities is to lower 
the latency if fragmentation is just light and there's enough memory. 
Once we start struggling, I don't see much point in not switching to the 
full compaction priority quickly.

[toc] | [prev] | [next] | [standalone]

#1401352

On Mon 16-05-16 09:31:44, Vlastimil Babka wrote:
> On 05/13/2016 04:15 PM, Michal Hocko wrote:
> > On Tue 10-05-16 09:36:02, Vlastimil Babka wrote:
> > > 
> > > - should_compact_retry() is only called when should_reclaim_retry() returns
> > >    false. This means that compaction priority cannot get increased as long
> > >    as reclaim makes sufficient progress. Theoretically, reclaim should stop
> > >    retrying for high-order allocations as long as the high-order page doesn't
> > >    exist but due to races, this may result in spurious retries when the
> > >    high-order page momentarily does exist.
> > 
> > This is intentional behavior and I would like to preserve it if it is
> > possible. For higher order pages should_reclaim_retry retries as long
> > as there are some eligible high order pages present which are just hidden
> > by the watermark check. So this is mostly to get us over watermarks to
> > start carrying about fragmentation. If we race there then nothing really
> > terrible should happen and we should eventually converge to a terminal
> > state.
> > 
> > Does this make sense to you?
> 
> Yeah it should work, my only worry was that this may get subtly wrong (as
> experience shows us) and due to e.g. slightly different watermark checks
> and/or a corner-case zone such as ZONE_DMA, should_reclaim_retry() would
> keep returning true, even if reclaim couldn't/wouldn't help anything. Then
> compaction would be needlessly kept at ineffective priority.

watermark check for ZONE_DMA should always fail because it fails even
when is completely free to the lowmem reserves. I had a subtle bug in
the original code to check highzone_idx rather than classzone_idx but
that should the fix has been posted recently:
http://lkml.kernel.org/r/1463051677-29418-2-git-send-email-mhocko@kernel.org

> Also my understanding of the initial compaction priorities is to lower the
> latency if fragmentation is just light and there's enough memory. Once we
> start struggling, I don't see much point in not switching to the full
> compaction priority quickly.

That is true but why to compact when there are high order pages and they
are just hidden by the watermark check.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1401391

On 05/16/2016 10:14 AM, Michal Hocko wrote:
> On Mon 16-05-16 09:31:44, Vlastimil Babka wrote:
>>
>> Yeah it should work, my only worry was that this may get subtly wrong (as
>> experience shows us) and due to e.g. slightly different watermark checks
>> and/or a corner-case zone such as ZONE_DMA, should_reclaim_retry() would
>> keep returning true, even if reclaim couldn't/wouldn't help anything. Then
>> compaction would be needlessly kept at ineffective priority.
>
> watermark check for ZONE_DMA should always fail because it fails even
> when is completely free to the lowmem reserves. I had a subtle bug in
> the original code to check highzone_idx rather than classzone_idx but
> that should the fix has been posted recently:
> http://lkml.kernel.org/r/1463051677-29418-2-git-send-email-mhocko@kernel.org

Sure, but that just adds to the experience of being subtly wrong in this 
area :) But sure we can leave this part alone until proven wrong, I 
don't insist strongly.

>> Also my understanding of the initial compaction priorities is to lower the
>> latency if fragmentation is just light and there's enough memory. Once we
>> start struggling, I don't see much point in not switching to the full
>> compaction priority quickly.
>
> That is true but why to compact when there are high order pages and they
> are just hidden by the watermark check.

Compaction should skip such zone regardless of priority.

[toc] | [prev] | [next] | [standalone]

#1401404

On Mon 16-05-16 11:27:56, Vlastimil Babka wrote:
> On 05/16/2016 10:14 AM, Michal Hocko wrote:
> > On Mon 16-05-16 09:31:44, Vlastimil Babka wrote:
[...]
> > > Also my understanding of the initial compaction priorities is to lower the
> > > latency if fragmentation is just light and there's enough memory. Once we
> > > start struggling, I don't see much point in not switching to the full
> > > compaction priority quickly.
> > 
> > That is true but why to compact when there are high order pages and they
> > are just hidden by the watermark check.
> 
> Compaction should skip such zone regardless of priority.

The point I've tried to raise is that we shouldn't conflate the purpose
of the two. The reclaim is here primarily to get us over the watermarks
while compaction is here to form high order pages. If we get both
together the distinction is blured which, I believe, will lead to more
complicated code in the end. I might be wrong here of course but let's
try to have compaction as much wmark check free as possible.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web