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=-3.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,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 C9446C54FD0 for ; Fri, 24 Apr 2020 16:51:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7375220774 for ; Fri, 24 Apr 2020 16:51:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg-org.20150623.gappssmtp.com header.i=@cmpxchg-org.20150623.gappssmtp.com header.b="cLqfDo0z" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7375220774 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id CF41A8E0005; Fri, 24 Apr 2020 12:51:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CA6248E0003; Fri, 24 Apr 2020 12:51:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B93C28E0005; Fri, 24 Apr 2020 12:51:06 -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 A23DD8E0003 for ; Fri, 24 Apr 2020 12:51:06 -0400 (EDT) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 68E0A180AD806 for ; Fri, 24 Apr 2020 16:51:06 +0000 (UTC) X-FDA: 76743338532.30.eye72_9968f89d544f X-HE-Tag: eye72_9968f89d544f X-Filterd-Recvd-Size: 5995 Received: from mail-qt1-f193.google.com (mail-qt1-f193.google.com [209.85.160.193]) by imf18.hostedemail.com (Postfix) with ESMTP for ; Fri, 24 Apr 2020 16:51:05 +0000 (UTC) Received: by mail-qt1-f193.google.com with SMTP id e17so5023170qtp.7 for ; Fri, 24 Apr 2020 09:51:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=LLWgRgBeL6u7CTZehG2n8AGRFKAbzykj38v03B4IxgI=; b=cLqfDo0zDUyTc3G0cD2LyHxwDW6bhH8PIG39B1OGCcaNDjr+nFiZXz/ZCrjWOOAmxy NgumJZEK575/8193aDuOzuyWwQ1pAg9I985PLlTjaW4rBesVNscQc5PB3JoLaOOKL2Xw X5rKIUJ9UL6ShRxq0EgY70/nOW56yyvqzymPPDH7S/W2cx00wzRCYFI1RuT3p6xoWlKq h5l8D0YjARMDMxm/CPmQetAGNmJ0ZOGMC5nuWSBdQjZJNYjUosy1oVPWhjEGinqgsPqj PxMzH83KBmgUN9DkB/3wSaCuqntzi5vVGDEqca/9h8oV7RKUA0pOWPw8SST5BqaAVgGB hlAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=LLWgRgBeL6u7CTZehG2n8AGRFKAbzykj38v03B4IxgI=; b=P4/YY/IEDgGkDWJqxIg50h7BB3s7+rXe20PP5hLHmqUTLVFmNVihcHci/9/1kGoruy K2eX8jR/9TI0kOZ4q1qMA1RjSyodBdWmI2QWa8cJIKMwBX2wAJjXNUfHCQmos0Tdtdjj aaOGqSV092mXL2EGshxbr7n8PnXYecXx9V3LVSxIAnYqsO/OIu9Jsi+7OsfK1UnpkCsF 2irV8F8VUm4/kwKiMIAnkgMaL7S87nZdqYyGZ32WPhRBFqFMdqNu2wOZQ7ocotkRo8ST RKxmaKiwd0kGa6dzTV4QByt+8/wXHl/2UihndccQx2Qf2TO032Co5n4DoqcU3h1+J3ll M48Q== X-Gm-Message-State: AGi0PubG5fI8/tAcEQhmZNbXzT5g8BBGTCSYlaCXdSuPbDy7NUSa9zXq OhpIFnFFaQqxDmp3paXz3AU4BQ== X-Google-Smtp-Source: APiQypLYX8tycLPSQbBePM3tGAGwiQADl6QX/fJ1sGqHE6YRqJ2vJqRkXUxRK3KRg738BARfBcwvsQ== X-Received: by 2002:ac8:3421:: with SMTP id u30mr10737775qtb.303.1587747065043; Fri, 24 Apr 2020 09:51:05 -0700 (PDT) Received: from localhost ([2620:10d:c091:480::921]) by smtp.gmail.com with ESMTPSA id l9sm4177675qth.60.2020.04.24.09.51.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Apr 2020 09:51:04 -0700 (PDT) Date: Fri, 24 Apr 2020 12:51:03 -0400 From: Johannes Weiner To: Michal Hocko Cc: Yafang Shao , akpm@linux-foundation.org, vdavydov.dev@gmail.com, linux-mm@kvack.org, Chris Down , Roman Gushchin , stable@vger.kernel.org Subject: Re: [PATCH] mm, memcg: fix wrong mem cgroup protection Message-ID: <20200424165103.GA575707@cmpxchg.org> References: <20200423061629.24185-1-laoar.shao@gmail.com> <20200424131450.GA495720@cmpxchg.org> <20200424142958.GF11591@dhcp22.suse.cz> <20200424151013.GA525165@cmpxchg.org> <20200424162103.GK11591@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200424162103.GK11591@dhcp22.suse.cz> 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 Fri, Apr 24, 2020 at 06:21:03PM +0200, Michal Hocko wrote: > On Fri 24-04-20 11:10:13, Johannes Weiner wrote: > > On Fri, Apr 24, 2020 at 04:29:58PM +0200, Michal Hocko wrote: > > > On Fri 24-04-20 09:14:50, Johannes Weiner wrote: > > > > On Thu, Apr 23, 2020 at 02:16:29AM -0400, Yafang Shao wrote: > > > > > This patch is an improvement of a previous version[1], as the previous > > > > > version is not easy to understand. > > > > > This issue persists in the newest kernel, I have to resend the fix. As > > > > > the implementation is changed, I drop Roman's ack from the previous > > > > > version. > > > > > > > > Now that I understand the problem, I much prefer the previous version. > > > > > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > > index 745697906ce3..2bf91ae1e640 100644 > > > > --- a/mm/memcontrol.c > > > > +++ b/mm/memcontrol.c > > > > @@ -6332,8 +6332,19 @@ enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root, > > > > > > > > if (!root) > > > > root = root_mem_cgroup; > > > > - if (memcg == root) > > > > + if (memcg == root) { > > > > + /* > > > > + * The cgroup is the reclaim root in this reclaim > > > > + * cycle, and therefore not protected. But it may have > > > > + * stale effective protection values from previous > > > > + * cycles in which it was not the reclaim root - for > > > > + * example, global reclaim followed by limit reclaim. > > > > + * Reset these values for mem_cgroup_protection(). > > > > + */ > > > > + memcg->memory.emin = 0; > > > > + memcg->memory.elow = 0; > > > > return MEMCG_PROT_NONE; > > > > + } > > > > > > Could you be more specific why you prefer this over the > > > mem_cgroup_protection which doesn't change the effective value? > > > Isn't it easier to simply ignore effective value for the reclaim roots? > > > > Because now both mem_cgroup_protection() and mem_cgroup_protected() > > have to know about the reclaim root semantics, instead of just the one > > central place. > > Yes this is true but it is also potentially overwriting the state with > a parallel reclaim which can lead to surprising results Checking in mem_cgroup_protection() doesn't avoid the fundamental race: root `- A (low=2G, elow=2G, max=3G) `- A1 (low=2G, elow=2G) If A does limit reclaim while global reclaim races, the memcg == root check in mem_cgroup_protection() will reliably calculate the "right" scan value for A, which has no pages, and the wrong scan value for A1 where the memory actually is. I'm okay with fixing the case where a really old left-over value is used by target reclaim. I don't see a point in special casing this one instance of a fundamental race condition at the expense of less robust code.