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 2007BC369C2 for ; Thu, 17 Apr 2025 00:15:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EA23E6B0032; Wed, 16 Apr 2025 20:15:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E51436B0083; Wed, 16 Apr 2025 20:15:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D40076B0085; Wed, 16 Apr 2025 20:15:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id B6BE36B0032 for ; Wed, 16 Apr 2025 20:15:42 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 2B4D55D7AB for ; Thu, 17 Apr 2025 00:15:44 +0000 (UTC) X-FDA: 83341617408.27.E43D785 Received: from mail-qt1-f170.google.com (mail-qt1-f170.google.com [209.85.160.170]) by imf11.hostedemail.com (Postfix) with ESMTP id 50C8C4000A for ; Thu, 17 Apr 2025 00:15:42 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PTDBUzt1; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of surenb@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744848942; 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=0JJihK6dPKRsrS1PEroxMgG1IwFRWNm5iCk7/Hx4ugI=; b=oIu8ucHvkR+wk9RT0paZulSu5GxVMXDXakzoLBj6WNo1M53ZRHeG7RKmdb0/s729L8NdUk WbC47fMS4g2E4wmhss8d+KtzuydYXd/tiCRnfZCVisf25YX7CAEirVOvXYtJMx1fwFBBXc Hxmd2DFH6wT1W8rWzDQ/usgDhx6dYbE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744848942; a=rsa-sha256; cv=none; b=5wJloqWHMHpKqD7pfK8cuHz6XKHJBT+E7Picvmzzvo84InMsGQxkx2DDdz4rBYNBZB4mYs EMPvx9MK4rjkrCbCcwkY2VyyO7i2DNucTtFRqQz0wbCjqOKGi7oIAtxJ/EfIdH5ZqtGozs CeM/AtRJUXsxrkTPuFpT2jfxXwfO7A8= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=PTDBUzt1; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of surenb@google.com designates 209.85.160.170 as permitted sender) smtp.mailfrom=surenb@google.com Received: by mail-qt1-f170.google.com with SMTP id d75a77b69052e-4774611d40bso124751cf.0 for ; Wed, 16 Apr 2025 17:15:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1744848941; x=1745453741; 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=0JJihK6dPKRsrS1PEroxMgG1IwFRWNm5iCk7/Hx4ugI=; b=PTDBUzt1W4a4HOQ6WTeHSnLVhH1UcEEBOJyByNTeyc5e/9rz+xx7MH0qaZqOMPfd18 uwpjUAE4Nzh48VUzGc0ExcXjXsuTBqXSta/ZoAucbcO+BcQnldc49/2iPNQVNMAoPkc5 qtfaflXdnCbyCO5BcgQeS5EJK3HyWO31nt/Py0l2FgZeXjrT0I4+2Jaxb/5zkOsFBQYQ H+oGpgIOcQ0UpA+U7FEgGLU2ZiTUnNJMxFQjXSaYol5Z+136aDh3LHL/SdH4/NWM9S6H J8WH7zO6bxarpA2MYuJ9O3RmYZ89nkNjIx3EGS2G7KFNbmWD3/wJoiub3n0UJwiWts49 4SMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744848941; x=1745453741; 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=0JJihK6dPKRsrS1PEroxMgG1IwFRWNm5iCk7/Hx4ugI=; b=sm6+Ls63z0G62yilrlqOcH565ZBBYn7qgS+InugIw4Z2cReG3oL31Z5W6dr123SRbZ OIQ5+U6nmdJ2UAggRZQpsS58MWPAqEBTjOnfN9KbJLAPi6nO1onTLPlxw4G2mNq1mUmz yy3qv+qSCulDLESASgraZ5VG01NhhHHGTGoCVAEkGTmzRV+NzgClCsiEzEThW2JOJJA6 07joQBY7gmNCZEZjZAHGjuwgffmvR5sHHljUJpuTtP80a2xyNgQPLHxUXmCZ/oHFZoU7 KxUKjYs4vtlri+bcxjtnL6N9fpchMTezXJsP9yx6Llt4cf2gX4IT0tFbvxmmzQScAbo3 iDoQ== X-Forwarded-Encrypted: i=1; AJvYcCUpA4J9MfpFEFWWo6x2b5JYdznZkXJgqiha5ujH5CrM6yXW4jrKD1H5XPsZnmx1uHUI3GV9F3UlGQ==@kvack.org X-Gm-Message-State: AOJu0YzKX66gAga8K4or2MtR7JFOGs9bKlmc9ZXP1JmJfKDgOQ6QeJ/l Yh8ATJiHjzu5L6CRu+HjcgbX5Ao7edNVadaIJIhRFu5OY3LKLL6N1+ELWUOkQSo5+jXAIEUr4Su j7WayNtHUKCbr0ZKADyo8SAfnuB1IrGE8zpMl X-Gm-Gg: ASbGncuAce+ab46q7t0so6zD6WFIPplsx0MlhdLQwggYlEI7s2xJrWdMBLfxG1bXytu Fz8Akiuz91x59meKeTbE2P5JUSRERAeLn0gQFkgybyxZJzlhJ/mzQLS+dW2vOmCu3yv2UicEFof fz3LqPubv9WCjli3su2mdUAcZz7FG5/Y8= X-Google-Smtp-Source: AGHT+IF/DioXRmqQqk6kvZDJENGBbv0pvFx3ZLUiuZfNtUxnoKj28NHtlbAY4Fnqxj5KAMo/QQod4H/XgipEgiVtbzs= X-Received: by 2002:ac8:7dcd:0:b0:476:6232:183f with SMTP id d75a77b69052e-47adfcefbb1mr797961cf.19.1744848941092; Wed, 16 Apr 2025 17:15:41 -0700 (PDT) MIME-Version: 1.0 References: <20250416180653.3438158-1-usamaarif642@gmail.com> In-Reply-To: From: Suren Baghdasaryan Date: Wed, 16 Apr 2025 17:15:30 -0700 X-Gm-Features: ATxdqUFpXxa7Rm8fYWUTa0QsErFtD8_aTZRropD5vN4mlr9IpCSUnFupxXNcBmU Message-ID: Subject: Re: [PATCH] alloc_tag: introduce Kconfig option for default compressed profiling To: Usama Arif Cc: Andrew Morton , linux-mm@kvack.org, hannes@cmpxchg.org, shakeel.butt@linux.dev, linux-kernel@vger.kernel.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 50C8C4000A X-Rspam-User: X-Stat-Signature: 6bozue9wp8jdw11ufqxeeqz9g6tmxdbd X-HE-Tag: 1744848942-771813 X-HE-Meta: U2FsdGVkX1/7tVr2sXfvPfv2/Yc4xldyW4QdRBLB8Na0p63v5dvxZrvf+ydVqXscHoPsMoIR6gUOcLHCdil8lo8uCEwoUpfT63CXszd43Agh9QF9dFerrZ98neWJASHpbzZ48+0yFj/3+1/pDIGNlQgxq0mHGoHB98CTe23+vym7xyv2oOilapvmnMGgWsCofNpx4J6KzmafbdiO8ssHIbupbdZWz3BKl35lfDUKqx7ScizLhRAF4GgFsGJX31Z5mo+gszdmkPLCH396ozWx3KZlDcqA34AZs5RriEYPhoYTBcLs0sYTSmJjvXu3tVg9f60/FPnjC2G9Pg/1NsPvrsL1HmjuWEqFapr34sCHQ9u3a8aOwBcxgG4I9vFs+uDi2xBP++w5dH+Ew8X1emPRSaF0Gsd1IYKdZ1pqI+DD32uMzCTXXACDFwTdaJHMUMgDn/M8fN5y31AvMo8kA6b9KPYtnc+SCW1aazAObPRRqq4MJUnK4+DpMvK6UNfKWuB4DlMW5O4E1vI6jL2Kc/rBUufa/a7g09EJU1Sv7C8hbAsfj2MNVy3lXqP2D67nqRXLfrKFh6lEIJvXzZjhsi8wewhu28yLwmkYoIKr6Qx2LY9yTGakzrNnNO2mcsG/gyJzjwRmboJn0mbJ7nPodQ1qgV7y5J2kCT3RhBWE9m22BX8l7AW4PFZ9IcSd/QJE0MvyZdf2p1U+eZ+7ZEpcQFDGQwU49Y1J6lgrkCDU70uoTVjXwg5ZOLl72IGK/39ZNz7VHOUMseMH3vHcfM3LVi2QrM1CP+YBb3OcJEQGX/DRyS7YyFWNJ9ZU3qY7BcLTetY0ssg8sz4O2C5sTGc70T9Nc7uQICQ3P6PScFLVsOJQiLUWVEewonJOYuMOPiyb+ID/x6/e8Dcp4U7wU2OEjR+I6M8ohog76TvzdBYa7yF3Tn1SkwPyLjQcpLObS97ATn1k+C45VX1vTdU4RaxYoyU dkn+mGXZ o6R9PW8odUbpDb4N+kBip3Sun9WvqtlIcm4F/3zx4+TOVAT/j49pWMMPjEb4agl51v9KpupldLv08klJGttB/Uzjq5xXwu/FDWkdbM+Qeh+WYZCe0OahQ8S2qG1fdfPqdtw8Gwe42TnVyVMp0yXHqyTTOCE3ixRX2CAQKpgQYZaIE9heohEdZCxwqJHY3Ri7SlrJNux/xy1dcLKr7rlQFpp7KUU8a+11DEVbuYkBW/TlQFetPwLLFRAoP7YV00rERqqzJNe7tAmTktq4zX4/vTAAIXYnVUqlYyvjHmLp5/W4MpIYlYVLdVw1FdQ== 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, Apr 16, 2025 at 2:52=E2=80=AFPM Usama Arif = wrote: > > > > On 16/04/2025 22:08, Suren Baghdasaryan wrote: > > On Wed, Apr 16, 2025 at 11:06=E2=80=AFAM Usama Arif wrote: > >> > >> With this Kconfig option enabled, the kernel stores allocation tag ref= erences > >> in the page flags by default. > >> > >> There are 2 reasons to introduce this: > >> - As mentioned in [1], compressed tags dont have system memory overhea= d > >> and much lower performance overhead. It would be preferrable to have t= his as > >> the default option, and to be able to switch it at compile time. Anoth= er > >> option is to just declare the static key as true by default? > >> - As compressed option is the best one, it doesn't make sense to have = to > >> change both defconfig and command line options to enable memory > >> allocation profiling. Changing commandline across a large number of se= rvices > >> can result in signifcant work, which shouldn't be needed if the kernel > >> defconfig needs to be changed anyways. > > > > The reason tag compression is not the default option is because it > > works only if there are enough free bits in the page flags to store a > > tag index. If you configure it to use page flags and your build does > > not have enough free bits, the profiling will be disabled (see > > alloc_tag_sec_init()). IOW there is no graceful fallback to use page > > extensions. Therefore, the current default option is not the most > > performant but the one which works on all builds. Instead of this just > > set sysctl.vm.mem_profiling boot parameter in your config file. > > Hi Suren, Hi Usama, > > Thanks for the review! The main reason is to not have to make a change in > both defconfig and kernel command line while deploying it. We can ofcours= e > set the commandline as well, but just makes deployment more tedious, and > adds an extra commandline parameter. In our case, we only want to deploy > compressed tags, and if there aren't enough free bits, we would prefer to > disable memory allocation profiling than to take the memory and performan= ce > hit. > > Would keeping the default value of this config disabled be an acceptable = option? > i.e. the below diff on top of this patch? Well, in that case I fail to see why CONFIG_MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT=3Dy is better than CONFIG_CMDLINE=3D"sysctl.vm.mem_profiling=3D1,compressed" ? Either way you need to change the config file, no? > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 66d8995f3514..163ffcece47a 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1030,7 +1030,7 @@ config MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT > > config MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT > bool "store page allocation tag references in the page flags by d= efault" > - default y > + default n > depends on MEM_ALLOC_PROFILING > > config MEM_ALLOC_PROFILING_DEBUG > > > Thanks, > Usama > > > > Your change effectively changes the default value of > > mem_profiling_compressed and I don't see why you need to introduce a > > new config option for that. But that really does not matter because > > changing default to compressed tags is not the right choice IMO. > > > >> > >> [1] https://lore.kernel.org/all/20241023170759.999909-7-surenb@google.= com/T/#m0da08879435f7673eaa10871a6e9d1be4f605ac8 > >> > >> Signed-off-by: Usama Arif > >> --- > >> include/linux/pgalloc_tag.h | 4 ++++ > >> lib/Kconfig.debug | 5 +++++ > >> lib/alloc_tag.c | 4 ++++ > >> 3 files changed, 13 insertions(+) > >> > >> diff --git a/include/linux/pgalloc_tag.h b/include/linux/pgalloc_tag.h > >> index c74077977830..0226059bcf00 100644 > >> --- a/include/linux/pgalloc_tag.h > >> +++ b/include/linux/pgalloc_tag.h > >> @@ -16,7 +16,11 @@ extern unsigned long alloc_tag_ref_mask; > >> extern int alloc_tag_ref_offs; > >> extern struct alloc_tag_kernel_section kernel_tags; > >> > >> +#ifdef CONFIG_MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT > >> +DECLARE_STATIC_KEY_TRUE(mem_profiling_compressed); > >> +#else > >> DECLARE_STATIC_KEY_FALSE(mem_profiling_compressed); > >> +#endif > >> > >> typedef u16 pgalloc_tag_idx; > >> > >> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > >> index 9fe4d8dfe578..66d8995f3514 100644 > >> --- a/lib/Kconfig.debug > >> +++ b/lib/Kconfig.debug > >> @@ -1028,6 +1028,11 @@ config MEM_ALLOC_PROFILING_ENABLED_BY_DEFAULT > >> default y > >> depends on MEM_ALLOC_PROFILING > >> > >> +config MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT > >> + bool "store page allocation tag references in the page flags b= y default" > >> + default y > >> + depends on MEM_ALLOC_PROFILING > >> + > >> config MEM_ALLOC_PROFILING_DEBUG > >> bool "Memory allocation profiler debugging" > >> default n > >> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c > >> index 25ecc1334b67..30adad5630dd 100644 > >> --- a/lib/alloc_tag.c > >> +++ b/lib/alloc_tag.c > >> @@ -31,7 +31,11 @@ DEFINE_STATIC_KEY_MAYBE(CONFIG_MEM_ALLOC_PROFILING_= ENABLED_BY_DEFAULT, > >> mem_alloc_profiling_key); > >> EXPORT_SYMBOL(mem_alloc_profiling_key); > >> > >> +#ifdef CONFIG_MEM_ALLOC_PROFILING_COMPRESSED_ENABLED_BY_DEFAULT > >> +DEFINE_STATIC_KEY_TRUE(mem_profiling_compressed); > >> +#else > >> DEFINE_STATIC_KEY_FALSE(mem_profiling_compressed); > >> +#endif > >> > >> struct alloc_tag_kernel_section kernel_tags =3D { NULL, 0 }; > >> unsigned long alloc_tag_ref_mask; > >> -- > >> 2.47.1 > >> >