From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6204BC83002 for ; Tue, 28 Apr 2020 01:46:05 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1EDD8206D6 for ; Tue, 28 Apr 2020 01:46:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="XdZTujGX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1EDD8206D6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B17A08E0005; Mon, 27 Apr 2020 21:46:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AC8348E0001; Mon, 27 Apr 2020 21:46:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9B6808E0005; Mon, 27 Apr 2020 21:46:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0120.hostedemail.com [216.40.44.120]) by kanga.kvack.org (Postfix) with ESMTP id 818208E0001 for ; Mon, 27 Apr 2020 21:46:04 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 3C8B85DC8 for ; Tue, 28 Apr 2020 01:46:04 +0000 (UTC) X-FDA: 76755573048.24.oven66_34f3668156762 X-HE-Tag: oven66_34f3668156762 X-Filterd-Recvd-Size: 5739 Received: from mail-io1-f66.google.com (mail-io1-f66.google.com [209.85.166.66]) by imf07.hostedemail.com (Postfix) with ESMTP for ; Tue, 28 Apr 2020 01:46:03 +0000 (UTC) Received: by mail-io1-f66.google.com with SMTP id u11so21222978iow.4 for ; Mon, 27 Apr 2020 18:46:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=sHkgZl8JiLlp4ARAcWR28y56ALAkJr3brWV86XXoPwo=; b=XdZTujGXPVzegWZOzHw6AhVumJP/LW/o4SLmyBa6snSWGoqNXA2gl5RHr3ZCBa1Q1T ZNNwN3H9eI6wk0tI8pAZyH2qyB2tllzIWefuYpibfxD5sJRVy28Ns3q98perqI52xWQ8 kXTQzyF8sOZKpDhNdUxrhG3kF1TjbtjTgM1VZG9KxSP0ds2GF/dxNG2BLFzbc0s4mT4A kUArjGL8V4+BPsuNjtWATqlGpJtXwjQZhjSnqoOFawdxO9t85m9v4+gJGwJL+tOmwDp3 e0VDbUYwwPI1AIkJaWfKdN0vatIK4KNEfy6hjZcrGiUFHNgM5eDW7rQDRwKfBT87XASU +KDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=sHkgZl8JiLlp4ARAcWR28y56ALAkJr3brWV86XXoPwo=; b=tWgqlp7MwBtLLFY/Lwv/NNrTch0WNF3QSb5PdiFrWO+KbIVl3jaFJOuzrgkbaAOCCW S9vEjw1a4EYHcvV6h3fpkIJdPwyO+CiqB4vZeXBqXnFl2kH+REr+uvnvIJnGfqbhdw4f 3cuHUBNOAoNABabH5Hr29dMqgM7NIhLfYAhyAriwSi2LvkbpWFZIQaa4OqaT/OmMN6/J MB1YBdLb9mk+Bfm58B+TYM1fxUSjCDtLFU615gK37CJIZB4k/78gI2DZdXvMZdKltEiN nTHQFjOppK0nYrofoLVxivir6AWH3nUyWTPOgdVpUXX9Xs8/UFPbZzc/xUEZ49GObPx5 ZN+g== X-Gm-Message-State: AGi0PuZYgqGdxooH7FASb0eg177p1lZdpu2Vcd6YoqNJBmBqUq7xkzm5 zgTGPZun99QlheC38s2FEzByMSMwg9D7PXN1bKY= X-Google-Smtp-Source: APiQypKjZbkKpP3R4tkul4P3mnGYIVv80kR2H/z5Br54BCHQKUUebCKbtY2WADtF4Hs2EDVqETocKTTxPSpuVlC5Ehk= X-Received: by 2002:a05:6638:31b:: with SMTP id w27mr22242531jap.94.1588038363163; Mon, 27 Apr 2020 18:46:03 -0700 (PDT) MIME-Version: 1.0 References: <20200425152418.28388-1-laoar.shao@gmail.com> <20200427170540.GB29022@cmpxchg.org> In-Reply-To: <20200427170540.GB29022@cmpxchg.org> From: Yafang Shao Date: Tue, 28 Apr 2020 09:45:27 +0800 Message-ID: Subject: Re: [PATCH 0/3] mm: improve proportional memcg protection To: Johannes Weiner Cc: Andrew Morton , Michal Hocko , Roman Gushchin , Chris Down , Linux MM Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Apr 28, 2020 at 1:05 AM Johannes Weiner wrote: > > On Sat, Apr 25, 2020 at 11:24:15AM -0400, Yafang Shao wrote: > > Since proportional memory.{min, low} reclaim is introduced in > > commit 9783aa9917f8 ("mm, memcg: proportional memory.{low,min} reclaim"), > > it have been proved that the proportional reclaim is hard to understand and > > the issues caused by it is harder to understand.[1]. That dilemma faced by > > us is caused by that the proportional reclaim mixed up memcg and the > > reclaim context. > > > > In proportional reclaim, the whole reclaim context - includes the memcg > > to be reclaimed and the reclaimer, should be considered, rather than > > memcg only. > > > > To make it clear, a new member 'protection' is introduced in the reclaim > > context (struct shrink_control) to replace mem_cgroup_protection(). This > > one is set when we check whether the memcg is protected or not. > > This patch series makes the code harder to understand. > I don't think so. With this patchset, you don't need to care about what the emin of this memcg and its parent should be when you calculate the scan count. Furthermore, you don't need to worry about the potential race when you are getting the scan count. 'Protection' is calculated when we check whether the memcg is 'protected' or not, that is really easy to understand. > It's already tricky to understand ownership and lifetime/validity of > struct scan_control members when it's just vmscan.c functions touching > them. Note that the reclaimers can call the funtions existing in vmscan.c, compaction.c, slub.c, filemap.c, oom_kill.c, workingset.c, page-writeback.c, swap.c, khugepaged.c and etc. IOW, it may be used in these functions sooner or later. > Expanding the scope to the entire MM is a bad idea. > Well, mem_cgroup_protection() is only used in memcontrol.c and vmscan.c. So expanding the scope to the entire LINUX is a MORE bad idea. There may be some other functions doing this as well. > Splitting up sc->memcg_low_reclaim handling and the associated restart > logic makes control flow harder to follow as well. > Placing sc->memcg_low_reclaim in lots of callsites make the control flow more than harder to follow. > The problem with the current code is a suboptimal interface between > the memcg protection calculation and the reclaim control flow. We need > to clean that up, not remove the interface and thereby all clarity on > who handles which information. 'remove the interface ' in the CURRENT CALLSITE is really a good 'clean up'. Seems we can't get an agreement on how to improve current code. So I will submit a patch to revert the commit 9783aa9917f8 ("mm, memcg: proportional memory.{low,min} reclaim") first. Then let's talk about the good solution to implement it. BTW, I can't image why such a bad and ugly implementation got merged in the first place. The quantity of memory cgroup is becoming more and more bad. -- Thanks Yafang