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=-6.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 4702AC54FCB for ; Fri, 24 Apr 2020 00:30:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F13F42071E for ; Fri, 24 Apr 2020 00:30:27 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="EOEONrQS" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F13F42071E 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 9BA908E0008; Thu, 23 Apr 2020 20:30:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 96B2C8E0003; Thu, 23 Apr 2020 20:30:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8A8FA8E0008; Thu, 23 Apr 2020 20:30:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0109.hostedemail.com [216.40.44.109]) by kanga.kvack.org (Postfix) with ESMTP id 74FEA8E0003 for ; Thu, 23 Apr 2020 20:30:27 -0400 (EDT) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 376DB52D5 for ; Fri, 24 Apr 2020 00:30:27 +0000 (UTC) X-FDA: 76740867294.20.cough09_2101b1f78c715 X-HE-Tag: cough09_2101b1f78c715 X-Filterd-Recvd-Size: 6465 Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by imf16.hostedemail.com (Postfix) with ESMTP for ; Fri, 24 Apr 2020 00:30:26 +0000 (UTC) Received: by mail-io1-f65.google.com with SMTP id i3so8550074ioo.13 for ; Thu, 23 Apr 2020 17:30:26 -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=MZDVazeGniMieWldqbSW2FtSsYHG/33tp3Tf9oRY5R8=; b=EOEONrQS4/T3DJI2+IKxpQXOiE51/2pTXot8WZLUTwshdDXOFSDfRBBwGXwAJcxEwG geSeHfe0x+N4z6RYp/apFt7/SfEcwMswAHxNRJdoRYCjnOppjruXM44lujQoxQdSAqnV CbbATX+78HEWZV1nbm7K+pLDvpqca6PCQwflQlqVsVy7sAdJvRpQFI2GGzpxhn57CYYm aL+EzN8MSFRSBQF4rTDQiQlrs9A756/mNRHChrt4E/RRgTO7hnOnF/5Oc9y+bTKa7UjG dnK7NBweFH5vA67rM5mvRAoBLxvrQnyLOHBJiRRltxOr9RotV6yjIQnM+OXNVYMCIsK0 6wSg== 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=MZDVazeGniMieWldqbSW2FtSsYHG/33tp3Tf9oRY5R8=; b=hu4oH7hT9JvAtbI/4nefjwuOvplio6VvEXeZFeeITA/6asmCIy3KM6Vo5PdxkKdv3U U9Ll5TV58q2EElN/g6tLNUf2RO9wPPKPWNXW1L5ZhfYeI/s+p4kj2wcxizAoehCrxYTd QqjWAWGnPvee9U+3rvMyMn4nqs4/qngOi4xTWbtDohUZ1cCt8ZmcXCZnUfFu5V6RtHYL PKnuvHdAlzuSOLqiuaHAN6uEUhjIFUQerEzr3pfxZP/RpKg1XFzFcYZx5khKRQI8Yxud McasZkZ1pG6I9b6kODMwo5qwqb3MdBkt0rZJyd4Q9PegmDYkobVzEn4i5v3VPHFa7duU dp6Q== X-Gm-Message-State: AGi0Pub5Ok8pZK55Bv2pNNXfyBFNPue4DVNp5AI5clhGbX5xUXm86X8W Qi9eiomZe0ZsSmrTYMkpPmcS1cf5qo4X28XJK/E= X-Google-Smtp-Source: APiQypIfBdphbg7tchwEL7rJI9D343ge4ubTf5XHAXR9hw6N5d4yakkgpYvFl+F+DqR1FZjcs00YEIqZ+oBOPKLU3UY= X-Received: by 2002:a5d:9042:: with SMTP id v2mr6140520ioq.81.1587688226100; Thu, 23 Apr 2020 17:30:26 -0700 (PDT) MIME-Version: 1.0 References: <20200423061629.24185-1-laoar.shao@gmail.com> <20200423210604.GB83398@carbon.DHCP.thefacebook.com> In-Reply-To: <20200423210604.GB83398@carbon.DHCP.thefacebook.com> From: Yafang Shao Date: Fri, 24 Apr 2020 08:29:50 +0800 Message-ID: Subject: Re: [PATCH] mm, memcg: fix wrong mem cgroup protection To: Roman Gushchin Cc: Andrew Morton , Michal Hocko , Vladimir Davydov , Linux MM , Chris Down , stable@vger.kernel.org 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 Fri, Apr 24, 2020 at 5:06 AM Roman Gushchin 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. > > > > Here's the explanation of this issue. > > memory.{low,min} won't take effect if the to-be-reclaimed memcg is the > > sc->target_mem_cgroup, that can also be proved by the implementation in > > mem_cgroup_protected(), see bellow, > > mem_cgroup_protected > > if (memcg == root) [2] > > return MEMCG_PROT_NONE; > > > > But this rule is ignored in mem_cgroup_protection(), which will read > > memory.{emin, elow} as the protection whatever the memcg is. > > > > How would this issue happen? > > Because in mem_cgroup_protected() we forget to clear the > > memory.{emin, elow} if the memcg is target_mem_cgroup [2]. > > > > An example to illustrate this issue. > > root_mem_cgroup > > / > > A memory.max: 1024M > > memory.min: 512M > > memory.current: 800M ('current' must be greater than 'min') > > Once kswapd starts to reclaim memcg A, it assigns 512M to memory.emin of A. > > Then kswapd stops. > > As a result of it, the memory values of A will be, > > root_mem_cgroup > > / > > A memory.max: 1024M > > memory.min: 512M > > memory.current: 512M (approximately) > > memory.emin: 512M > > > > Then a new workload starts to run in memcg A, and it will trigger memcg > > relcaim in A soon. As memcg A is the target_mem_cgroup of this > > reclaimer, so it return directly without touching memory.{emin, elow}.[2] > > The memory values of A will be, > > root_mem_cgroup > > / > > A memory.max: 1024M > > memory.min: 512M > > memory.current: 1024M (approximately) > > memory.emin: 512M > > Then this memory.emin will be used in mem_cgroup_protection() to get the > > scan count, which is obvoiusly a wrong scan count. > > > > [1]. https://lore.kernel.org/linux-mm/20200216145249.6900-1-laoar.shao@gmail.com/ > > > > Fixes: 9783aa9917f8 ("mm, memcg: proportional memory.{low,min} reclaim") > > Cc: Chris Down > > Cc: Roman Gushchin > > Cc: stable@vger.kernel.org > > Signed-off-by: Yafang Shao > > --- > > include/linux/memcontrol.h | 13 +++++++++++-- > > mm/vmscan.c | 4 ++-- > > 2 files changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > index d275c72c4f8e..114cfe06bf60 100644 > > --- a/include/linux/memcontrol.h > > +++ b/include/linux/memcontrol.h > > @@ -344,12 +344,20 @@ static inline bool mem_cgroup_disabled(void) > > return !cgroup_subsys_enabled(memory_cgrp_subsys); > > } > > > > -static inline unsigned long mem_cgroup_protection(struct mem_cgroup *memcg, > > +static inline unsigned long mem_cgroup_protection(struct mem_cgroup *root, > > + struct mem_cgroup *memcg, > > bool in_low_reclaim) > > I'd rename "root" to "target", maybe it will make the whole thing more clear. > That would make it better. I will change it. > I'll think a bit more about it, but at the first glance the patch looks sane to me. > > Thanks! -- Thanks Yafang