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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 2A03FC433E0 for ; Mon, 15 Feb 2021 11:34:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9416464DDA for ; Mon, 15 Feb 2021 11:34:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9416464DDA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bytedance.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 02EDA8D00F0; Mon, 15 Feb 2021 06:34:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 006918D00E6; Mon, 15 Feb 2021 06:34:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E85F28D00F0; Mon, 15 Feb 2021 06:34:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0124.hostedemail.com [216.40.44.124]) by kanga.kvack.org (Postfix) with ESMTP id D2A308D00E6 for ; Mon, 15 Feb 2021 06:34:53 -0500 (EST) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 94B3B1801A0B5 for ; Mon, 15 Feb 2021 11:34:53 +0000 (UTC) X-FDA: 77820295266.01.milk60_4a0572d2763a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin01.hostedemail.com (Postfix) with ESMTP id 6FCA310054DFF for ; Mon, 15 Feb 2021 11:34:53 +0000 (UTC) X-HE-Tag: milk60_4a0572d2763a X-Filterd-Recvd-Size: 5561 Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by imf01.hostedemail.com (Postfix) with ESMTP for ; Mon, 15 Feb 2021 11:34:52 +0000 (UTC) Received: by mail-pj1-f45.google.com with SMTP id cl8so3574086pjb.0 for ; Mon, 15 Feb 2021 03:34:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=l865vVupJANTr8r5LWdTPagS4MMpwqLzjm7i9kZsffo=; b=DIldKF3h+HGUTTkXdgO1HcOE27rqK13KgreCr9rYNZM8vICXYReY2h1jmZuKluHWy6 u0OVEHaui4daB/x4TMYYhtClWcVjd1m0fdqohXkkGUvciosAGNz19JMJKwZjpgsgVYun on/KKfyItRgN7nvYufhX57/gHvVHvbYZ85k5wudAbXNnIkIYvWm3I+a1huWJHa/WETt+ T3XhrQqmyWPiGhhIldBpdvNYQe8AVk5kWi2g7mr8bcIlLaXchSl9uV+bYshA2SdQP43t LhLaWW0/L8V7oh/rZ7cmJNLPSF9/zdfqgLlci7t0NEXtRvi0YuqKCmENP0WrKrJ+Awnv 5k7Q== 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=l865vVupJANTr8r5LWdTPagS4MMpwqLzjm7i9kZsffo=; b=WwQoepZm4MHsi6iM1/fjl0eH5XpbjnASc8OYgc5Z8R/kvFJMIc5eqrf6EQIY+dAwaF yba2wKnyOqSm+D/Shd7ruIs/2WR8bHT1OGaCfS4XKhbkiKqvb9nfAflX2jPY3RNMTbvg b9Fc3AzMTLv8ASJXOhRcQXRTLgyWaxL7DjSLq03L9hieYHOk731LI/XI7UymHUdP54sL o4xg6oiyqId2Eqwner3nw/uxKCf2x61g7NXogkxQLDp3dNY5T0KP9UIftI7TrJqL/Evx OwooZpffTm+9IWe7kbkjNfhkzTc+l7TQ1jEP7Y7gSHfhizq/Xwyj/a94xGbu1KdWSuF4 KU9g== X-Gm-Message-State: AOAM531dZ33Zk4w7KfpbjfcpCWCnR6diMVs4HydSBLd9CfIxatnt1ZFw SesDN4c+RiCNM0Fw1lOHnNZVMRkUGCwVYIq+3JqUww== X-Google-Smtp-Source: ABdhPJwxcxvwODy8C+DbFH/mkU9NvXGvPpfF0/C61BusA2btA/ciWluIl8Up2QQUeiwNml5TBWl1KweOn9KuNLOY9fs= X-Received: by 2002:a17:902:9341:b029:e1:7b4e:57a8 with SMTP id g1-20020a1709029341b02900e17b4e57a8mr14845364plp.34.1613388890312; Mon, 15 Feb 2021 03:34:50 -0800 (PST) MIME-Version: 1.0 References: <20210212170159.32153-1-songmuchun@bytedance.com> <20210212170159.32153-3-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Mon, 15 Feb 2021 19:34:13 +0800 Message-ID: Subject: Re: [External] Re: [PATCH 3/4] mm: memcontrol: bail out early when id is zero To: Michal Hocko Cc: Johannes Weiner , Vladimir Davydov , Andrew Morton , Cgroups , Linux Memory Management List , LKML 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 Mon, Feb 15, 2021 at 6:27 PM Michal Hocko wrote: > > On Mon 15-02-21 18:09:44, Muchun Song wrote: > > On Mon, Feb 15, 2021 at 5:39 PM Michal Hocko wrote: > > > > > > On Sat 13-02-21 01:01:58, Muchun Song wrote: > > > > The memcg ID cannot be zero, but we can pass zero to mem_cgroup_from_id, > > > > so idr_find() is pointless and wastes CPU cycles. > > > > > > Is this possible at all to happen? If not why should we add a test for > > > _all_ invocations? > > > > Yeah, this indeed can happen. If we allocate a new swap cache page > > and charge it via mem_cgroup_charge, then the page will uncharge > > the swap counter via mem_cgroup_uncharge_swap. When the swap > > entry is indeed freed, we will call mem_cgroup_uncharge_swap again, > > In this routine, we can pass zero to mem_cgroup_from_id. Right? > > If the above claim is correct, which I would need to double check then > it should have been part of the changelog! Please think of your poor > reviewers and the time they have to invest into the review. The easy way may be adding a printk to mem_cgroup_from_id when the parameter is zero. > > I would also like to see your waste of CPU cycles argument to be backed > by something. Are we talking about cycles due to an additional function Yeah, when the parameter is already zero, idr_find() must return zero. So I think that the additional function call is unnecessary. I have added a printk to mem_cgroup_from_id, I found the parameter can be zero several times. > call? Is this really something we should even care about? Maybe not. Just my thoughts. Thanks. > > > > > > > > > Signed-off-by: Muchun Song > > > > --- > > > > mm/memcontrol.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > > index a3f26522765a..68ed4b297c13 100644 > > > > --- a/mm/memcontrol.c > > > > +++ b/mm/memcontrol.c > > > > @@ -5173,6 +5173,9 @@ static inline void mem_cgroup_id_put(struct mem_cgroup *memcg) > > > > struct mem_cgroup *mem_cgroup_from_id(unsigned short id) > > > > { > > > > WARN_ON_ONCE(!rcu_read_lock_held()); > > > > + /* The memcg ID cannot be zero. */ > > > > + if (id == 0) > > > > + return NULL; > > > > return idr_find(&mem_cgroup_idr, id); > > > > } > > > > > > > > -- > > > > 2.11.0 > > > > > > -- > > > Michal Hocko > > > SUSE Labs > > -- > Michal Hocko > SUSE Labs