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 6299CC282DE for ; Thu, 13 Mar 2025 15:26:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AAF5B280004; Thu, 13 Mar 2025 11:26:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A5F19280002; Thu, 13 Mar 2025 11:26:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8DC88280004; Thu, 13 Mar 2025 11:26:28 -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 6C56D280002 for ; Thu, 13 Mar 2025 11:26:28 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 410B6801F8 for ; Thu, 13 Mar 2025 15:26:29 +0000 (UTC) X-FDA: 83216904498.14.D7A5D5A Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf16.hostedemail.com (Postfix) with ESMTP id CABBC180011 for ; Thu, 13 Mar 2025 15:26:26 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=Ol3aOEqD; spf=pass (imf16.hostedemail.com: domain of pmladek@suse.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=pmladek@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741879587; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=nF8bJCnaqRKgut+9OeE/JCffQQlDqNay3vHyeCfE5bs=; b=NqUDHOcJvo221JAaT34FKjir3m2gi4WKH0dFo4b4H6qLjgyZaDVhsPpSoWW4wzpbBbs7kX r7XNzOlvlfziZNWC1og5VfwGMSye4wFxVRdRN8sWkIeZ8DeHIUwEXTKQMzWvlxFEFhgR80 5TBuTZrQY/0x0E1IKZ5ikrdGOq7M2jE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741879587; a=rsa-sha256; cv=none; b=n2U+w1z7gXlY5RM4H06lKJNcv2xTL+QOqaE09o6JRE6RqNO14bL04T8ZPbDOnh4WOppwnQ S3TvS9JXNvv+wLigBz/FTs1ghQRkwkyrQ2Gg1K9ZmVfLEvBpjSrQcBzWRGm3S+s4e6bt+l 3VmeL6Yko7evzGvXMfI5Z2JWjneyit4= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=Ol3aOEqD; spf=pass (imf16.hostedemail.com: domain of pmladek@suse.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=pmladek@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-4394345e4d5so7653505e9.0 for ; Thu, 13 Mar 2025 08:26:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1741879585; x=1742484385; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=nF8bJCnaqRKgut+9OeE/JCffQQlDqNay3vHyeCfE5bs=; b=Ol3aOEqDG1WL1jjqeuQYCwhjXItHI9585ENIYsqajtKGmrkUcA1aXkKxu/oHPNr7Ot R2GiUKagArDWe4xE1DL4vz33YGqju9qpDNAmOJe7WQXEiSrvzYoFDUDamxfQvwzwCe+n MkyNOeNDXx4D95JtoM8SHM3ugV+uzwGRWTVJArAZym+wLYBMjwqs2+dhffl/jR/WTc5c lwXdp7myAKdfqFJcFVa1JNOeNL3SlATre/0p3kZPyVfHj2JE0bWwp12T7Qo4oMw/pfG+ 52JoqucPn0kFtVGtom2QVBZnZfPK9MuuO6VI6mwX5SxaPaGcE96SFkoRiSwy9FHpKQli YN6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741879585; x=1742484385; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=nF8bJCnaqRKgut+9OeE/JCffQQlDqNay3vHyeCfE5bs=; b=JJL27RlqCn5nME7nCWZN1cQ2dFXGB8RLgsupwATQOVcJ1owC2HuHR0QmjZjDon0B4/ n+7TfRQDIF8HKhVbe53njuKTxdCPmU9guvfnmWbit1WA+wbJf0NrsBOwN8ipx8hBufnB mw8k+pKYSxaIj1Kfspb3bcxZNZiZz5Lv/POLOdUZoHJ1olIAdhkiZhtCbhmM4IgACqAL dk0WULeLE749hghSIJ6szBLCWBGLe0SjBSRK3ccdF7WxJP6jhBMYeeaNTypcSPBiyeYm UU5kjSZfSnFjFit8ODUyronLHNZSrtkdt4Tw+8Z2u7NAN5qc1icnyzzI5uK5jVAH6Y9a 5vig== X-Forwarded-Encrypted: i=1; AJvYcCWtcaIGjMTDIbnpmd7Ka0IuPFG/pZQZkECMysFWqLm8wFKGDgVvHIcWfeH33NgXgLTzvPqknkx0wg==@kvack.org X-Gm-Message-State: AOJu0YwTgkON6MG33+t41cpFcrutM8BuXdiGDL8GSE5Gy2OZtW/5usch 8WUYhIbZY1+fvIlYjCDVf9sWdi563KC//2/QawO/1KV/Umj+21Y6+hTAAAlKhOY= X-Gm-Gg: ASbGncsln4Xjk9Sy9x2HVYxbpdWlwZO/ESIuxOhLUIJs0BD2xI0hsPNthTy+AV69Eup LQCMTfqzZHpS77pN7/mUpGJENn/y+HvR4sFgzmBwAM3ou7YYTu6nez+t9Y+zZgBjKWBDk45eyft 9HYTQLFpNeJbjFcevJIebVGAMCG0yChXo0vcfIlXGzqGB6ulvSF1draOHewtvZKMY2Lr3IdOldl hk5JRl59OR/ExDIK8tkqfANaHAwUInf7yhcDVSjITCrtc6/EI4Y4dcUeRUgYp+RwvK/T3rEif9h qe/IlGZvFJpFKgjiUt/+p9kWQcIhcx6JCXt9HP3Cvke/Aac+c4vNCSsD1g== X-Google-Smtp-Source: AGHT+IHpEd8Fh94FY1f5OI0QJxvxQUXksTb6Cugi3gcHProMZyWArvioV2LMZr+zyl/u8VvB1ctqyw== X-Received: by 2002:a5d:5889:0:b0:391:4389:f36a with SMTP id ffacd0b85a97d-3914389f8a9mr17759445f8f.48.1741879584893; Thu, 13 Mar 2025 08:26:24 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-395c7df3512sm2584522f8f.12.2025.03.13.08.26.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Mar 2025 08:26:24 -0700 (PDT) Date: Thu, 13 Mar 2025 16:26:22 +0100 From: Petr Mladek To: Steven Rostedt Cc: LKML , Linux Trace Kernel , linux-mm@kvack.org, Masami Hiramatsu , Mathieu Desnoyers , Andrew Morton , Michael Petlan , Veronika Molnarova , Suren Baghdasaryan , Rasmus Villemoes , Andy Shevchenko , Tamir Duberstein , Linus Torvalds Subject: Re: [RESEND][PATCH] tracing: gfp: Remove duplication of recording GFP flags Message-ID: References: <20250225135611.1942b65c@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250225135611.1942b65c@gandalf.local.home> X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: CABBC180011 X-Stat-Signature: ze144xriq4but3pohp33ubxrhro9zn3i X-HE-Tag: 1741879586-752083 X-HE-Meta: U2FsdGVkX1+KaTSzAjKcFrDjxkOVo4Qcxv5PPkTqFSxrl8kP21U5aI5JBo2gUV/A7IokYJrE7snOk1T4L10ocwtz5wk5RqIHHAfaUEQNqJWEN1TeSS7pKsNEkpn0cntb4AkF8ERZHbw8UjE6Lu2CpOyid1rlXRbfLZtnx4a2PX4qM5dYT1toTQKzherlpCWwrQwHoQJTd4wkSE49TwTU0L/yjIOnNo6YjYxPLG8ZejxwA5fvpCL05Q4NCG7A+5bC/ON3NFIAx51gLNX+1KY/wNji+BbGdrJfmwLIWOgw6wZdwOwgaZe7MJj+chPI+kVd0z6ZSwa/9qbCT9P0h7Qb8Hz1gyRWcFvQZLOZvlLjVUzBIJorn2jLnl+5vBmWNqhMJxjT2Hred54khmkeB6vrtZVSx+VDDxCcapCqzLeR209Ax4levrRYhN55kqN+ZhOUiv6FFRNHf7iizrz6RxGDvY7OzT4ALh2RoZv8YBHgk8sTNBV/AaccIARDl7u3lAmAz7tHjE+sKASwjEnUeU8tXYLKfdNbW8mtt5+I1GtYjhlGVKB6EvsXVPVXfKkdVc2HtJTCtZOPe6HZLgaPfzBmj8BwShNNqAX5HiTqC+ir0/LJGo2kES7PdCF31f/mCJwO3TS6et+NuCetzKEMBPN9vW0ewxH2LAtnV9MsyxODJ3pO7jBurRtxnnIF8hyaM5u7m435q0jxYVbl0DeibXGvZTKSI+Oo3kLk9GG/NZpdUsG0hQ+vPCbpuM8C6LA/9zdYHltaU7Mz6TnS0K9ZZcotMqKn6Rbr9bxytrY+nMQv+430dIh4mwmkCgK4RjL86uYKp5PzSouf6TMgsrn5vPUZIVyebC+5wlnuJowKYzAzS+zKle9zDHH/TYQHUzO+2K6JJJ0vHtrmXajRkFZHpB3TT8jTbSB2S5rybxBFRnLn2TWnBTRRpUT3GJ9mWNcABv6+mGI2o6P3/tpgHlHdBUP H78ADD2s KX9RRlot2zIY1Zi1vG8IGHxvxk+ZRPdB3LB+0e+xUNRIIbEKH52qOVV2bPg4W68dAuDPeZCmQmoo5ENHbC7ZWGHc0ZzrFQXwhN+QC0R/p4o2ATXLT/sSAHjHsgU3C9fRKcZVtKC0+MxBcZw9LFvK2NUBi3koF7kZKAPUVd6xqsALkYvHjDubiCw7JWdTlgqvwgCDzCwfxyxT0wHYd8/2hwBgL/FmTBea5Mnt/dyXEpsW3Sz1Yf2Tv+LnY9rToSEvHYeKFwImZh3Heqn4n5Q6VbsyzMsxdM31M1cC8P17pv9uIKeOVc9B62awSRuhOb13F7/YOzZV0uEywg4rwLzii6nGcYdZwOJLUd2EI 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 Tue 2025-02-25 13:56:11, Steven Rostedt wrote: > From: Steven Rostedt > > The gfp_flags when recorded in the trace require being converted from > their numbers to values. Various macros are used to help facilitate this, > but there's two sets of macros that need to keep track of the same GFP > flags to stay in sync. > > Commit 60295b944ff68 ("tracing: gfp: Fix the GFP enum values shown for > user space tracing tools") added a TRACE_GFP_FLAGS macro that holds the > enum ___GFP_*_BIT defined bits, and creates the TRACE_DEFINE_ENUM() > wrapper around them. > > The __def_gfpflag_names() macro creates the mapping of various flags or > multiple flags to give them human readable names via the __print_flags() > tracing helper macro. > > As the TRACE_GFP_FLAGS is a subset of the __def_gfpflags_names(), it can > be used to cover the individual bit names, by redefining the internal > macro TRACE_GFP_EM(): > > #undef TRACE_GFP_EM > #define TRACE_GFP_EM(a) gfpflag_string(__GFP_##a), > > This will remove the bits that are duplicate between the two macros. If a > new bit is created, only the TRACE_GFP_FLAGS needs to be updated and that > will also update the __def_gfpflags_names() macro. > > Signed-off-by: Steven Rostedt (Google) > --- > Last version: https://lore.kernel.org/20250116214439.046082618@goodmis.org > > This was originally sent with a patch that fixed the output of gfp flags > in trace events to show human readable flags and not hex numbers. > > This patch on the other hand is a clean up as the there's now two macros > that define the bits to print. This makes the one macro use the other > macro that is a subset of the first. > > Can someone in the memory management subsystem either give me an acked-by > and I can take this through my tree, or you can just take this through > the memory management tree. Either way works for me. > > include/trace/events/mmflags.h | 41 +++++++++------------------------- > 1 file changed, 10 insertions(+), 31 deletions(-) > > diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h > index 72fbfe3caeaf..82371177ef79 100644 > --- a/include/trace/events/mmflags.h > +++ b/include/trace/events/mmflags.h > @@ -78,6 +78,13 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); > > #define gfpflag_string(flag) {(__force unsigned long)flag, #flag} > > +/* > + * For the values that match the bits, use the TRACE_GFP_FLAGS > + * which will allow any updates to be included automatically. > + */ > +#undef TRACE_GFP_EM > +#define TRACE_GFP_EM(a) gfpflag_string(__GFP_##a), > + > #define __def_gfpflag_names \ > gfpflag_string(GFP_TRANSHUGE), \ > gfpflag_string(GFP_TRANSHUGE_LIGHT), \ > @@ -91,41 +98,13 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); > gfpflag_string(GFP_NOIO), \ > gfpflag_string(GFP_NOWAIT), \ > gfpflag_string(GFP_DMA), \ > - gfpflag_string(__GFP_HIGHMEM), \ > gfpflag_string(GFP_DMA32), \ > - gfpflag_string(__GFP_HIGH), \ > - gfpflag_string(__GFP_IO), \ > - gfpflag_string(__GFP_FS), \ > - gfpflag_string(__GFP_NOWARN), \ > - gfpflag_string(__GFP_RETRY_MAYFAIL), \ > - gfpflag_string(__GFP_NOFAIL), \ > - gfpflag_string(__GFP_NORETRY), \ > - gfpflag_string(__GFP_COMP), \ > - gfpflag_string(__GFP_ZERO), \ > - gfpflag_string(__GFP_NOMEMALLOC), \ > - gfpflag_string(__GFP_MEMALLOC), \ > - gfpflag_string(__GFP_HARDWALL), \ > - gfpflag_string(__GFP_THISNODE), \ > - gfpflag_string(__GFP_RECLAIMABLE), \ > - gfpflag_string(__GFP_MOVABLE), \ > - gfpflag_string(__GFP_ACCOUNT), \ > - gfpflag_string(__GFP_WRITE), \ > gfpflag_string(__GFP_RECLAIM), \ > - gfpflag_string(__GFP_DIRECT_RECLAIM), \ > - gfpflag_string(__GFP_KSWAPD_RECLAIM), \ > - gfpflag_string(__GFP_ZEROTAGS) > - > -#ifdef CONFIG_KASAN_HW_TAGS > -#define __def_gfpflag_names_kasan , \ > - gfpflag_string(__GFP_SKIP_ZERO), \ > - gfpflag_string(__GFP_SKIP_KASAN) > -#else > -#define __def_gfpflag_names_kasan > -#endif > + TRACE_GFP_FLAGS \ > + { 0, "none" } This causes regression in the printf selftest: # modprobe test_printf modprobe: ERROR: could not insert 'test_printf': Invalid argument # dmesg | tail [ 46.206779] test_printf: vsnprintf(buf, 256, "%pGg", ...) returned 15, expected 10 [ 46.208192] test_printf: vsnprintf(buf, 3, "%pGg", ...) returned 15, expected 10 [ 46.208196] test_printf: vsnprintf(buf, 0, "%pGg", ...) returned 15, expected 10 [ 46.208199] test_printf: kvasprintf(..., "%pGg", ...) returned 'none|0xfc000000', expected '0xfc000000' [ 46.208202] test_printf: vsnprintf(buf, 256, "%pGg", ...) returned 26, expected 21 [ 46.208204] test_printf: vsnprintf(buf, 17, "%pGg", ...) returned 26, expected 21 [ 46.208206] test_printf: vsnprintf(buf, 0, "%pGg", ...) returned 26, expected 21 [ 46.208209] test_printf: kvasprintf(..., "%pGg", ...) returned '__GFP_HIGH|none|0xfc000000', expected '__GFP_HIGH|0xfc000000' [ 46.208865] test_printf: failed 8 out of 448 tests => vprintf() started printing the "none|" string. It seems to me that "{ 0, "none" }" was added as an "innocent" entry to avoid the trailing "," generated by TRACE_GFP_FLAGS. So, it is not really needed. In fact, I think that it probably causes similar regression in the trace output because the logic in trace_print_flags_seq() seems to be the same as in format_flags() in lib/vsprintf.c. The following worked for me: diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h index 82371177ef79..15aae955a10b 100644 --- a/include/trace/events/mmflags.h +++ b/include/trace/events/mmflags.h @@ -101,7 +101,7 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); gfpflag_string(GFP_DMA32), \ gfpflag_string(__GFP_RECLAIM), \ TRACE_GFP_FLAGS \ - { 0, "none" } + { 0, NULL } #define show_gfp_flags(flags) \ (flags) ? __print_flags(flags, "|", __def_gfpflag_names \ It seems to be safe because the callers end up the cycle when .name == NULL. I think that it actually allows to remove similar trailing {} but I am not sure if we want it. > > #define show_gfp_flags(flags) \ > - (flags) ? __print_flags(flags, "|", \ > - __def_gfpflag_names __def_gfpflag_names_kasan \ > + (flags) ? __print_flags(flags, "|", __def_gfpflag_names \ > ) : "none" > > #ifdef CONFIG_MMU Best Regards, Petr