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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7FE9C2BD09 for ; Wed, 3 Jul 2024 16:16:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E7A1A6B0092; Wed, 3 Jul 2024 12:16:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E031A6B0093; Wed, 3 Jul 2024 12:16:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7CBB6B0095; Wed, 3 Jul 2024 12:16:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id A07046B0092 for ; Wed, 3 Jul 2024 12:16:52 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 16B261A04F1 for ; Wed, 3 Jul 2024 16:16:52 +0000 (UTC) X-FDA: 82298945064.14.D8D1E15 Received: from mail-yw1-f172.google.com (mail-yw1-f172.google.com [209.85.128.172]) by imf29.hostedemail.com (Postfix) with ESMTP id 1FE0B120021 for ; Wed, 3 Jul 2024 16:16:48 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1JSjMWGS; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.128.172 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720023385; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=VXeCBK6xBBxTM3ezoeQkU8YiFNckh1438wTBxrn2JME=; b=WQfd7dijAshG5Oy5105UN0xcngxzxvJTkZXsivRm69ZcneAHo9i8WgsAH7BeExr4cEp0xd QKoA0ahRoa/C4hLB/RLBVYjhVZt0Uu9OrXUd1K60FpL9NxOzGbCHvoKy1zg4fMEvAbrxjV U/oWfsTlLN2cgkjwg8dn9abRXSsPDF0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720023385; a=rsa-sha256; cv=none; b=QtxskjasjJMtdNfdH/bZ8N4zoakviDBzy8JtuzR2damXHKWGOLiSepPXOwX/2q8nvQ1pkk 3HVZyybGoVRmvyEBzlm0X+F71FQ0hTyyf0REKQsjNb/NmPYI0/68CNS4co/UG7X/YGPy8J vrmJHY0bIyOngZSesMXzF0iNjRQKMVg= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1JSjMWGS; spf=pass (imf29.hostedemail.com: domain of surenb@google.com designates 209.85.128.172 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yw1-f172.google.com with SMTP id 00721157ae682-64b05fab14eso53454447b3.2 for ; Wed, 03 Jul 2024 09:16:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1720023408; x=1720628208; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=VXeCBK6xBBxTM3ezoeQkU8YiFNckh1438wTBxrn2JME=; b=1JSjMWGSMyyQU77Wi7DpfcQKT/O8qpw5laWKBAzdVwvvooRo6ATJmLD6VKYh1sKNmL qKPcYwS0Zv1VxDvFew++Kc4772keCzbHE/zJPfzO5XX1Wm5T+UFNEwc2TkxX7HjukOO2 FsJXtz4HPkAAdOK69v6VE8xxvs9shxBXnOOox0U4ZgwhR2uEMXEOHBvr8O9bWEVz6AGq kOV1d+f/DuW3gmjqwemNQYiQbQZPQYC5YeJgDayvSAZScVU1SyIaVnNQhJCZc/lmTCtQ yD8csSNFgya4mqv1V4XKKW1y78dWo6jU/heXG7jKKCgFZzYAN1NNpg9xhJ944H0i6sZf K9RQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720023408; x=1720628208; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=VXeCBK6xBBxTM3ezoeQkU8YiFNckh1438wTBxrn2JME=; b=mgCGZ8Z5n+HfDGEsQ1smW3gD4wDyjrVOY7+JGjziAGewtbOJ+tBFzHawyZuexHCTP9 EiWvegPAHCarB4pOIzaHEOBhUCqkYSYZX2bGCX63xPJSXv288+w/RpllmcWnMZlq4+bC ESLnl9M8ajiRwP22VNd1sI+BjmtvAoQ1Ip5AuV8EqcmMazteEGKNSPkv8UpIBYOeR32R vFr0OQKOPlTkg8pFK8zqWWQlHsuti7CSGn4f40FMrgLYasYy6bI/wbzByWsBp6r8b2jo xThHPHpK5Tzx5J8a36R/gk6i1SVHqKPtxvC1pAPZJ//TUhgZJzYMj6670RHi41MLkMHz 30fQ== X-Forwarded-Encrypted: i=1; AJvYcCWaq0KrGHMIj7M6GKvEEMXV3h7knycANqAe5xD437FJu8CJ3HsGv+b4k+I6Sd6Jjug3dAfuGZuxBBZ5xt3iT1WoCgk= X-Gm-Message-State: AOJu0Yxtht5sM/dEeTmdrFINKhsyVWPbWDOkUSv7WEveIUzsJ+LUYkEq FrETyDbYK7fBrRBzjHaS3YxZZyrprBWk2hSVTL09n5KskSCjP5DCoqEgiX7o+yVM0BvWD/uuwVA 4gTG/fuEML8HmHaNuVqKlSmgJ1LwwEGwGl+tW X-Google-Smtp-Source: AGHT+IHcvYoJzHzq6NPJxo48byZhuoc23xrfZJXsgV2ZEg04vdGcT3QFNtyAQKJcrv59lLV3h5fgiIXj1J8xchI2/KM= X-Received: by 2002:a81:a24e:0:b0:62f:b04c:2442 with SMTP id 00721157ae682-64c70c76533mr124349677b3.7.1720023407792; Wed, 03 Jul 2024 09:16:47 -0700 (PDT) MIME-Version: 1.0 References: <20240703015354.3370503-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Wed, 3 Jul 2024 09:16:34 -0700 Message-ID: Subject: Re: [PATCH 1/1] mm, slab: move allocation tagging code in the alloc path into a hook To: Vlastimil Babka Cc: akpm@linux-foundation.org, kent.overstreet@linux.dev, cl@linux.com, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1FE0B120021 X-Stat-Signature: 9t5ru9qp8qjmtdn9pkbncbsx7jajiu5x X-HE-Tag: 1720023408-499642 X-HE-Meta: U2FsdGVkX1/wyx0DYmUcrKTAfDJbTh30vT06YKXCeZ4M36VwBwZ9oI/PvZSv/MGR4ehIUbYVhHrR6AH+Gc3ac1zhsTiRgmqZdQ7w7E9JSybNNymM2OpizcB/OsZgtwmR8IrewySqbwrPjBba78pooYckUfX7rjWtKzUbQ7+cVfjtKUr0zUm/SkMXqL7eShm3bJg/afCBMahaU3ewcSOvk76IxDXITt8XIWZGB78NNuNjDNqIAJC1760V1ZkmDY0BuEsW8X8ej+S5XAReLQOETmWdVIDNvkKqDN+b5y0iGPpYeLqOwmCFE0xr+YGEfGXnJ5adfRju1sj7SOtLjpMsGMhLsqUX76syuoqs14peeWNuoWPKUF5OZ1K/+Ng0ns7Ae5R2+igZ8v6jIivGvNknOwAY7T6Al9coivmYJ++I23mFTyMsWO3gy++JSywEUSgo+TEbI4b2OuWENDobsI2YvJtkq7nCSj0dxt8CTBz6/2jW/zduLlEiLrDdTqD2XNtDtXTnKXsxbAXyKJL/zidvZO8iLCzHTL3m2AwPJLe9Jxu746Q68rgpv/7L8wUckfeWllHxzGeiIfH7/vie0j8LgJV3afFZxy1tn4laevQGEytwwx3oz56lOD6BJ0+fogYFYO8l5Dt5ovvcwHuueKiTolqCsAHEpzOku8QIQRStfBH7LyBVfKFKhddyXxtt6U8zRBx+mpkei1r/wbr7Uwquryoj14ObDzksAC4dWo2BIDaZvP3mFKLq0HXxYv61x5a2bqZowV+c2f6lqc+OAOD3k0hUH9+ioZCDELuoP60SBWpLbIKDuc4Tsdly1+K003WDwuRmaNG7DjSzBzZDmQhymeVnpEJEtYVL5GWwfUA2RBsgLDZSCi+DsZhs4P2+ObuQC8FH36/3I0JVtXuPEopvBgUjFxaNapHl+tpcGxIIg2yrDdtLrHNS6fAla/UZ2MO55LfwbksLut2xPdjisi3 EHNF4cL8 1dLnenYPc7T0wUwMVWoyfSG8/7Z7jY3q95/u0WsCzA66u88BCFDhvuyn3kT0kdZ/gbNiYFXKdeqDB7o7luC5ClpF0Pt2Q3lOo24BVfHeCmFZsMXjROLvQmOjFQ242RQT/Ycbysg6BWTGmm+pxMN6xTu4k/dpZPD8JB/4SQ807rhzCCA9xSpVOQeJYMzegIN24GprW 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: List-Subscribe: List-Unsubscribe: On Wed, Jul 3, 2024 at 3:47=E2=80=AFAM Vlastimil Babka wro= te: > > On 7/3/24 3:53 AM, Suren Baghdasaryan wrote: > > Move allocation tagging specific code in the allocation path into > > alloc_tagging_slab_alloc_hook, similar to how freeing path uses > > alloc_tagging_slab_free_hook. No functional changes, just code > > cleanup. > > > > Suggested-by: Vlastimil Babka > > Signed-off-by: Suren Baghdasaryan > > --- > > mm/slub.c | 34 +++++++++++++++++++++++++++++----- > > 1 file changed, 29 insertions(+), 5 deletions(-) > > > > diff --git a/mm/slub.c b/mm/slub.c > > index 4927edec6a8c..99d53190cfcf 100644 > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -2033,11 +2033,18 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s= , gfp_t flags, void *p) > > return slab_obj_exts(slab) + obj_to_index(s, slab, p); > > } > > > > +#ifdef CONFIG_MEM_ALLOC_PROFILING > > I think if you extract this whole #ifdef CONFIG_MEM_ALLOC_PROFILING secti= on > to go outside of CONFIG_SLAB_OBJ_EXT sections, i.e. below the final > > #endif /* CONFIG_SLAB_OBJ_EXT */ > > then it wouldn't be necessary to have two instances of the empty hooks? > > > + > > +static inline void > > +alloc_tagging_slab_alloc_hook(struct slabobj_ext *obj_exts, unsigned i= nt size) > > +{ > > + alloc_tag_add(&obj_exts->ref, current->alloc_tag, size); > > +} > > + > > static inline void > > alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, = void **p, > > int objects) > > { > > -#ifdef CONFIG_MEM_ALLOC_PROFILING > > struct slabobj_ext *obj_exts; > > int i; > > > > @@ -2053,9 +2060,23 @@ alloc_tagging_slab_free_hook(struct kmem_cache *= s, struct slab *slab, void **p, > > > > alloc_tag_sub(&obj_exts[off].ref, s->size); > > } > > -#endif > > } > > > > +#else /* CONFIG_MEM_ALLOC_PROFILING */ > > + > > +static inline void > > +alloc_tagging_slab_alloc_hook(struct slabobj_ext *obj_exts, unsigned i= nt size) > > +{ > > +} > > + > > +static inline void > > +alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, = void **p, > > + int objects) > > +{ > > +} > > + > > +#endif /* CONFIG_MEM_ALLOC_PROFILING*/ > > + > > #else /* CONFIG_SLAB_OBJ_EXT */ > > > > static int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s= , > > @@ -2079,6 +2100,11 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s,= gfp_t flags, void *p) > > return NULL; > > } > > > > +static inline void > > +alloc_tagging_slab_alloc_hook(struct slabobj_ext *obj_exts, unsigned i= nt size) > > +{ > > +} > > + > > static inline void > > alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, = void **p, > > int objects) > > @@ -3944,7 +3970,6 @@ bool slab_post_alloc_hook(struct kmem_cache *s, s= truct list_lru *lru, > > kmemleak_alloc_recursive(p[i], s->object_size, 1, > > s->flags, init_flags); > > kmsan_slab_alloc(s, p[i], init_flags); > > -#ifdef CONFIG_MEM_ALLOC_PROFILING > > if (need_slab_obj_ext()) { > > struct slabobj_ext *obj_exts; > > > > @@ -3955,9 +3980,8 @@ bool slab_post_alloc_hook(struct kmem_cache *s, s= truct list_lru *lru, > > * check should be added before alloc_tag_add(). > > */ > > if (likely(obj_exts)) > > - alloc_tag_add(&obj_exts->ref, current->al= loc_tag, s->size); > > + alloc_tagging_slab_alloc_hook(obj_exts, s= ->size); > > } > > Could this whole "if (need_slab_obj_ext())" block be part of > alloc_tagging_slab_alloc_hook()? That would match > __memcg_slab_post_alloc_hook also taking care of calloing > alloc_slab_obj_exts on its own. Maybe then we won't even need empty versi= ons > of prepare_slab_obj_exts_hook() Yeah, since we don't have other slab_obj_ext users yet, we can simplify the code that way. Originally I wanted to keep it more generic and make it easy to add new slab_obj_ext users, but I'm fine with this simplification. When there are other users we can change the code back to be more generic. I'll post v2 shortly with your suggestions. Thanks, Suren. > > > -#endif > > } > > > > return memcg_slab_post_alloc_hook(s, lru, flags, size, p); > > > > base-commit: e9d22f7a6655941fc8b2b942ed354ec780936b3e >