From: Shakeel Butt <shakeelb@google.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Andrey Ryabinin <aryabinin@virtuozzo.com>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>, Linux MM <linux-mm@kvack.org>,
Cgroups <cgroups@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH 08/11] mm: vmscan: harmonize writeback congestion tracking for nodes & memcgs
Date: Wed, 6 Nov 2019 18:52:06 -0800 [thread overview]
Message-ID: <CALvZod6XVRbvaRk2_HoB-EQLCGm9e9FbPAhrpPOLiLU37g-xag@mail.gmail.com> (raw)
In-Reply-To: <20190603210746.15800-9-hannes@cmpxchg.org>
On Mon, Jun 3, 2019 at 3:04 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> The current writeback congestion tracking has separate flags for
> kswapd reclaim (node level) and cgroup limit reclaim (memcg-node
> level). This is unnecessarily complicated: the lruvec is an existing
> abstraction layer for that node-memcg intersection.
>
> Introduce lruvec->flags and LRUVEC_CONGESTED. Then track that at the
> reclaim root level, which is either the NUMA node for global reclaim,
> or the cgroup-node intersection for cgroup reclaim.
>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reviewed-by: Shakeel Butt <shakeelb@google.com>
> ---
> include/linux/memcontrol.h | 6 +--
> include/linux/mmzone.h | 11 ++++--
> mm/vmscan.c | 80 ++++++++++++--------------------------
> 3 files changed, 36 insertions(+), 61 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index fc32cfaebf32..d33e09c51acc 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -144,9 +144,6 @@ struct mem_cgroup_per_node {
> unsigned long usage_in_excess;/* Set to the value by which */
> /* the soft limit is exceeded*/
> bool on_tree;
> - bool congested; /* memcg has many dirty pages */
> - /* backed by a congested BDI */
> -
> struct mem_cgroup *memcg; /* Back pointer, we cannot */
> /* use container_of */
> };
> @@ -401,6 +398,9 @@ static inline struct lruvec *mem_cgroup_lruvec(struct mem_cgroup *memcg,
> goto out;
> }
>
> + if (!memcg)
> + memcg = root_mem_cgroup;
> +
> mz = mem_cgroup_nodeinfo(memcg, pgdat->node_id);
> lruvec = &mz->lruvec;
> out:
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 95d63a395f40..b3ab64cf5619 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -293,6 +293,12 @@ struct zone_reclaim_stat {
> unsigned long recent_scanned[2];
> };
>
> +enum lruvec_flags {
> + LRUVEC_CONGESTED, /* lruvec has many dirty pages
> + * backed by a congested BDI
> + */
> +};
> +
> struct lruvec {
> struct list_head lists[NR_LRU_LISTS];
> struct zone_reclaim_stat reclaim_stat;
> @@ -300,6 +306,8 @@ struct lruvec {
> atomic_long_t inactive_age;
> /* Refaults at the time of last reclaim cycle */
> unsigned long refaults;
> + /* Various lruvec state flags (enum lruvec_flags) */
> + unsigned long flags;
> #ifdef CONFIG_MEMCG
> struct pglist_data *pgdat;
> #endif
> @@ -562,9 +570,6 @@ struct zone {
> } ____cacheline_internodealigned_in_smp;
>
> enum pgdat_flags {
> - PGDAT_CONGESTED, /* pgdat has many dirty pages backed by
> - * a congested BDI
> - */
> PGDAT_DIRTY, /* reclaim scanning has recently found
> * many dirty file pages at the tail
> * of the LRU.
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ee79b39d0538..eb535c572733 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -267,29 +267,6 @@ static bool writeback_working(struct scan_control *sc)
> #endif
> return false;
> }
> -
> -static void set_memcg_congestion(pg_data_t *pgdat,
> - struct mem_cgroup *memcg,
> - bool congested)
> -{
> - struct mem_cgroup_per_node *mn;
> -
> - if (!memcg)
> - return;
> -
> - mn = mem_cgroup_nodeinfo(memcg, pgdat->node_id);
> - WRITE_ONCE(mn->congested, congested);
> -}
> -
> -static bool memcg_congested(pg_data_t *pgdat,
> - struct mem_cgroup *memcg)
> -{
> - struct mem_cgroup_per_node *mn;
> -
> - mn = mem_cgroup_nodeinfo(memcg, pgdat->node_id);
> - return READ_ONCE(mn->congested);
> -
> -}
> #else
> static bool cgroup_reclaim(struct scan_control *sc)
> {
> @@ -300,18 +277,6 @@ static bool writeback_working(struct scan_control *sc)
> {
> return true;
> }
> -
> -static inline void set_memcg_congestion(struct pglist_data *pgdat,
> - struct mem_cgroup *memcg, bool congested)
> -{
> -}
> -
> -static inline bool memcg_congested(struct pglist_data *pgdat,
> - struct mem_cgroup *memcg)
> -{
> - return false;
> -
> -}
> #endif
>
> /*
> @@ -2659,12 +2624,6 @@ static inline bool should_continue_reclaim(struct pglist_data *pgdat,
> return true;
> }
>
> -static bool pgdat_memcg_congested(pg_data_t *pgdat, struct mem_cgroup *memcg)
> -{
> - return test_bit(PGDAT_CONGESTED, &pgdat->flags) ||
> - (memcg && memcg_congested(pgdat, memcg));
> -}
> -
> static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
> {
> struct mem_cgroup *root = sc->target_mem_cgroup;
> @@ -2748,8 +2707,11 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
> struct reclaim_state *reclaim_state = current->reclaim_state;
> struct mem_cgroup *root = sc->target_mem_cgroup;
> unsigned long nr_reclaimed, nr_scanned;
> + struct lruvec *target_lruvec;
> bool reclaimable = false;
>
> + target_lruvec = mem_cgroup_lruvec(sc->target_mem_cgroup, pgdat);
> +
> again:
> memset(&sc->nr, 0, sizeof(sc->nr));
>
> @@ -2792,14 +2754,6 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
> if (sc->nr.writeback && sc->nr.writeback == sc->nr.taken)
> set_bit(PGDAT_WRITEBACK, &pgdat->flags);
>
> - /*
> - * Tag a node as congested if all the dirty pages
> - * scanned were backed by a congested BDI and
> - * wait_iff_congested will stall.
> - */
> - if (sc->nr.dirty && sc->nr.dirty == sc->nr.congested)
> - set_bit(PGDAT_CONGESTED, &pgdat->flags);
> -
> /* Allow kswapd to start writing pages during reclaim.*/
> if (sc->nr.unqueued_dirty == sc->nr.file_taken)
> set_bit(PGDAT_DIRTY, &pgdat->flags);
> @@ -2815,12 +2769,17 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
> }
>
> /*
> + * Tag a node/memcg as congested if all the dirty pages
> + * scanned were backed by a congested BDI and
> + * wait_iff_congested will stall.
> + *
> * Legacy memcg will stall in page writeback so avoid forcibly
> * stalling in wait_iff_congested().
> */
> - if (cgroup_reclaim(sc) && writeback_working(sc) &&
> + if ((current_is_kswapd() ||
> + (cgroup_reclaim(sc) && writeback_working(sc))) &&
> sc->nr.dirty && sc->nr.dirty == sc->nr.congested)
> - set_memcg_congestion(pgdat, root, true);
> + set_bit(LRUVEC_CONGESTED, &target_lruvec->flags);
>
> /*
> * Stall direct reclaim for IO completions if underlying BDIs
> @@ -2828,8 +2787,9 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc)
> * starts encountering unqueued dirty pages or cycling through
> * the LRU too quickly.
> */
> - if (!sc->hibernation_mode && !current_is_kswapd() &&
> - current_may_throttle() && pgdat_memcg_congested(pgdat, root))
> + if (!current_is_kswapd() && current_may_throttle() &&
> + !sc->hibernation_mode &&
> + test_bit(LRUVEC_CONGESTED, &target_lruvec->flags))
> wait_iff_congested(BLK_RW_ASYNC, HZ/10);
>
> if (should_continue_reclaim(pgdat, sc->nr_reclaimed - nr_reclaimed,
> @@ -3043,8 +3003,16 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
> if (zone->zone_pgdat == last_pgdat)
> continue;
> last_pgdat = zone->zone_pgdat;
> +
> snapshot_refaults(sc->target_mem_cgroup, zone->zone_pgdat);
> - set_memcg_congestion(last_pgdat, sc->target_mem_cgroup, false);
> +
> + if (cgroup_reclaim(sc)) {
> + struct lruvec *lruvec;
> +
> + lruvec = mem_cgroup_lruvec(sc->target_mem_cgroup,
> + zone->zone_pgdat);
> + clear_bit(LRUVEC_CONGESTED, &lruvec->flags);
> + }
> }
>
> delayacct_freepages_end();
> @@ -3419,7 +3387,9 @@ static bool pgdat_balanced(pg_data_t *pgdat, int order, int classzone_idx)
> /* Clear pgdat state for congested, dirty or under writeback. */
> static void clear_pgdat_congested(pg_data_t *pgdat)
> {
> - clear_bit(PGDAT_CONGESTED, &pgdat->flags);
> + struct lruvec *lruvec = mem_cgroup_lruvec(NULL, pgdat);
> +
> + clear_bit(LRUVEC_CONGESTED, &lruvec->flags);
> clear_bit(PGDAT_DIRTY, &pgdat->flags);
> clear_bit(PGDAT_WRITEBACK, &pgdat->flags);
> }
> --
> 2.21.0
>
next prev parent reply other threads:[~2019-11-07 2:52 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-03 21:07 [PATCH 00/11] mm: fix page aging across multiple cgroups Johannes Weiner
2019-06-03 21:07 ` [PATCH 01/11] mm: vmscan: move inactive_list_is_low() swap check to the caller Johannes Weiner
2019-11-07 2:50 ` Shakeel Butt
2019-11-08 3:43 ` Andrew Morton
2019-06-03 21:07 ` [PATCH 02/11] mm: clean up and clarify lruvec lookup procedure Johannes Weiner
2019-11-07 2:50 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 03/11] mm: vmscan: simplify lruvec_lru_size() Johannes Weiner
2019-11-07 2:51 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 04/11] mm: vmscan: naming fixes: cgroup_reclaim() and writeback_working() Johannes Weiner
2019-11-07 2:51 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 05/11] mm: vmscan: replace shrink_node() loop with a retry jump Johannes Weiner
2019-11-07 2:51 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 06/11] mm: vmscan: turn shrink_node_memcg() into shrink_lruvec() Johannes Weiner
2019-11-07 2:51 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 07/11] mm: vmscan: split shrink_node() into node part and memcgs part Johannes Weiner
2019-11-07 2:51 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 08/11] mm: vmscan: harmonize writeback congestion tracking for nodes & memcgs Johannes Weiner
2019-11-07 2:52 ` Shakeel Butt [this message]
2019-06-03 21:07 ` [PATCH 09/11] mm: vmscan: move file exhaustion detection to the node level Johannes Weiner
2019-11-07 2:52 ` Shakeel Butt
2019-06-03 21:07 ` [PATCH 10/11] mm: vmscan: detect file thrashing at the reclaim root Johannes Weiner
2019-06-03 21:07 ` [PATCH 11/11] mm: vmscan: enforce inactive:active ratio " Johannes Weiner
2019-11-07 2:50 ` [PATCH 00/11] mm: fix page aging across multiple cgroups Shakeel Butt
2019-11-07 17:45 ` Johannes Weiner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALvZod6XVRbvaRk2_HoB-EQLCGm9e9FbPAhrpPOLiLU37g-xag@mail.gmail.com \
--to=shakeelb@google.com \
--cc=akpm@linux-foundation.org \
--cc=aryabinin@virtuozzo.com \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=surenb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox