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=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 833D7C433DB for ; Fri, 19 Mar 2021 03:40:06 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id ED34464F30 for ; Fri, 19 Mar 2021 03:40:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ED34464F30 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 21FA86B006E; Thu, 18 Mar 2021 23:40:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1CFC46B0071; Thu, 18 Mar 2021 23:40:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 070C86B0072; Thu, 18 Mar 2021 23:40:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0106.hostedemail.com [216.40.44.106]) by kanga.kvack.org (Postfix) with ESMTP id DC1B56B006E for ; Thu, 18 Mar 2021 23:40:04 -0400 (EDT) Received: from smtpin35.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 85CDE942A for ; Fri, 19 Mar 2021 03:40:04 +0000 (UTC) X-FDA: 77935220328.35.3E2775A Received: from mail-lf1-f48.google.com (mail-lf1-f48.google.com [209.85.167.48]) by imf26.hostedemail.com (Postfix) with ESMTP id 2531D407F8F1 for ; Fri, 19 Mar 2021 03:40:04 +0000 (UTC) Received: by mail-lf1-f48.google.com with SMTP id q29so7835418lfb.4 for ; Thu, 18 Mar 2021 20:40:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=DN1htinD/rvt+eRcaheNveowyjIyAAls26Xb5z4rh3Y=; b=keUjPKzNyXkHT3ju6jATjlakZ+W21Mf3wMGJYydgHBGevSgtwWHJaCxwu59LAJgewI BhRJZ+XTepaOY2p2riwL2ezTECve6A27yXmZTIUey1H4nTEAsuzP8LfPa7Kx9VDVv8l2 mBUOQCpPxP++/S6nhNHmwMY9HiV3uzlChv9c7mEZeiaGG7Jm+is+B4dDMYiDT57Pao0W j2ZJHVXC+/dBdQ2ubP2r0zHN1r9FOMiYGKbQwSlDkSHdNaWLyqzWEVr8cXmhc0AKD0mX 7luAMpDc9mIzn9eLZk8YVsQCXty80tP1UbvzVPxxrvQINTizaXyVq0eUgl6k8KkD9W2i nxAg== 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=DN1htinD/rvt+eRcaheNveowyjIyAAls26Xb5z4rh3Y=; b=H8g/A23X3Mq8A7CS7T2GkmfsYbwDayUGTWCfeaWG4ZXld93ZZbqnerkma7Bqw6iJC4 8SB2c13gNr3rdyiFDEd2o6wdMgFSgTAS6+sPzU9AeWJc1k7oTF9kuv8A/EBwQ9SldOVK VDP3/HmeVY5arR4M3zQyBXjrax5WgwzD/1fxtYxcBytV95z4MAjYKxX0pFsHkhA62Fot EpNIVAf20UW8FUVcSAElnerJV+weJyzfSDImp38DAnYC1mbCr1nXFDeIN0Ws416IpGk1 wsgLI7iS2N1YRQwU9b/AL6M2mtQAKGuItNATeMSydZvCDVzuL7X4euWYu0L3Kw9H33oS h7gA== X-Gm-Message-State: AOAM531vDHpvdF2WwwrfediD08R9B1i5K18ZyzTe8QEkk8WED0cT1t5N 05n500+zRlMo6zv0RXhdyIXU0/g/j4oOy8FexHxHBg== X-Google-Smtp-Source: ABdhPJw6lb+SdqDwvZaxVMgRXpD6HXSDxZKbrG0Y8YvfqUUbvNSwUfEFB4RLN9YS0xbH8E1qzhkm71susgP21Ma3TFs= X-Received: by 2002:a05:6512:b93:: with SMTP id b19mr3286251lfv.432.1616125202132; Thu, 18 Mar 2021 20:40:02 -0700 (PDT) MIME-Version: 1.0 References: <20210318110658.60892-1-songmuchun@bytedance.com> <20210318110658.60892-5-songmuchun@bytedance.com> In-Reply-To: <20210318110658.60892-5-songmuchun@bytedance.com> From: Shakeel Butt Date: Thu, 18 Mar 2021 20:39:50 -0700 Message-ID: Subject: Re: [PATCH v4 4/5] mm: memcontrol: use obj_cgroup APIs to charge kmem pages To: Muchun Song Cc: Roman Gushchin , Johannes Weiner , Michal Hocko , Andrew Morton , Vladimir Davydov , LKML , Linux MM , Xiongchun duan Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 2531D407F8F1 X-Stat-Signature: r99dq7i6n9aa8end391ipi8hf4f4pgjd Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf26; identity=mailfrom; envelope-from=""; helo=mail-lf1-f48.google.com; client-ip=209.85.167.48 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616125204-451724 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 Thu, Mar 18, 2021 at 4:08 AM Muchun Song wrote: > [...] > > +static inline struct mem_cgroup *get_obj_cgroup_memcg(struct obj_cgroup *objcg) I would prefer get_mem_cgroup_from_objcg(). > +{ > + struct mem_cgroup *memcg; > + > + rcu_read_lock(); > +retry: > + memcg = obj_cgroup_memcg(objcg); > + if (unlikely(!css_tryget(&memcg->css))) > + goto retry; > + rcu_read_unlock(); > + > + return memcg; > +} > + > #ifdef CONFIG_MEMCG_KMEM > int memcg_alloc_page_obj_cgroups(struct page *page, struct kmem_cache *s, > gfp_t gfp, bool new_page) > @@ -3070,15 +3088,8 @@ static int obj_cgroup_charge_pages(struct obj_cgroup *objcg, gfp_t gfp, > struct mem_cgroup *memcg; > int ret; > > - rcu_read_lock(); > -retry: > - memcg = obj_cgroup_memcg(objcg); > - if (unlikely(!css_tryget(&memcg->css))) > - goto retry; > - rcu_read_unlock(); > - > + memcg = get_obj_cgroup_memcg(objcg); > ret = __memcg_kmem_charge(memcg, gfp, nr_pages); Why not manually inline __memcg_kmem_charge() here? This is the only user. Similarly manually inline __memcg_kmem_uncharge() into obj_cgroup_uncharge_pages() and call obj_cgroup_uncharge_pages() in obj_cgroup_release(). > - > css_put(&memcg->css); > > return ret; > @@ -3143,18 +3154,18 @@ static void __memcg_kmem_uncharge(struct mem_cgroup *memcg, unsigned int nr_page > */ > int __memcg_kmem_charge_page(struct page *page, gfp_t gfp, int order) > { > - struct mem_cgroup *memcg; > + struct obj_cgroup *objcg; > int ret = 0; > > - memcg = get_mem_cgroup_from_current(); This was the only use of get_mem_cgroup_from_current(). Why not remove it? > - if (memcg && !mem_cgroup_is_root(memcg)) { > - ret = __memcg_kmem_charge(memcg, gfp, 1 << order); > + objcg = get_obj_cgroup_from_current(); > + if (objcg) { > + ret = obj_cgroup_charge_pages(objcg, gfp, 1 << order); > if (!ret) { > - page->memcg_data = (unsigned long)memcg | > + page->memcg_data = (unsigned long)objcg | > MEMCG_DATA_KMEM; > return 0; > } > - css_put(&memcg->css); > + obj_cgroup_put(objcg); > } > return ret; > } [...] > static void uncharge_page(struct page *page, struct uncharge_gather *ug) > { > unsigned long nr_pages; > + struct mem_cgroup *memcg; > + struct obj_cgroup *objcg; > > VM_BUG_ON_PAGE(PageLRU(page), page); > > - if (!page_memcg(page)) > - return; > - > /* > * Nobody should be changing or seriously looking at > - * page_memcg(page) at this point, we have fully > + * page memcg or objcg at this point, we have fully > * exclusive access to the page. > */ > + if (PageMemcgKmem(page)) { > + objcg = __page_objcg(page); > + memcg = get_obj_cgroup_memcg(objcg); Can you add a comment that this get matches the put at the end of the function and kmem pages do not hold memcg references anymore. > + } else { > + memcg = __page_memcg(page); > + } > + > + if (!memcg) > + return; > > - if (ug->memcg != page_memcg(page)) { > + if (ug->memcg != memcg) { > if (ug->memcg) { > uncharge_batch(ug); > uncharge_gather_clear(ug); > } > - ug->memcg = page_memcg(page); > + ug->memcg = memcg; > ug->dummy_page = page; > > /* pairs with css_put in uncharge_batch */ > - css_get(&ug->memcg->css); > + css_get(&memcg->css); > } > > nr_pages = compound_nr(page); > - ug->nr_pages += nr_pages; > > - if (PageMemcgKmem(page)) > + if (PageMemcgKmem(page)) { > + ug->nr_memory += nr_pages; > ug->nr_kmem += nr_pages; > - else > + > + page->memcg_data = 0; > + obj_cgroup_put(objcg); > + } else { > + /* LRU pages aren't accounted at the root level */ > + if (!mem_cgroup_is_root(memcg)) > + ug->nr_memory += nr_pages; > ug->pgpgout++; > > - page->memcg_data = 0; > - css_put(&ug->memcg->css); > + page->memcg_data = 0; > + } > + > + css_put(&memcg->css); > } > > /** > -- > 2.11.0 >