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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,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 05127C433E0 for ; Mon, 15 Feb 2021 10:27:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 675A764D73 for ; Mon, 15 Feb 2021 10:27:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 675A764D73 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id D3D1B8D00EC; Mon, 15 Feb 2021 05:27:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CEDB78D00E6; Mon, 15 Feb 2021 05:27:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BB56D8D00EC; Mon, 15 Feb 2021 05:27:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0248.hostedemail.com [216.40.44.248]) by kanga.kvack.org (Postfix) with ESMTP id A3F228D00E6 for ; Mon, 15 Feb 2021 05:27:50 -0500 (EST) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 64EE48248047 for ; Mon, 15 Feb 2021 10:27:50 +0000 (UTC) X-FDA: 77820126300.15.7B07452 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf10.hostedemail.com (Postfix) with ESMTP id 96845407F8DB for ; Mon, 15 Feb 2021 10:27:47 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1613384868; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=73NkWy0SYucsJms5v/JqP31gNuHokmiIPgdC8knJweg=; b=Tmnax6sJaIBzZXp3R9sNYEAdnpn1Ugf0Kem4uhGfk88hDPKs3PUvyOkp27ScBspmABWtSQ x1mqyWFbV/M8ApNz3GFqshMqfOREiaOw22s46pasXI8stgyvG/yWdbk5dqb+Zpje2rMnM/ 1s+WpacNOOfge1Mf709dhpF/q0ixbAE= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id C5CFFAC32; Mon, 15 Feb 2021 10:27:48 +0000 (UTC) Date: Mon, 15 Feb 2021 11:27:47 +0100 From: Michal Hocko To: Muchun Song Cc: Johannes Weiner , Vladimir Davydov , Andrew Morton , Cgroups , Linux Memory Management List , LKML Subject: Re: [External] Re: [PATCH 3/4] mm: memcontrol: bail out early when id is zero Message-ID: References: <20210212170159.32153-1-songmuchun@bytedance.com> <20210212170159.32153-3-songmuchun@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 96845407F8DB X-Stat-Signature: j8w3qes6zijj73uwgn5qborzexejr83u Received-SPF: none (suse.com>: No applicable sender policy available) receiver=imf10; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1613384867-750762 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 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. 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 call? Is this really something we should even care about? > > > > > > 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