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

Groups > linux.kernel > #1399924 > unrolled thread

[PATCH 0/2] oom detection fixups

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] oom detection fixups Michal Hocko <mhocko@kernel.org> - 2016-05-12 13:20 +0200
    [PATCH 2/2] mm, oom: protect !costly allocations some more for !CONFIG_COMPACTION Michal Hocko <mhocko@kernel.org> - 2016-05-12 13:20 +0200
      Re: [PATCH 2/2] mm, oom: protect !costly allocations some more for !CONFIG_COMPACTION "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-05-13 06:20 +0200
    [PATCH 1/2] mmotm: mm-oom-rework-oom-detection-fix Michal Hocko <mhocko@kernel.org> - 2016-05-12 13:20 +0200
      Re: [PATCH 1/2] mmotm: mm-oom-rework-oom-detection-fix "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-05-13 06:10 +0200

#1399924 — [PATCH 0/2] oom detection fixups

Hi Andrew,
these two patches are follow-up fixups for the oom detection rework.
The first patch should be folded into mm-oom-rework-oom-detection.patch.
I haven't noticed classzone_idx vs high_zoneidx only when working on
the second patch which is a fix for !CONFIG_COMPACTION where Joonsoo
reported a premature OOM killer. I didn't pay attention to this
configuration before but I have tested a heavy fork load on a small
machine and the patch helps as it restores the original behavior more or
less for !costly high order requests.

[toc] | [next] | [standalone]

#1399928 — [PATCH 2/2] mm, oom: protect !costly allocations some more for !CONFIG_COMPACTION

From: Michal Hocko <mhocko@suse.com>

Joonsoo has reported that he is able to trigger OOM for !costly high
order requests (heavy fork() workload close the OOM) with the new
oom detection rework. This is because we rely only on should_reclaim_retry
when the compaction is disabled and it only checks watermarks for the
requested order and so we might trigger OOM when there is a lot of free
memory.

It is not very clear what are the usual workloads when the compaction
is disabled. Relying on high order allocations heavily without any
mechanism to create those orders except for unbound amount of reclaim is
certainly not a good idea.

To prevent from potential regressions let's help this configuration
some. We have to sacrifice the determinsm though because there simply is
none here possible. should_compact_retry implementation for
!CONFIG_COMPACTION, which was empty so far, will do watermark check
for order-0 on all eligible zones. This will cause retrying until either
the reclaim cannot make any further progress or all the zones are
depleted even for order-0 pages. This means that the number of retries
is basically unbounded for !costly orders but that was the case before
the rework as well so this shouldn't regress.

Reported-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/page_alloc.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 620ec002aea2..7e2defbfe55b 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3310,6 +3310,24 @@ should_compact_retry(struct alloc_context *ac, unsigned int order, int alloc_fla
 		     enum migrate_mode *migrate_mode,
 		     int compaction_retries)
 {
+	struct zone *zone;
+	struct zoneref *z;
+
+	if (!order || order > PAGE_ALLOC_COSTLY_ORDER)
+		return false;
+
+	/*
+	 * There are setups with compaction disabled which would prefer to loop
+	 * inside the allocator rather than hit the oom killer prematurely. Let's
+	 * give them a good hope and keep retrying while the order-0 watermarks
+	 * are OK.
+	 */
+	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->high_zoneidx,
+					ac->nodemask) {
+		if(zone_watermark_ok(zone, 0, min_wmark_pages(zone),
+					ac_classzone_idx(ac), alloc_flags))
+			return true;
+	}
 	return false;
 }
 #endif /* CONFIG_COMPACTION */
-- 
2.8.1

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

#1400494 — Re: [PATCH 2/2] mm, oom: protect !costly allocations some more for !CONFIG_COMPACTION

> From: Michal Hocko <mhocko@suse.com>
> 
> Joonsoo has reported that he is able to trigger OOM for !costly high
> order requests (heavy fork() workload close the OOM) with the new
> oom detection rework. This is because we rely only on should_reclaim_retry
> when the compaction is disabled and it only checks watermarks for the
> requested order and so we might trigger OOM when there is a lot of free
> memory.
> 
> It is not very clear what are the usual workloads when the compaction
> is disabled. Relying on high order allocations heavily without any
> mechanism to create those orders except for unbound amount of reclaim is
> certainly not a good idea.
> 
> To prevent from potential regressions let's help this configuration
> some. We have to sacrifice the determinsm though because there simply is
> none here possible. should_compact_retry implementation for
> !CONFIG_COMPACTION, which was empty so far, will do watermark check
> for order-0 on all eligible zones. This will cause retrying until either
> the reclaim cannot make any further progress or all the zones are
> depleted even for order-0 pages. This means that the number of retries
> is basically unbounded for !costly orders but that was the case before
> the rework as well so this shouldn't regress.
> 
> Reported-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>

>  mm/page_alloc.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 620ec002aea2..7e2defbfe55b 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3310,6 +3310,24 @@ should_compact_retry(struct alloc_context *ac, unsigned int order, int alloc_fla
>  		     enum migrate_mode *migrate_mode,
>  		     int compaction_retries)
>  {
> +	struct zone *zone;
> +	struct zoneref *z;
> +
> +	if (!order || order > PAGE_ALLOC_COSTLY_ORDER)
> +		return false;
> +
> +	/*
> +	 * There are setups with compaction disabled which would prefer to loop
> +	 * inside the allocator rather than hit the oom killer prematurely. Let's
> +	 * give them a good hope and keep retrying while the order-0 watermarks
> +	 * are OK.
> +	 */
> +	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->high_zoneidx,
> +					ac->nodemask) {
> +		if(zone_watermark_ok(zone, 0, min_wmark_pages(zone),

s/if(zone_/if (zone_/
> +					ac_classzone_idx(ac), alloc_flags))
> +			return true;
> +	}
>  	return false;
>  }
>  #endif /* CONFIG_COMPACTION */
> --
> 2.8.1

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

#1399932 — [PATCH 1/2] mmotm: mm-oom-rework-oom-detection-fix

From: Michal Hocko <mhocko@suse.com>

watermark check should use classzone_idx rather than high_zoneidx
to check reserves against the correct (preferred) zone.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0d9008042efa..620ec002aea2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3496,7 +3496,7 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
 		 * available?
 		 */
 		if (__zone_watermark_ok(zone, order, min_wmark_pages(zone),
-				ac->high_zoneidx, alloc_flags, available)) {
+				ac_classzone_idx(ac), alloc_flags, available)) {
 			/*
 			 * If we didn't make any progress and have a lot of
 			 * dirty + writeback pages then we should wait for
-- 
2.8.1

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

#1400493 — Re: [PATCH 1/2] mmotm: mm-oom-rework-oom-detection-fix

> From: Michal Hocko <mhocko@suse.com>
> 
> watermark check should use classzone_idx rather than high_zoneidx
> to check reserves against the correct (preferred) zone.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>

>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0d9008042efa..620ec002aea2 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3496,7 +3496,7 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
>  		 * available?
>  		 */
>  		if (__zone_watermark_ok(zone, order, min_wmark_pages(zone),
> -				ac->high_zoneidx, alloc_flags, available)) {
> +				ac_classzone_idx(ac), alloc_flags, available)) {
>  			/*
>  			 * If we didn't make any progress and have a lot of
>  			 * dirty + writeback pages then we should wait for
> --
> 2.8.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web