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 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 EE4C5C7C820 for ; Mon, 27 Apr 2020 11:32:38 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9ED6E206A4 for ; Mon, 27 Apr 2020 11:32:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="NCef+Knt" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9ED6E206A4 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 470F08E0006; Mon, 27 Apr 2020 07:32:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 420A78E0001; Mon, 27 Apr 2020 07:32:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3366B8E0006; Mon, 27 Apr 2020 07:32:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0064.hostedemail.com [216.40.44.64]) by kanga.kvack.org (Postfix) with ESMTP id 17BD18E0001 for ; Mon, 27 Apr 2020 07:32:38 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id CD726180AD804 for ; Mon, 27 Apr 2020 11:32:37 +0000 (UTC) X-FDA: 76753422354.29.trick45_16eabf720ff2d X-HE-Tag: trick45_16eabf720ff2d X-Filterd-Recvd-Size: 7780 Received: from mail-io1-f67.google.com (mail-io1-f67.google.com [209.85.166.67]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Mon, 27 Apr 2020 11:32:37 +0000 (UTC) Received: by mail-io1-f67.google.com with SMTP id u11so18465780iow.4 for ; Mon, 27 Apr 2020 04:32:37 -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=wyznoGVSn50Swv17I9gvstpYmCDmoXz3/Asps8Rg0hs=; b=NCef+KntbGRzD07q8qr6alcsb7snGUZeVql83OaX1rgZXz4XiKiTQ1g0Rgo42tiCps 3Ot4QBw4dkhQiFh9DKAPqC5NeOwSJhdcKu/z8tNmGZ+FE1+pHPq40qRbEEmkXHK7WOfz Q4cadaVFx9iztdz3xVoY//atzRXKIGLGrljiPTYnUScKmxwHzI+RuLhk1BpcVwzrFa5b nU8YtwlB85Dq8afi1UBSmAxKq4ajBlmZxep0AksBg2v5kMcLQ/B2AmvSSqiqQS+JCo4P tRLv4pz/wqVL7ZEa2sZeK2wq7g2PVvafg+GI7KEPefPQdvBgLHnnixxxj/WMU8Js15Uf 2e8g== 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=wyznoGVSn50Swv17I9gvstpYmCDmoXz3/Asps8Rg0hs=; b=r+PZzixnmAbzn6N81Cf67kcz578FOw6DmFpjHdD0Qwqrqnpbd/OJF3h3Bqz4/nEloU lN8XJxbJtYM4qil7Zaam6JIbQ+fePXnUzHJAeIRdI2rezRn0ia/NumQYrePd5N1lh4UX QuGThW4TV68dBjczm6HJ5UdOCxzBm3ufeiu19ROGV7LK2REStsGgmqFIVOxnVMCyiaTM 3IWTop7G07p4nKSmbgN4cLLifyDW+oO7YblJ2E4H/twMPbTMO+l8c7fDVWLiiOh+B2a/ zac5MqWz4xpOwyIUfuygkufAyeowqoog03Z+KcVySnjSAnsl9UFS8ALmklgnr65tJlXV PhPw== X-Gm-Message-State: AGi0PuZQEhRR1cqJLfkeOX//XKm5dLLyhOpWRtjh1Q3+e28d3+jmr7hH zbGIhVE0P7fj1iT9QwUaIBnViqkZLGSRcNrPS+4= X-Google-Smtp-Source: APiQypLx/xi+Bz16AjGAkxn94mBcLQF2Os+XxTLymCPktwu502lupA1hZEgC8EraoL618NoYia7NQu1HKfSZT0JOQVM= X-Received: by 2002:a05:6602:199:: with SMTP id m25mr20471955ioo.13.1587987156722; Mon, 27 Apr 2020 04:32:36 -0700 (PDT) MIME-Version: 1.0 References: <20200425152418.28388-1-laoar.shao@gmail.com> <20200425152418.28388-4-laoar.shao@gmail.com> <20200427094006.GD28637@dhcp22.suse.cz> <20200427105022.GE28637@dhcp22.suse.cz> <20200427112443.GF28637@dhcp22.suse.cz> In-Reply-To: <20200427112443.GF28637@dhcp22.suse.cz> From: Yafang Shao Date: Mon, 27 Apr 2020 19:32:00 +0800 Message-ID: Subject: Re: [PATCH 3/3] mm: improvements on memcg protection functions To: Michal Hocko Cc: Andrew Morton , Johannes Weiner , Roman Gushchin , Chris Down , Linux MM Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Apr 27, 2020 at 7:24 PM Michal Hocko wrote: > > On Mon 27-04-20 19:06:52, Yafang Shao wrote: > > On Mon, Apr 27, 2020 at 6:50 PM Michal Hocko wrote: > > > > > > On Mon 27-04-20 18:09:27, Yafang Shao wrote: > > > > On Mon, Apr 27, 2020 at 5:40 PM Michal Hocko wrote: > > > > > > > > > > On Sat 25-04-20 11:24:18, 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 > > > > > > > > > > s@shrink_control@scan_control@ > > > > > > > > > > > > > Thanks for pointing this out. > > > > > > > > > > one is set when we check whether the memcg is protected or not. > > > > > > > > > > > > After this change, the issue pointed by me[1] - a really old left-over > > > > > > value can slow donw target reclaim - can be fixed, and I think it could > > > > > > also avoid some potential race. > > > > > > > > > > The patch would have been much esier to review if you only focused on > > > > > the effective protection value caching. I really fail to see why you had > > > > > to make mem_cgroup_protected even more convoluted with more side effects > > > > > (e.g. sc->memcg_low_skipped). This goes directly opposite to what > > > > > Johannes was proposing in other email AFAICS. > > > > > > > > > > > > > Sorry, I failed to see what the advantage of Johannes's proposal > > > > except the better naming. > > > > > > The immediate advantage is that predicate should better not have any > > > side effect. > > > > No, it still has side effect. That is not an immediate advantage neither. > > See bellow, > > I believe you have misunderstood the proposal. > mem_cgroup_below_{min,low,protection} won't have any side effect on the > memcg. It would be only mem_cgroup_calculate_protection which updates > the cached state. So there won't be any predicate to have side effect. > Maybe I misunderstood this porposal, or both of us misunderstood this proposal. > > > So splitting into the calculation part which has clearly > > > defined side effects and having a simple predicate that consults > > > pre-calculated values makes a lot of sense to me. > > > > > > > When you calculate the effective values, the source values to > > calculate it may be changed with these values when you do the > > predicate. > > IOW, this proposal greatly increase the race window. > > Why do you think so? > See above, I don't think it is proper to disccuss it any more until we see the code. > > > > > Your changelog doesn't explain why double caching the effective value is > > > > > an improvement. > > > > > > > > The improvement is, to avoid getting an wrong value calculated by > > > > other reclaimers and avoid issues in mem_cgroup_protection() that we > > > > haven't noticed. > > > > > > This is not true in general. There is still parallel calculation done > > > and so parallel reclaimers might affect each other. Your patch only > > > makes a real difference for leaf memcgs which are the reclaim target as > > > well. > > > > The race between parallel reclaimers is fundamentally exist, and we > > can do now is reducing the race window as far as possible. > > Reducing the race window is futile. The situation changes after each > reclaim attempt. All we need is to keep a protection consistent > regardless of where the reclaim root. That means that hierarchies deeper > in the tree cannot override the protection from those which are higher. > That is what I have implemented in this patchset. > > > All intermediate nodes really do not care about the cached values > > > because they do not have any pages on the LRU lists. > > > > > > > But read these cached values can save lot of time against the existing > > code, as the existing code still calculates them even if they are > > useless. > > They are not useless. Intermediate values are necessary for the > protection distribution for lower level memcgs. > -- Thanks Yafang