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

Groups > linux.kernel > #1577519 > unrolled thread

[PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on underflow

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on underflow Vinayak Menon <vinmenon@codeaurora.org> - 2017-02-09 13:10 +0100
    Re: [PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on  underflow Michal Hocko <mhocko@kernel.org> - 2017-02-09 13:30 +0100
      Re: [PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on  underflow Michal Hocko <mhocko@kernel.org> - 2017-02-09 13:30 +0100

#1577519 — [PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on underflow

At the end of a window period, if the reclaimed pages
is greater than scanned, an unsigned underflow can
result in a huge pressure value and thus a critical event.
Reclaimed pages is found to go higher than scanned because
of the addition of reclaimed slab pages to reclaimed in
shrink_node without a corresponding increment to scanned
pages. Minchan Kim mentioned that this can also happen in
the case of a THP page where the scanned is 1 and reclaimed
could be 512.

Acked-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
---
v2: Adding a comment and reordering the patches
    as per Michal's suggestion

 mm/vmpressure.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index 149fdf6..6063581 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -112,9 +112,16 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 						    unsigned long reclaimed)
 {
 	unsigned long scale = scanned + reclaimed;
-	unsigned long pressure;
+	unsigned long pressure = 0;
 
 	/*
+	 * reclaimed can be greater than scanned in cases
+	 * like THP, where the scanned is 1 and reclaimed
+	 * could be 512
+	 */
+	if (reclaimed >= scanned)
+		goto out;
+	/*
 	 * We calculate the ratio (in percents) of how many pages were
 	 * scanned vs. reclaimed in a given time frame (window). Note that
 	 * time is in VM reclaimer's "ticks", i.e. number of pages
@@ -124,6 +131,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 	pressure = scale - (reclaimed * scale / scanned);
 	pressure = pressure * 100 / scale;
 
+out:
 	pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
 		 scanned, reclaimed);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

[toc] | [next] | [standalone]

#1577531 — Re: [PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on underflow

On Thu 09-02-17 17:29:36, Vinayak Menon wrote:
> At the end of a window period, if the reclaimed pages
> is greater than scanned, an unsigned underflow can
> result in a huge pressure value and thus a critical event.
> Reclaimed pages is found to go higher than scanned because
> of the addition of reclaimed slab pages to reclaimed in
> shrink_node without a corresponding increment to scanned
> pages. Minchan Kim mentioned that this can also happen in
> the case of a THP page where the scanned is 1 and reclaimed
> could be 512.
> 
> Acked-by: Minchan Kim <minchan@kernel.org>
> Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>

Acked-by: Michal Hocko <mhocko@suse.com>

I would prefer the fixup in vmpressure() as already mentioned but this
should work as well.

> ---
> v2: Adding a comment and reordering the patches
>     as per Michal's suggestion
> 
>  mm/vmpressure.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> index 149fdf6..6063581 100644
> --- a/mm/vmpressure.c
> +++ b/mm/vmpressure.c
> @@ -112,9 +112,16 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
>  						    unsigned long reclaimed)
>  {
>  	unsigned long scale = scanned + reclaimed;
> -	unsigned long pressure;
> +	unsigned long pressure = 0;
>  
>  	/*
> +	 * reclaimed can be greater than scanned in cases
> +	 * like THP, where the scanned is 1 and reclaimed
> +	 * could be 512
> +	 */
> +	if (reclaimed >= scanned)
> +		goto out;
> +	/*
>  	 * We calculate the ratio (in percents) of how many pages were
>  	 * scanned vs. reclaimed in a given time frame (window). Note that
>  	 * time is in VM reclaimer's "ticks", i.e. number of pages
> @@ -124,6 +131,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
>  	pressure = scale - (reclaimed * scale / scanned);
>  	pressure = pressure * 100 / scale;
>  
> +out:
>  	pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
>  		 scanned, reclaimed);
>  
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
> 

-- 
Michal Hocko
SUSE Labs

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

#1577533 — Re: [PATCH 1/2 v2] mm: vmpressure: fix sending wrong events on underflow

On Thu 09-02-17 13:10:57, Michal Hocko wrote:
> On Thu 09-02-17 17:29:36, Vinayak Menon wrote:
> > At the end of a window period, if the reclaimed pages
> > is greater than scanned, an unsigned underflow can
> > result in a huge pressure value and thus a critical event.
> > Reclaimed pages is found to go higher than scanned because
> > of the addition of reclaimed slab pages to reclaimed in
> > shrink_node without a corresponding increment to scanned
> > pages. Minchan Kim mentioned that this can also happen in
> > the case of a THP page where the scanned is 1 and reclaimed
> > could be 512.
> > 
> > Acked-by: Minchan Kim <minchan@kernel.org>
> > Signed-off-by: Vinayak Menon <vinmenon@codeaurora.org>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> I would prefer the fixup in vmpressure() as already mentioned but this
> should work as well.

Btw. I guess this should be good to mark for stable. Reclaiming THP is
not all that rare (even though we try to avoid anon reclaim as much as
possible) and hitting critical events can lead to disruptive actions to
early.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web