linux-kernel - Re: [PATCH v2 0/3] Ignore non-LRU-based reclaim in memcg reclaim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJD7tkY-S-v117nYuQN7=3cte+kv0QgMa2B-NgOf6bH8bm1XbQ@mail.gmail.com>
Date:   Wed, 22 Mar 2023 21:05:45 -0700
From:   Yosry Ahmed <yosryahmed@...gle.com>
To:     Alexander Viro <viro@...iv.linux.org.uk>,
        "Darrick J. Wong" <djwong@...nel.org>,
        Christoph Lameter <cl@...ux.com>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Roman Gushchin <roman.gushchin@...ux.dev>,
        Hyeonggon Yoo <42.hyeyoo@...il.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Miaohe Lin <linmiaohe@...wei.com>,
        David Hildenbrand <david@...hat.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Peter Xu <peterx@...hat.com>, NeilBrown <neilb@...e.de>,
        Shakeel Butt <shakeelb@...gle.com>,
        Michal Hocko <mhocko@...nel.org>, Yu Zhao <yuzhao@...gle.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v2 0/3] Ignore non-LRU-based reclaim in memcg reclaim

Any thoughts on this respin?

On Thu, Mar 9, 2023 at 1:31 AM Yosry Ahmed <yosryahmed@...gle.com> wrote:
>
> Upon running some proactive reclaim tests using memory.reclaim, we
> noticed some tests flaking where writing to memory.reclaim would be
> successful even though we did not reclaim the requested amount fully.
> Looking further into it, I discovered that *sometimes* we over-report
> the number of reclaimed pages in memcg reclaim.
>
> Reclaimed pages through other means than LRU-based reclaim are tracked
> through reclaim_state in struct scan_control, which is stashed in
> current task_struct. These pages are added to the number of reclaimed
> pages through LRUs. For memcg reclaim, these pages generally cannot be
> linked to the memcg under reclaim and can cause an overestimated count
> of reclaimed pages. This short series tries to address that.
>
> Patches 1-2 are just refactoring, they add helpers that wrap some
> operations on current->reclaim_state, and rename
> reclaim_state->reclaimed_slab to reclaim_state->reclaimed.
>
> Patch 3 ignores pages reclaimed outside of LRU reclaim in memcg reclaim.
> The pages are uncharged anyway, so even if we end up under-reporting
> reclaimed pages we will still succeed in making progress during
> charging.
>
> Do not let the diff stat deceive you, the core of this series is patch 3,
> which has one line of code change. All the rest is refactoring and one
> huge comment.
>
> v1 -> v2:
> - Renamed report_freed_pages() to mm_account_reclaimed_pages(), as
>   suggested by Dave Chinner. There were discussions about leaving
>   updating current->reclaim_state open-coded as it's not worth hiding
>   the current dereferencing to remove one line, but I'd rather have the
>   logic contained with mm/vmscan.c so that the next person that changes
>   this logic doesn't have to change 7 different files.
> - Renamed add_non_vmscan_reclaimed() to flush_reclaim_state() (Johannes
>   Weiner).
> - Added more context about how this problem was found in the cover
>   letter (Johannes Weiner).
> - Added a patch to move set_task_reclaim_state() below the definition of
>   cgroup_reclaim(), and added additional helpers in the same position.
>   This way all the helpers for reclaim_state live together, and there is
>   no need to declare cgroup_reclaim() early or move its definition
>   around to call it from flush_reclaim_state(). This should also fix the
>   build error reported by the bot in !CONFIG_MEMCG.
>
> RFC -> v1:
> - Exported report_freed_pages() in case XFS is built as a module (Matthew
>   Wilcox).
> - Renamed reclaimed_slab to reclaim in previously missed MGLRU code.
> - Refactored using reclaim_state to update sc->nr_reclaimed into a
>   helper and added an XL comment explaining why we ignore
>   reclaim_state->reclaimed in memcg reclaim (Johannes Weiner).
>
> Yosry Ahmed (3):
>   mm: vmscan: move set_task_reclaim_state() after cgroup_reclaim()
>   mm: vmscan: refactor updating reclaimed pages in reclaim_state
>   mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim
>
>  fs/inode.c           |  3 +-
>  fs/xfs/xfs_buf.c     |  3 +-
>  include/linux/swap.h |  5 ++-
>  mm/slab.c            |  3 +-
>  mm/slob.c            |  6 +--
>  mm/slub.c            |  5 +--
>  mm/vmscan.c          | 88 +++++++++++++++++++++++++++++++++++---------
>  7 files changed, 81 insertions(+), 32 deletions(-)
>
> --
> 2.40.0.rc0.216.gc4246ad0f0-goog
>