Message ID | 20230501165450.15352-4-surenb@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp72743vqo; Mon, 1 May 2023 10:21:29 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7P/A6ob1gDqqYlH7bFgI4UuiX5MDlA6255SFeYamAFJSUvrnVGE1WUiF8uRnBYnxdx+D5v X-Received: by 2002:a17:90a:3906:b0:24e:12f4:b74f with SMTP id y6-20020a17090a390600b0024e12f4b74fmr1938124pjb.20.1682961688792; Mon, 01 May 2023 10:21:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682961688; cv=none; d=google.com; s=arc-20160816; b=m1yDgt084kjViDC9jMEWdIS3UQboTX/8Jwo1CjW35osRE8juX6orKKicrJkVEjZGcD reO5m6kHxxPR4lA1tT8+8yjZwv4EO3mdBi1rHRMGcNNFKujDPXPP9AXEDYCpQuAupMWL H1Oo/T4yQX3xPEQna/KF27g4SfPSteY+tqenEW8eYV3CbDLxRyPYxp2v57xQEK6x84LZ /JjMqqMG7LzmRAJcYYevrD/oD1ymhwlFmYnGUk6FM4SNSB+eUb23porphkqpj2MDVlQA TbFsg0Mr+Apv/VqGfedgphp0IvE4MoThZTh/RP8FdpQyd9dDtglJ4G643dtVAGPKDzdo 8IwQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=bGdFaZ9yLSvpkzL04e/VHNU0a1vWusSmGZaxEHf2wm8=; b=LC/LIc1Dw2mdOiXYJxepJzYJVNTg6YHIDbjXOygHwedgYngbUuo9QuTGDKn2CzxJNT poWMHfTSck4jy2YISDsEUxfif+UcnqA3aycq4avu4pniyqPPfWHMH3bMZfK8TPC7exg1 8UYF5DveXvUN4YNyubO+RQN5udg7Up2yyAHmyIGrnHODxHBUUgaJX9V/NUmLOpSWIZuY LMEjiOZbS9/jmFuQwrLi8CFS1HBMuxw1kPDCKxgPv7OnBAMSJW6gvnltwzpXc054l9N/ 6fHrTnD0aSkrBj+T+hP41ivVBtzzLb1CWFhUnK58AXMInqmoZjZpM9WSHR4ziRLCLy6s I4Yg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=kNURb9oR; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id nu1-20020a17090b1b0100b0024dec043858si5017891pjb.74.2023.05.01.10.21.16; Mon, 01 May 2023 10:21:28 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20221208 header.b=kNURb9oR; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232684AbjEAQzl (ORCPT <rfc822;rbbytesnap@gmail.com> + 99 others); Mon, 1 May 2023 12:55:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60082 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232627AbjEAQzb (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 1 May 2023 12:55:31 -0400 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61863198A for <linux-kernel@vger.kernel.org>; Mon, 1 May 2023 09:55:15 -0700 (PDT) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-559ffd15df9so31972927b3.3 for <linux-kernel@vger.kernel.org>; Mon, 01 May 2023 09:55:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1682960114; x=1685552114; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=bGdFaZ9yLSvpkzL04e/VHNU0a1vWusSmGZaxEHf2wm8=; b=kNURb9oRoWKMv7g0yo2L8rW1EcO17+d5GcAx/UcGQY6Nq04eWDbD4SbdCNOLXgOYzq iIefzjvXI4Snk9VN4a44AHM12D0iuNdtL0q3JhvnQXzZtf429k8SdHQyNq1DhIH3u3D/ vgRvJtIdIGprvSKRamhSV8bn0NWTicrhYnvBvcVO6D5qUdO+wBWr5u0wD0Nu7LbQQb+M IIs2pzxiZlPELIg9+H7fv7z/PKPu/SB3zB+CWnrv1/57UJqzRl/3C3lgPuiSCXQkCpOG 4JQ2sAFEorBhte2EmAzube4SlGlrch2L/NYmCg7/2eMQe55a1P+zlu78IrB4haHDMTMn h3Fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682960114; x=1685552114; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=bGdFaZ9yLSvpkzL04e/VHNU0a1vWusSmGZaxEHf2wm8=; b=YAZAwiFnk54oFXQKJQ/GeFX5hfmqeW7V7iagKSampf9i/9bkQos5+opB7+XkJBrQR8 BCZlKgJ0aew4A8xOke1MGaDBvMTtd25e+abxG97Gr53KYvhVJlOyFD5BgSrW5zeMADl9 gbgcYZlxeVhNs5aTrbn5c1XoDjlnFZ9dqA2JQgfkjv5p1BN63QxgY4Vkv0eAo/9I0wv3 FnhKbTMwxAk64lIpKAjg+15xtvOmCMlrxcJuvgG05Fp2wdQYUkSap76a0nXRUGXV1xUD 1zVBYl3oldKb96Hw6a3Z3YOSyjCkja3js1VUG68G6o6nW4iiH2WtCd1UTRcbE15RJF0s zEvg== X-Gm-Message-State: AC+VfDwK6mMNMUYcQqmQ5VtBtHQxZ7U+cvAIjg9dATITr37R2iSMzKtY A1SA9LsyogLszxsGkhkNtYqQ8/9rFzA= X-Received: from surenb-desktop.mtv.corp.google.com ([2620:15c:211:201:6d24:3efd:facc:7ac4]) (user=surenb job=sendgmr) by 2002:a05:690c:723:b0:54f:68a1:b406 with SMTP id bt3-20020a05690c072300b0054f68a1b406mr8285886ywb.2.1682960114403; Mon, 01 May 2023 09:55:14 -0700 (PDT) Date: Mon, 1 May 2023 09:54:13 -0700 In-Reply-To: <20230501165450.15352-1-surenb@google.com> Mime-Version: 1.0 References: <20230501165450.15352-1-surenb@google.com> X-Mailer: git-send-email 2.40.1.495.gc816e09b53d-goog Message-ID: <20230501165450.15352-4-surenb@google.com> Subject: [PATCH 03/40] fs: Convert alloc_inode_sb() to a macro From: Suren Baghdasaryan <surenb@google.com> To: akpm@linux-foundation.org Cc: kent.overstreet@linux.dev, mhocko@suse.com, vbabka@suse.cz, hannes@cmpxchg.org, roman.gushchin@linux.dev, mgorman@suse.de, dave@stgolabs.net, willy@infradead.org, liam.howlett@oracle.com, corbet@lwn.net, void@manifault.com, peterz@infradead.org, juri.lelli@redhat.com, ldufour@linux.ibm.com, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, tglx@linutronix.de, mingo@redhat.com, dave.hansen@linux.intel.com, x86@kernel.org, peterx@redhat.com, david@redhat.com, axboe@kernel.dk, mcgrof@kernel.org, masahiroy@kernel.org, nathan@kernel.org, dennis@kernel.org, tj@kernel.org, muchun.song@linux.dev, rppt@kernel.org, paulmck@kernel.org, pasha.tatashin@soleen.com, yosryahmed@google.com, yuzhao@google.com, dhowells@redhat.com, hughd@google.com, andreyknvl@gmail.com, keescook@chromium.org, ndesaulniers@google.com, gregkh@linuxfoundation.org, ebiggers@google.com, ytcoode@gmail.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, bristot@redhat.com, vschneid@redhat.com, cl@linux.com, penberg@kernel.org, iamjoonsoo.kim@lge.com, 42.hyeyoo@gmail.com, glider@google.com, elver@google.com, dvyukov@google.com, shakeelb@google.com, songmuchun@bytedance.com, jbaron@akamai.com, rientjes@google.com, minchan@google.com, kaleshsingh@google.com, surenb@google.com, kernel-team@android.com, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, iommu@lists.linux.dev, linux-arch@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-modules@vger.kernel.org, kasan-dev@googlegroups.com, cgroups@vger.kernel.org, Alexander Viro <viro@zeniv.linux.org.uk> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764713236070117462?= X-GMAIL-MSGID: =?utf-8?q?1764713236070117462?= |
Series |
Memory allocation profiling
|
|
Commit Message
Suren Baghdasaryan
May 1, 2023, 4:54 p.m. UTC
From: Kent Overstreet <kent.overstreet@linux.dev> We're introducing alloc tagging, which tracks memory allocations by callsite. Converting alloc_inode_sb() to a macro means allocations will be tracked by its caller, which is a bit more useful. Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> Signed-off-by: Suren Baghdasaryan <surenb@google.com> Cc: Alexander Viro <viro@zeniv.linux.org.uk> --- include/linux/fs.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
Comments
On Mon, 1 May 2023 09:54:13 -0700 Suren Baghdasaryan <surenb@google.com> wrote: > From: Kent Overstreet <kent.overstreet@linux.dev> > > We're introducing alloc tagging, which tracks memory allocations by > callsite. Converting alloc_inode_sb() to a macro means allocations will > be tracked by its caller, which is a bit more useful. > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > --- > include/linux/fs.h | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 21a981680856..4905ce14db0b 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2699,11 +2699,7 @@ int setattr_should_drop_sgid(struct mnt_idmap *idmap, > * This must be used for allocating filesystems specific inodes to set > * up the inode reclaim context correctly. > */ > -static inline void * > -alloc_inode_sb(struct super_block *sb, struct kmem_cache *cache, gfp_t gfp) > -{ > - return kmem_cache_alloc_lru(cache, &sb->s_inode_lru, gfp); > -} > +#define alloc_inode_sb(_sb, _cache, _gfp) kmem_cache_alloc_lru(_cache, &_sb->s_inode_lru, _gfp) Honestly, I don't like this change. In general, pre-processor macros are ugly and error-prone. Besides, it works for you only because __kmem_cache_alloc_lru() is declared __always_inline (unless CONFIG_SLUB_TINY is defined, but then you probably don't want the tracking either). In any case, it's going to be difficult for people to understand why and how this works. If the actual caller of alloc_inode_sb() is needed, I'd rather add it as a parameter and pass down _RET_IP_ explicitly here. Just my two cents, Petr T
On Tue, May 02, 2023 at 02:35:30PM +0200, Petr Tesařík wrote: > On Mon, 1 May 2023 09:54:13 -0700 > Suren Baghdasaryan <surenb@google.com> wrote: > > > From: Kent Overstreet <kent.overstreet@linux.dev> > > > > We're introducing alloc tagging, which tracks memory allocations by > > callsite. Converting alloc_inode_sb() to a macro means allocations will > > be tracked by its caller, which is a bit more useful. > > > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > > --- > > include/linux/fs.h | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index 21a981680856..4905ce14db0b 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -2699,11 +2699,7 @@ int setattr_should_drop_sgid(struct mnt_idmap *idmap, > > * This must be used for allocating filesystems specific inodes to set > > * up the inode reclaim context correctly. > > */ > > -static inline void * > > -alloc_inode_sb(struct super_block *sb, struct kmem_cache *cache, gfp_t gfp) > > -{ > > - return kmem_cache_alloc_lru(cache, &sb->s_inode_lru, gfp); > > -} > > +#define alloc_inode_sb(_sb, _cache, _gfp) kmem_cache_alloc_lru(_cache, &_sb->s_inode_lru, _gfp) > > Honestly, I don't like this change. In general, pre-processor macros > are ugly and error-prone. It's a one line macro, it's fine. > Besides, it works for you only because __kmem_cache_alloc_lru() is > declared __always_inline (unless CONFIG_SLUB_TINY is defined, but then > you probably don't want the tracking either). In any case, it's going > to be difficult for people to understand why and how this works. I think you must be confused. kmem_cache_alloc_lru() is a macro, and we need that macro to be expanded at the alloc_inode_sb() callsite. It's got nothing to do with whether or not __kmem_cache_alloc_lru() is inline or not. > If the actual caller of alloc_inode_sb() is needed, I'd rather add it > as a parameter and pass down _RET_IP_ explicitly here. That approach was considered, but adding an ip parameter to every memory allocation function would've been far more churn.
On Tue, 2 May 2023 15:57:51 -0400 Kent Overstreet <kent.overstreet@linux.dev> wrote: > On Tue, May 02, 2023 at 02:35:30PM +0200, Petr Tesařík wrote: > > On Mon, 1 May 2023 09:54:13 -0700 > > Suren Baghdasaryan <surenb@google.com> wrote: > > > > > From: Kent Overstreet <kent.overstreet@linux.dev> > > > > > > We're introducing alloc tagging, which tracks memory allocations by > > > callsite. Converting alloc_inode_sb() to a macro means allocations will > > > be tracked by its caller, which is a bit more useful. > > > > > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev> > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > > Cc: Alexander Viro <viro@zeniv.linux.org.uk> > > > --- > > > include/linux/fs.h | 6 +----- > > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > > index 21a981680856..4905ce14db0b 100644 > > > --- a/include/linux/fs.h > > > +++ b/include/linux/fs.h > > > @@ -2699,11 +2699,7 @@ int setattr_should_drop_sgid(struct mnt_idmap *idmap, > > > * This must be used for allocating filesystems specific inodes to set > > > * up the inode reclaim context correctly. > > > */ > > > -static inline void * > > > -alloc_inode_sb(struct super_block *sb, struct kmem_cache *cache, gfp_t gfp) > > > -{ > > > - return kmem_cache_alloc_lru(cache, &sb->s_inode_lru, gfp); > > > -} > > > +#define alloc_inode_sb(_sb, _cache, _gfp) kmem_cache_alloc_lru(_cache, &_sb->s_inode_lru, _gfp) > > > > Honestly, I don't like this change. In general, pre-processor macros > > are ugly and error-prone. > > It's a one line macro, it's fine. It's not the same. A macro effectively adds a keyword, because it gets expanded regardless of context; for example, you can't declare a local variable called alloc_inode_sb, and the compiler errors may be quite confusing at first. See also the discussion about patch 19/40 in this series. > > Besides, it works for you only because __kmem_cache_alloc_lru() is > > declared __always_inline (unless CONFIG_SLUB_TINY is defined, but then > > you probably don't want the tracking either). In any case, it's going > > to be difficult for people to understand why and how this works. > > I think you must be confused. kmem_cache_alloc_lru() is a macro, and we > need that macro to be expanded at the alloc_inode_sb() callsite. It's > got nothing to do with whether or not __kmem_cache_alloc_lru() is inline > or not. Oh no, I am not confused. Look at the definition of kmem_cache_alloc_lru(): void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru, gfp_t gfpflags) { return __kmem_cache_alloc_lru(s, lru, gfpflags); } See? No _RET_IP_ here. That's because it's here: static __fastpath_inline void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru, gfp_t gfpflags) { void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size); trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE); return ret; } Now, if __kmem_cache_alloc_lru() is not inlined, then this _RET_IP_ will be somewhere inside kmem_cache_alloc_lru(), which is not very useful. But what is __fastpath_inline? Well, it depends: #ifndef CONFIG_SLUB_TINY #define __fastpath_inline __always_inline #else #define __fastpath_inline #endif In short, if CONFIG_SLUB_TINY is defined, it's up to the C compiler whether __kmem_cache_alloc_lru() is inlined or not. > > If the actual caller of alloc_inode_sb() is needed, I'd rather add it > > as a parameter and pass down _RET_IP_ explicitly here. > > That approach was considered, but adding an ip parameter to every memory > allocation function would've been far more churn. See my reply to patch 19/40. Rename the original function, but add an __always_inline function with the original signature, and let it take care of _RET_IP_. Petr T
diff --git a/include/linux/fs.h b/include/linux/fs.h index 21a981680856..4905ce14db0b 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2699,11 +2699,7 @@ int setattr_should_drop_sgid(struct mnt_idmap *idmap, * This must be used for allocating filesystems specific inodes to set * up the inode reclaim context correctly. */ -static inline void * -alloc_inode_sb(struct super_block *sb, struct kmem_cache *cache, gfp_t gfp) -{ - return kmem_cache_alloc_lru(cache, &sb->s_inode_lru, gfp); -} +#define alloc_inode_sb(_sb, _cache, _gfp) kmem_cache_alloc_lru(_cache, &_sb->s_inode_lru, _gfp) extern void __insert_inode_hash(struct inode *, unsigned long hashval); static inline void insert_inode_hash(struct inode *inode)