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=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 E001EC433B4 for ; Thu, 15 Apr 2021 18:10:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3DF6860249 for ; Thu, 15 Apr 2021 18:10:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3DF6860249 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 B53FA6B0070; Thu, 15 Apr 2021 14:10:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B2A2C6B0071; Thu, 15 Apr 2021 14:10:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9CBCE6B0072; Thu, 15 Apr 2021 14:10:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0039.hostedemail.com [216.40.44.39]) by kanga.kvack.org (Postfix) with ESMTP id 808646B0070 for ; Thu, 15 Apr 2021 14:10:34 -0400 (EDT) Received: from smtpin02.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 4050A18215440 for ; Thu, 15 Apr 2021 18:10:34 +0000 (UTC) X-FDA: 78035391588.02.DE40857 Received: from mail-qt1-f173.google.com (mail-qt1-f173.google.com [209.85.160.173]) by imf01.hostedemail.com (Postfix) with ESMTP id BE4B45001532 for ; Thu, 15 Apr 2021 18:10:31 +0000 (UTC) Received: by mail-qt1-f173.google.com with SMTP id m16so18242522qtx.9 for ; Thu, 15 Apr 2021 11:10:32 -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=0BLFn8+sPdB8VIxTPCUa0jMJG0oUhmRim/8smTI9xvY=; b=tGeopoVpJDMNEO8IkqF3ObY96XlRxdzEQbmT9ebDYCAN4ET4ZXVB5BRc1gdegOLnak R8ziD48IV5FhyKUzjU/965vBnK3955PsmO/8G6vqpc6FGij3sKbVDMJ0k15g75yhqXwv Obm+pY4Y+EGZNhKxOR/m//LoLYgQb/8andXi1lEzejlWt68oznT+oVYvFzgAFGADEsvO D/oUUST0NO20OhtluWsk4eLbpqrQ9NiAfrHk5abtOTRRxlcNByjSosE1MWQfOpXAZY46 AWkz6NuwLa7NgR4h667WlfYY7biLEF+jWhwCyrOSthB7+/SIix5qEZZdP6NsirMSVje6 oAig== 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=0BLFn8+sPdB8VIxTPCUa0jMJG0oUhmRim/8smTI9xvY=; b=QDnjijlrycswt77+Mu0E9D7xfOFtq3TV8HO7vUT2ZVu4D4Fe2VMsWhqFS5lVTt8q4Z i4Dl0yB9NCNXsOuNhoe7iYqKRZLdfhLivYwaaH6NaYGFluUjvtXpE5FkV/v7GB4NKQok HwtVeTYxOober0U4hormgLIvmngOtk0e4zG4DYyRaYRN1EaYrOe95oGQNK1yVQeZFwAy NWEm3mLWfZoFjeObk7Lp/F2mPMlPGCY/KwJ+xQ78GbCQSM+FK60RcIfoPmNF2UHxrbLY mA29xwljsc4d56pPJD4zCCjXEBalfeBYfy/r1x1kDG8M0n7/+7xuw9MYOvdSkPxKu6/a uoaw== X-Gm-Message-State: AOAM533b2vD/BlqlqxRDIGoGQ49rzItXdgK61FYoWAKfLw6oZYOKW7wn ChgFSR1XcXKpFsMFrlrZZErqoA== X-Google-Smtp-Source: ABdhPJw6PHuhjW/tD1vcjOhY1V1iCeSWzEI3KrO9oZ5Knuwf7DACMiVKlt/E/e57U6gUYzF+4Ftuxw== X-Received: by 2002:ac8:4b7b:: with SMTP id g27mr4048708qts.220.1618510232013; Thu, 15 Apr 2021 11:10:32 -0700 (PDT) Received: from localhost (70.44.39.90.res-cmts.bus.ptd.net. [70.44.39.90]) by smtp.gmail.com with ESMTPSA id 23sm850142qkk.51.2021.04.15.11.10.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Apr 2021 11:10:31 -0700 (PDT) Date: Thu, 15 Apr 2021 14:10:30 -0400 From: Johannes Weiner To: Waiman Long Cc: Michal Hocko , Vladimir Davydov , Andrew Morton , Tejun Heo , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Vlastimil Babka , Roman Gushchin , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org, Shakeel Butt , Muchun Song , Alex Shi , Chris Down , Yafang Shao , Wei Yang , Masayoshi Mizuma , Xing Zhengjun Subject: Re: [PATCH v3 2/5] mm/memcg: Introduce obj_cgroup_uncharge_mod_state() Message-ID: References: <20210414012027.5352-1-longman@redhat.com> <20210414012027.5352-3-longman@redhat.com> <1c85e8f6-e8b9-33e1-e29b-81fbadff959f@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1c85e8f6-e8b9-33e1-e29b-81fbadff959f@redhat.com> X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: BE4B45001532 X-Stat-Signature: uydomkf3iex9gnc4966phyhmdsxz6ggo Received-SPF: none (cmpxchg.org>: No applicable sender policy available) receiver=imf01; identity=mailfrom; envelope-from=""; helo=mail-qt1-f173.google.com; client-ip=209.85.160.173 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1618510231-361690 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, Apr 15, 2021 at 12:35:45PM -0400, Waiman Long wrote: > On 4/15/21 12:30 PM, Johannes Weiner wrote: > > On Tue, Apr 13, 2021 at 09:20:24PM -0400, Waiman Long wrote: > > > In memcg_slab_free_hook()/pcpu_memcg_free_hook(), obj_cgroup_uncharge() > > > is followed by mod_objcg_state()/mod_memcg_state(). Each of these > > > function call goes through a separate irq_save/irq_restore cycle. That > > > is inefficient. Introduce a new function obj_cgroup_uncharge_mod_state() > > > that combines them with a single irq_save/irq_restore cycle. > > > > > > @@ -3292,6 +3296,25 @@ void obj_cgroup_uncharge(struct obj_cgroup *objcg, size_t size) > > > refill_obj_stock(objcg, size); > > > } > > > +void obj_cgroup_uncharge_mod_state(struct obj_cgroup *objcg, size_t size, > > > + struct pglist_data *pgdat, int idx) > > The optimization makes sense. > > > > But please don't combine independent operations like this into a > > single function. It makes for an unclear parameter list, it's a pain > > in the behind to change the constituent operations later on, and it > > has a habit of attracting more random bools over time. E.g. what if > > the caller already has irqs disabled? What if it KNOWS that irqs are > > enabled and it could use local_irq_disable() instead of save? > > > > Just provide an __obj_cgroup_uncharge() that assumes irqs are > > disabled, combine with the existing __mod_memcg_lruvec_state(), and > > bubble the irq handling up to those callsites which know better. > > > That will also work. However, the reason I did that was because of patch 5 > in the series. I could put the get_obj_stock() and put_obj_stock() code in > slab.h and allowed them to be used directly in various places, but hiding in > one function is easier. Yeah it's more obvious after getting to patch 5. But with the irq disabling gone entirely, is there still an incentive to combine the atomic section at all? Disabling preemption is pretty cheap, so it wouldn't matter to just do it twice. I.e. couldn't the final sequence in slab code simply be objcg_uncharge() mod_objcg_state() again and each function disables preemption (and in the rare case irqs) as it sees fit? You lose the irqsoff batching in the cold path, but as you say, hit rates are pretty good, and it doesn't seem worth complicating the code for the cold path.