On Mon 06-03-17 11:24:10, Johannes Weiner wrote:
[...]
> >From e126db716926ff353b35f3a6205bd5853e01877b Mon Sep 17 00:00:00 2001
> From: Johannes Weiner <han...@cmpxchg.org>
> Date: Mon, 6 Mar 2017 10:53:59 -0500
> Subject: [PATCH] mm: fix 100% CPU kswapd busyloop on unreclaimable nodes fix
> 
> Check kswapd failure against the cumulative nr_reclaimed count, not
> against the count from the lowest priority iteration.
> 
> Suggested-by: Minchan Kim <minc...@kernel.org>
> Signed-off-by: Johannes Weiner <han...@cmpxchg.org>
> ---
>  mm/vmscan.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ddcff8a11c1e..b834b2dd4e19 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3179,9 +3179,9 @@ static int balance_pgdat(pg_data_t *pgdat, int order, 
> int classzone_idx)
>       count_vm_event(PAGEOUTRUN);
>  
>       do {
> +             unsigned long nr_reclaimed = sc.nr_reclaimed;
>               bool raise_priority = true;
>  
> -             sc.nr_reclaimed = 0;
>               sc.reclaim_idx = classzone_idx;
>  
>               /*
> @@ -3271,7 +3271,8 @@ static int balance_pgdat(pg_data_t *pgdat, int order, 
> int classzone_idx)
>                * Raise priority if scanning rate is too low or there was no
>                * progress in reclaiming pages
>                */
> -             if (raise_priority || !sc.nr_reclaimed)
> +             nr_reclaimed = sc.nr_reclaimed - nr_reclaimed;
> +             if (raise_priority || !nr_reclaimed)
>                       sc.priority--;
>       } while (sc.priority >= 1);
>  

I would rather not play with the sc state here. From a quick look at
least 
        /*
         * Fragmentation may mean that the system cannot be rebalanced for
         * high-order allocations. If twice the allocation size has been
         * reclaimed then recheck watermarks only at order-0 to prevent
         * excessive reclaim. Assume that a process requested a high-order
         * can direct reclaim/compact.
         */
        if (sc->order && sc->nr_reclaimed >= compact_gap(sc->order))
                sc->order = 0;

does rely on the value. Wouldn't something like the following be safer?
---
diff --git a/mm/vmscan.c b/mm/vmscan.c
index c15b2e4c47ca..b731f24fed12 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3183,6 +3183,7 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int 
classzone_idx)
                .may_unmap = 1,
                .may_swap = 1,
        };
+       bool reclaimable = false;
        count_vm_event(PAGEOUTRUN);
 
        do {
@@ -3274,6 +3275,9 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int 
classzone_idx)
                if (try_to_freeze() || kthread_should_stop())
                        break;
 
+               if (sc.nr_reclaimed)
+                       reclaimable = true;
+
                /*
                 * Raise priority if scanning rate is too low or there was no
                 * progress in reclaiming pages
@@ -3282,7 +3286,7 @@ static int balance_pgdat(pg_data_t *pgdat, int order, int 
classzone_idx)
                        sc.priority--;
        } while (sc.priority >= 1);
 
-       if (!sc.nr_reclaimed)
+       if (!reclaimable)
                pgdat->kswapd_failures++;
 
 out:
-- 
Michal Hocko
SUSE Labs

Reply via email to