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

Groups > linux.kernel > #1443070 > unrolled thread

Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether kswapd sleeps

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: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-14 07:20 +0200
    Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Vlastimil Babka <vbabka@suse.cz> - 2016-07-14 10:40 +0200
      Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-18 07:10 +0200
        Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Vlastimil Babka <vbabka@suse.cz> - 2016-07-18 09:00 +0200
          Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-18 09:30 +0200
    Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Mel Gorman <mgorman@techsingularity.net> - 2016-07-14 11:10 +0200
      Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether  kswapd sleeps Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-07-18 07:00 +0200

#1443070 — Re: [PATCH 08/31] mm, vmscan: simplify the logic deciding whether kswapd sleeps

On Fri, Jul 08, 2016 at 11:11:47AM +0100, Mel Gorman wrote:
> On Fri, Jul 08, 2016 at 11:44:47AM +0900, Joonsoo Kim wrote:
> > > > > @@ -3390,12 +3386,24 @@ static int kswapd(void *p)
> > > > >  		 * We can speed up thawing tasks if we don't call balance_pgdat
> > > > >  		 * after returning from the refrigerator
> > > > >  		 */
> > > > > -		if (!ret) {
> > > > > -			trace_mm_vmscan_kswapd_wake(pgdat->node_id, order);
> > > > > +		if (ret)
> > > > > +			continue;
> > > > >  
> > > > > -			/* return value ignored until next patch */
> > > > > -			balance_pgdat(pgdat, order, classzone_idx);
> > > > > -		}
> > > > > +		/*
> > > > > +		 * Reclaim begins at the requested order but if a high-order
> > > > > +		 * reclaim fails then kswapd falls back to reclaiming for
> > > > > +		 * order-0. If that happens, kswapd will consider sleeping
> > > > > +		 * for the order it finished reclaiming at (reclaim_order)
> > > > > +		 * but kcompactd is woken to compact for the original
> > > > > +		 * request (alloc_order).
> > > > > +		 */
> > > > > +		trace_mm_vmscan_kswapd_wake(pgdat->node_id, alloc_order);
> > > > > +		reclaim_order = balance_pgdat(pgdat, alloc_order, classzone_idx);
> > > > > +		if (reclaim_order < alloc_order)
> > > > > +			goto kswapd_try_sleep;
> > > > 
> > > > This 'goto' would cause kswapd to sleep prematurely. We need to check
> > > > *new* pgdat->kswapd_order and classzone_idx even in this case.
> > > > 
> > > 
> > > It only matters if the next request coming is also high-order requests but
> > > one thing that needs to be avoided is kswapd staying awake periods of time
> > > constantly reclaiming for high-order pages. This is why the check means
> > > "If we reclaimed for high-order and failed, then consider sleeping now".
> > > If allocations still require it, they direct reclaim instead.
> > 
> > But, assume that next request is zone-constrained allocation. We need
> > to balance memory for it but kswapd would skip it.
> > 
> 
> Then it'll also be woken up again in the very near future as the
> zone-constrained allocation. If the zone is at the min watermark, then
> it'll have direct reclaimed but between min and low, it'll be a simple
> wakeup.
> 
> The premature sleep, wakeup with new requests logic was a complete mess.
> However, what I did do is remove the -1 handling of kswapd_classzone_idx
> handling and the goto full-sleep. In the event of a premature wakeup,
> it'll recheck for wakeups and if none has occured, it'll use the old
> classzone information.
> 
> Note that it will *not* use the original allocation order if it's a
> premature sleep. This is because it's known that high-order reclaim
> failed in the near past and restarting it has a high risk of
> overreclaiming.
> 
> > > > And, I'd like to know why max() is used for classzone_idx rather than
> > > > min()? I think that kswapd should balance the lowest zone requested.
> > > > 
> > > 
> > > If there are two allocation requests -- one zone-constraned and the other
> > > zone-unconstrained, it does not make sense to have kswapd skip the pages
> > > usable for the zone-unconstrained and waste a load of CPU. You could
> > 
> > I agree that, in this case, it's not good to skip the pages usable
> > for the zone-unconstrained request. But, what I am concerned is that
> > kswapd stop reclaim prematurely in the view of zone-constrained
> > requestor.
> 
> It doesn't stop reclaiming for the lower zones. It's reclaiming the LRU
> for the whole node that may or may not have lower zone pages at the end
> of the LRU. If it does, then the allocation request will be satisfied.
> If it does not, then kswapd will think the node is balanced and get
> rewoken to do a zone-constrained reclaim pass.

If zone-constrained request could go direct reclaim pass, there would
be no problem. But, please assume that request is zone-constrained
without __GFP_DIRECT_RECLAIM which is common for some device driver
implementation. And, please assume one more thing that this request
always comes with zone-unconstrained allocation request. In this case,
your max() logic will set kswapd_classzone_idx to highest zone index
and re-worken kswapd would not balance for low zone again. In the end,
zone-constrained allocation request without __GFP_DIRECT_RECLAIM could
fail.

Thanks.

[toc] | [next] | [standalone]

#1443258

On 07/14/2016 07:23 AM, Joonsoo Kim wrote:
> On Fri, Jul 08, 2016 at 11:11:47AM +0100, Mel Gorman wrote:
>> On Fri, Jul 08, 2016 at 11:44:47AM +0900, Joonsoo Kim wrote:
>>
>> It doesn't stop reclaiming for the lower zones. It's reclaiming the LRU
>> for the whole node that may or may not have lower zone pages at the end
>> of the LRU. If it does, then the allocation request will be satisfied.
>> If it does not, then kswapd will think the node is balanced and get
>> rewoken to do a zone-constrained reclaim pass.
>
> If zone-constrained request could go direct reclaim pass, there would
> be no problem. But, please assume that request is zone-constrained
> without __GFP_DIRECT_RECLAIM which is common for some device driver
> implementation. And, please assume one more thing that this request
> always comes with zone-unconstrained allocation request. In this case,
> your max() logic will set kswapd_classzone_idx to highest zone index
> and re-worken kswapd would not balance for low zone again. In the end,
> zone-constrained allocation request without __GFP_DIRECT_RECLAIM could
> fail.

I don't think there's a problem in the scenario? Kswapd will keep being 
woken up and reclaim from the node lru. It will hit and free any low 
zone pages that are on the lru, even though it doesn't "balance for low 
zone". Eventually it will either satisfy the constrained allocation by 
reclaiming those low-zone pages during the repeated wakeups, or the 
low-zone wakeups will stop coming together with higher-zone wakeups and 
then it will reclaim the low-zone pages in a single low-zone wakeup. If 
the zone-constrained request is not allowed to fail, then it will just 
keep waking up kswapd and waiting for the progress. If it's allowed to 
fail (i.e. not __GFP_NOFAIL), but not allowed to direct reclaim, it goes 
"goto nopage" rather quickly in __alloc_pages_slowpath(), without any 
waiting for kswapd's progress, so there's not really much difference 
whether the kswapd wakeup picked up a low classzone or not. Note the 
__GFP_NOFAIL but ~__GFP_DIRECT_RECLAIM is a WARN_ON_ONCE() scenario, so 
definitely not common...

> Thanks.
>

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

#1445209

On Thu, Jul 14, 2016 at 10:32:09AM +0200, Vlastimil Babka wrote:
> On 07/14/2016 07:23 AM, Joonsoo Kim wrote:
> >On Fri, Jul 08, 2016 at 11:11:47AM +0100, Mel Gorman wrote:
> >>On Fri, Jul 08, 2016 at 11:44:47AM +0900, Joonsoo Kim wrote:
> >>
> >>It doesn't stop reclaiming for the lower zones. It's reclaiming the LRU
> >>for the whole node that may or may not have lower zone pages at the end
> >>of the LRU. If it does, then the allocation request will be satisfied.
> >>If it does not, then kswapd will think the node is balanced and get
> >>rewoken to do a zone-constrained reclaim pass.
> >
> >If zone-constrained request could go direct reclaim pass, there would
> >be no problem. But, please assume that request is zone-constrained
> >without __GFP_DIRECT_RECLAIM which is common for some device driver
> >implementation. And, please assume one more thing that this request
> >always comes with zone-unconstrained allocation request. In this case,
> >your max() logic will set kswapd_classzone_idx to highest zone index
> >and re-worken kswapd would not balance for low zone again. In the end,
> >zone-constrained allocation request without __GFP_DIRECT_RECLAIM could
> >fail.
> 
> I don't think there's a problem in the scenario? Kswapd will keep
> being woken up and reclaim from the node lru. It will hit and free
> any low zone pages that are on the lru, even though it doesn't
> "balance for low zone". Eventually it will either satisfy the
> constrained allocation by reclaiming those low-zone pages during the
> repeated wakeups, or the low-zone wakeups will stop coming together
> with higher-zone wakeups and then it will reclaim the low-zone pages
> in a single low-zone wakeup. If the zone-constrained request is not

Yes, probability of this would be low.

> allowed to fail, then it will just keep waking up kswapd and waiting
> for the progress. If it's allowed to fail (i.e. not __GFP_NOFAIL),
> but not allowed to direct reclaim, it goes "goto nopage" rather
> quickly in __alloc_pages_slowpath(), without any waiting for
> kswapd's progress, so there's not really much difference whether the
> kswapd wakeup picked up a low classzone or not. Note the

Hmm... Even if allocation could fail, we should do our best to prevent
failure. Relying on luck isn't good idea to me.

Thanks.

> __GFP_NOFAIL but ~__GFP_DIRECT_RECLAIM is a WARN_ON_ONCE() scenario,
> so definitely not common...
> 
> >Thanks.
> >
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

#1445251

On 07/18/2016 07:07 AM, Joonsoo Kim wrote:
> On Thu, Jul 14, 2016 at 10:32:09AM +0200, Vlastimil Babka wrote:
>> On 07/14/2016 07:23 AM, Joonsoo Kim wrote:
>>
>> I don't think there's a problem in the scenario? Kswapd will keep
>> being woken up and reclaim from the node lru. It will hit and free
>> any low zone pages that are on the lru, even though it doesn't
>> "balance for low zone". Eventually it will either satisfy the
>> constrained allocation by reclaiming those low-zone pages during the
>> repeated wakeups, or the low-zone wakeups will stop coming together
>> with higher-zone wakeups and then it will reclaim the low-zone pages
>> in a single low-zone wakeup. If the zone-constrained request is not
>
> Yes, probability of this would be low.
>
>> allowed to fail, then it will just keep waking up kswapd and waiting
>> for the progress. If it's allowed to fail (i.e. not __GFP_NOFAIL),
>> but not allowed to direct reclaim, it goes "goto nopage" rather
>> quickly in __alloc_pages_slowpath(), without any waiting for
>> kswapd's progress, so there's not really much difference whether the
>> kswapd wakeup picked up a low classzone or not. Note the
>
> Hmm... Even if allocation could fail, we should do our best to prevent
> failure. Relying on luck isn't good idea to me.

But "Doing our best" has to have some sane limits. Allocation, that 
cannot direct reclaim, already relies on luck. And we are not really 
changing this. The allocation will "goto nopage" before kswapd can even 
wake up and start doing something, regardless of classzone_idx used.

> Thanks.
>
>> __GFP_NOFAIL but ~__GFP_DIRECT_RECLAIM is a WARN_ON_ONCE() scenario,
>> so definitely not common...
>>
>>> Thanks.
>>>
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

#1445274

On Mon, Jul 18, 2016 at 08:51:16AM +0200, Vlastimil Babka wrote:
> On 07/18/2016 07:07 AM, Joonsoo Kim wrote:
> >On Thu, Jul 14, 2016 at 10:32:09AM +0200, Vlastimil Babka wrote:
> >>On 07/14/2016 07:23 AM, Joonsoo Kim wrote:
> >>
> >>I don't think there's a problem in the scenario? Kswapd will keep
> >>being woken up and reclaim from the node lru. It will hit and free
> >>any low zone pages that are on the lru, even though it doesn't
> >>"balance for low zone". Eventually it will either satisfy the
> >>constrained allocation by reclaiming those low-zone pages during the
> >>repeated wakeups, or the low-zone wakeups will stop coming together
> >>with higher-zone wakeups and then it will reclaim the low-zone pages
> >>in a single low-zone wakeup. If the zone-constrained request is not
> >
> >Yes, probability of this would be low.
> >
> >>allowed to fail, then it will just keep waking up kswapd and waiting
> >>for the progress. If it's allowed to fail (i.e. not __GFP_NOFAIL),
> >>but not allowed to direct reclaim, it goes "goto nopage" rather
> >>quickly in __alloc_pages_slowpath(), without any waiting for
> >>kswapd's progress, so there's not really much difference whether the
> >>kswapd wakeup picked up a low classzone or not. Note the
> >
> >Hmm... Even if allocation could fail, we should do our best to prevent
> >failure. Relying on luck isn't good idea to me.
> 
> But "Doing our best" has to have some sane limits. Allocation, that

Ensuring to do something for the requested zone at least once isn't insane.

> cannot direct reclaim, already relies on luck. And we are not really
> changing this. The allocation will "goto nopage" before kswapd can
> even wake up and start doing something, regardless of classzone_idx
> used.

But, this patch makes things worse. Even if next allocation comes
after kswapd is waking up and doing something, low zone would not be
balanced due to max classzone_idx and allocation could fail. It is
what this patch changes and I worry.

Thanks.

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

#1443303

On Thu, Jul 14, 2016 at 02:23:32PM +0900, Joonsoo Kim wrote:
> > 
> > > > > And, I'd like to know why max() is used for classzone_idx rather than
> > > > > min()? I think that kswapd should balance the lowest zone requested.
> > > > > 
> > > > 
> > > > If there are two allocation requests -- one zone-constraned and the other
> > > > zone-unconstrained, it does not make sense to have kswapd skip the pages
> > > > usable for the zone-unconstrained and waste a load of CPU. You could
> > > 
> > > I agree that, in this case, it's not good to skip the pages usable
> > > for the zone-unconstrained request. But, what I am concerned is that
> > > kswapd stop reclaim prematurely in the view of zone-constrained
> > > requestor.
> > 
> > It doesn't stop reclaiming for the lower zones. It's reclaiming the LRU
> > for the whole node that may or may not have lower zone pages at the end
> > of the LRU. If it does, then the allocation request will be satisfied.
> > If it does not, then kswapd will think the node is balanced and get
> > rewoken to do a zone-constrained reclaim pass.
> 
> If zone-constrained request could go direct reclaim pass, there would
> be no problem. But, please assume that request is zone-constrained
> without __GFP_DIRECT_RECLAIM which is common for some device driver
> implementation.

Then it's likely GFP_ATOMIC and it'll wake kswapd on each failure. If
kswapd is containtly awake for highmem requests then we're reclaiming
everything anyway.  Remember that if kswapd is reclaiming for higher zones,
it'll still cover the lower zones eventually. There is no guarantee that
skipping the highmem pages will satisfy the atomic allocations any faster
but consuming the CPU to skip the pages is a definite cost.

Even worse, skipping highmem pages when a highmem pages are required may
ake lowmem pressure worse because those pages are freed faster and can
be consumed by zone-unconstrained requests.

If this really is a problem in practice then we can consider having
allocation requests that are zone-constrained and !__GFP_DIRECT_RECLAIM
set a flag and use the min classzone for the wakeup. That flag remains
set until kswapd takes at least one pass using the lower classzone and
clears it. The classzone will not be adjusted higher until that flag is
cleared. I don't think we should do it without evidence that it's a real
problem because kswapd potentially uses useless CPU and the potential for
higher lowmem pressure.

-- 
Mel Gorman
SUSE Labs

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

#1445205

On Thu, Jul 14, 2016 at 10:05:00AM +0100, Mel Gorman wrote:
> On Thu, Jul 14, 2016 at 02:23:32PM +0900, Joonsoo Kim wrote:
> > > 
> > > > > > And, I'd like to know why max() is used for classzone_idx rather than
> > > > > > min()? I think that kswapd should balance the lowest zone requested.
> > > > > > 
> > > > > 
> > > > > If there are two allocation requests -- one zone-constraned and the other
> > > > > zone-unconstrained, it does not make sense to have kswapd skip the pages
> > > > > usable for the zone-unconstrained and waste a load of CPU. You could
> > > > 
> > > > I agree that, in this case, it's not good to skip the pages usable
> > > > for the zone-unconstrained request. But, what I am concerned is that
> > > > kswapd stop reclaim prematurely in the view of zone-constrained
> > > > requestor.
> > > 
> > > It doesn't stop reclaiming for the lower zones. It's reclaiming the LRU
> > > for the whole node that may or may not have lower zone pages at the end
> > > of the LRU. If it does, then the allocation request will be satisfied.
> > > If it does not, then kswapd will think the node is balanced and get
> > > rewoken to do a zone-constrained reclaim pass.
> > 
> > If zone-constrained request could go direct reclaim pass, there would
> > be no problem. But, please assume that request is zone-constrained
> > without __GFP_DIRECT_RECLAIM which is common for some device driver
> > implementation.
> 
> Then it's likely GFP_ATOMIC and it'll wake kswapd on each failure. If
> kswapd is containtly awake for highmem requests then we're reclaiming
> everything anyway.  Remember that if kswapd is reclaiming for higher zones,
> it'll still cover the lower zones eventually. There is no guarantee that
> skipping the highmem pages will satisfy the atomic allocations any faster
> but consuming the CPU to skip the pages is a definite cost.

Okay.

> 
> Even worse, skipping highmem pages when a highmem pages are required may
> ake lowmem pressure worse because those pages are freed faster and can
> be consumed by zone-unconstrained requests.

Okay.

> 
> If this really is a problem in practice then we can consider having
> allocation requests that are zone-constrained and !__GFP_DIRECT_RECLAIM
> set a flag and use the min classzone for the wakeup. That flag remains
> set until kswapd takes at least one pass using the lower classzone and
> clears it. The classzone will not be adjusted higher until that flag is

It would work.

> cleared. I don't think we should do it without evidence that it's a real
> problem because kswapd potentially uses useless CPU and the potential for
> higher lowmem pressure.

Hmmm... I think differently. Your patch changes current behaviour
without any evidence. Code simplification cannot compensate
potential stability issue. Before your patch, kswapd try to
balance for minimum classzone so until dis-advantage of this approach
is proved, it's better to keep original logic.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web