From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0A4B52E3390; Thu, 13 Mar 2025 16:53:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741884801; cv=none; b=mN9095mdC2W8aqNPmF0rh0sTsT4Gp2cNVK0umgVeidWYrJuFg/w9uGz0mV6HHqmPdPzJyZOEQe6WAnFPU5StUVPzCjEOmBFgGBBuuCvkVXa/D5ECUkogBKdowgjMC+uSCfB1VIKVNlJ5h856LLPzpmQ2SmpKxs0tHnchf8cNyM0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741884801; c=relaxed/simple; bh=bZu+cNOOnGA6AZDPhiyFBGA4fxgl6uShkg70d/HmjjI=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=m6l0+VoKkzss/uyb3iAnAFAr7OOaUjU50+NA6YZIDn0YfXXxwz8kOILnuF9+dCeaU8PrvuP9K2eZpXqkbN+H5sGggwVMdW+qlxJl0spnsa+MF7ZZRn1GJ/GfUuCLaMgabO+7i30jrpbA6qzBDOShbVD+729oVDlmtN3VhJsoCS0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id B5988C4CEDD; Thu, 13 Mar 2025 16:53:16 +0000 (UTC) Date: Thu, 13 Mar 2025 12:53:13 -0400 From: Steven Rostedt To: Petr Mladek 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: <20250313125313.4f3d98f4@batman.local.home> In-Reply-To: References: <20250225135611.1942b65c@gandalf.local.home> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 13 Mar 2025 16:26:22 +0100 Petr Mladek wrote: > 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. Hmm, I could get rid of that last one with this patch. What do you think? diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h index 82371177ef79..74af49c33d14 100644 --- a/include/trace/events/mmflags.h +++ b/include/trace/events/mmflags.h @@ -37,26 +37,26 @@ TRACE_GFP_EM(HARDWALL) \ TRACE_GFP_EM(THISNODE) \ TRACE_GFP_EM(ACCOUNT) \ - TRACE_GFP_EM(ZEROTAGS) + TRACE_GFP_EME(ZEROTAGS) #ifdef CONFIG_KASAN_HW_TAGS # define TRACE_GFP_FLAGS_KASAN \ - TRACE_GFP_EM(SKIP_ZERO) \ - TRACE_GFP_EM(SKIP_KASAN) + TRACE_COMMA TRACE_GFP_EM(SKIP_ZERO) \ + TRACE_GFP_EME(SKIP_KASAN) #else # define TRACE_GFP_FLAGS_KASAN #endif #ifdef CONFIG_LOCKDEP # define TRACE_GFP_FLAGS_LOCKDEP \ - TRACE_GFP_EM(NOLOCKDEP) + TRACE_COMMA TRACE_GFP_EME(NOLOCKDEP) #else # define TRACE_GFP_FLAGS_LOCKDEP #endif #ifdef CONFIG_SLAB_OBJ_EXT # define TRACE_GFP_FLAGS_SLAB \ - TRACE_GFP_EM(NO_OBJ_EXT) + TRACE_COMMA TRACE_GFP_EME(NO_OBJ_EXT) #else # define TRACE_GFP_FLAGS_SLAB #endif @@ -69,6 +69,10 @@ #undef TRACE_GFP_EM #define TRACE_GFP_EM(a) TRACE_DEFINE_ENUM(___GFP_##a##_BIT); +#undef TRACE_GFP_EME +#define TRACE_GFP_EME(a) TRACE_DEFINE_ENUM(___GFP_##a##_BIT); +#undef TRACE_COMMA +#define TRACE_COMMA TRACE_GFP_FLAGS @@ -84,6 +88,10 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); */ #undef TRACE_GFP_EM #define TRACE_GFP_EM(a) gfpflag_string(__GFP_##a), +#undef TRACE_GFP_EME +#define TRACE_GFP_EME(a) gfpflag_string(__GFP_##a) +#undef TRACE_COMMA +#define TRACE_COMMA , #define __def_gfpflag_names \ gfpflag_string(GFP_TRANSHUGE), \ @@ -100,8 +108,7 @@ TRACE_DEFINE_ENUM(___GFP_LAST_BIT); gfpflag_string(GFP_DMA), \ gfpflag_string(GFP_DMA32), \ gfpflag_string(__GFP_RECLAIM), \ - TRACE_GFP_FLAGS \ - { 0, "none" } + TRACE_GFP_FLAGS #define show_gfp_flags(flags) \ (flags) ? __print_flags(flags, "|", __def_gfpflag_names \ -- Steve