linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] percpu: improve percpu_alloc_percpu event trace
       [not found] <8d627f02-183f-c4e7-7c15-77b2b438536b@openvz.org>
@ 2022-05-06 20:38 ` kernel test robot
       [not found]   ` <e1c09bbb-2c58-a986-c704-1db538da905a@openvz.org>
  0 siblings, 1 reply; 9+ messages in thread
From: kernel test robot @ 2022-05-06 20:38 UTC (permalink / raw)
  To: Vasily Averin, Shakeel Butt, Steven Rostedt, Ingo Molnar
  Cc: kbuild-all, kernel, linux-kernel, Roman Gushchin,
	Vlastimil Babka, Michal Hocko, cgroups, Andrew Morton,
	Linux Memory Management List, Dennis Zhou, Tejun Heo,
	Christoph Lameter

Hi Vasily,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rostedt-trace/for-next]
[also build test WARNING on hnaz-mm/master v5.18-rc5]
[cannot apply to dennis-percpu/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Vasily-Averin/percpu-improve-percpu_alloc_percpu-event-trace/20220506-124742
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: i386-randconfig-s001 (https://download.01.org/0day-ci/archive/20220507/202205070420.aAhuqpYk-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/intel-lab-lkp/linux/commit/dee6876db0a7a4715516e673f9edaca2ba40677c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Vasily-Averin/percpu-improve-percpu_alloc_percpu-event-trace/20220506-124742
        git checkout dee6876db0a7a4715516e673f9edaca2ba40677c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   mm/percpu.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/percpu.h):
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast from restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected unsigned long flags @@     got restricted gfp_t [usertype] gfp_flags @@
   include/trace/events/percpu.h:11:1: sparse:     expected unsigned long flags
   include/trace/events/percpu.h:11:1: sparse:     got restricted gfp_t [usertype] gfp_flags
   mm/percpu.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/percpu.h):
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast to restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: cast to restricted gfp_t
>> include/trace/events/percpu.h:11:1: sparse: sparse: restricted gfp_t degrades to integer
>> include/trace/events/percpu.h:11:1: sparse: sparse: restricted gfp_t degrades to integer
   mm/percpu.c:2012:24: sparse: sparse: context imbalance in 'pcpu_balance_free' - unexpected unlock

vim +11 include/trace/events/percpu.h

df95e795a72289 Dennis Zhou   2017-06-19  10  
df95e795a72289 Dennis Zhou   2017-06-19 @11  TRACE_EVENT(percpu_alloc_percpu,
df95e795a72289 Dennis Zhou   2017-06-19  12  
df95e795a72289 Dennis Zhou   2017-06-19  13  	TP_PROTO(bool reserved, bool is_atomic, size_t size,
dee6876db0a7a4 Vasily Averin 2022-05-06  14  		 size_t align, void *base_addr, int off,
dee6876db0a7a4 Vasily Averin 2022-05-06  15  		 void __percpu *ptr, size_t bytes_alloc, gfp_t gfp_flags),
df95e795a72289 Dennis Zhou   2017-06-19  16  
dee6876db0a7a4 Vasily Averin 2022-05-06  17  	TP_ARGS(reserved, is_atomic, size, align, base_addr, off, ptr,
dee6876db0a7a4 Vasily Averin 2022-05-06  18  		bytes_alloc, gfp_flags),
df95e795a72289 Dennis Zhou   2017-06-19  19  
df95e795a72289 Dennis Zhou   2017-06-19  20  	TP_STRUCT__entry(
df95e795a72289 Dennis Zhou   2017-06-19  21  		__field(	bool,			reserved	)
df95e795a72289 Dennis Zhou   2017-06-19  22  		__field(	bool,			is_atomic	)
df95e795a72289 Dennis Zhou   2017-06-19  23  		__field(	size_t,			size		)
df95e795a72289 Dennis Zhou   2017-06-19  24  		__field(	size_t,			align		)
df95e795a72289 Dennis Zhou   2017-06-19  25  		__field(	void *,			base_addr	)
df95e795a72289 Dennis Zhou   2017-06-19  26  		__field(	int,			off		)
df95e795a72289 Dennis Zhou   2017-06-19  27  		__field(	void __percpu *,	ptr		)
dee6876db0a7a4 Vasily Averin 2022-05-06  28  		__field(	size_t,			bytes_alloc	)
dee6876db0a7a4 Vasily Averin 2022-05-06  29  		__field(	gfp_t,			gfp_flags	)
df95e795a72289 Dennis Zhou   2017-06-19  30  	),
df95e795a72289 Dennis Zhou   2017-06-19  31  	TP_fast_assign(
df95e795a72289 Dennis Zhou   2017-06-19  32  		__entry->reserved	= reserved;
df95e795a72289 Dennis Zhou   2017-06-19  33  		__entry->is_atomic	= is_atomic;
df95e795a72289 Dennis Zhou   2017-06-19  34  		__entry->size		= size;
df95e795a72289 Dennis Zhou   2017-06-19  35  		__entry->align		= align;
df95e795a72289 Dennis Zhou   2017-06-19  36  		__entry->base_addr	= base_addr;
df95e795a72289 Dennis Zhou   2017-06-19  37  		__entry->off		= off;
df95e795a72289 Dennis Zhou   2017-06-19  38  		__entry->ptr		= ptr;
dee6876db0a7a4 Vasily Averin 2022-05-06  39  		__entry->bytes_alloc	= bytes_alloc;
dee6876db0a7a4 Vasily Averin 2022-05-06  40  		__entry->gfp_flags	= gfp_flags;
df95e795a72289 Dennis Zhou   2017-06-19  41  	),
df95e795a72289 Dennis Zhou   2017-06-19  42  
dee6876db0a7a4 Vasily Averin 2022-05-06  43  	TP_printk("reserved=%d is_atomic=%d size=%zu align=%zu base_addr=%p off=%d ptr=%p bytes_alloc=%zu gfp_flags=%s",
df95e795a72289 Dennis Zhou   2017-06-19  44  		  __entry->reserved, __entry->is_atomic,
df95e795a72289 Dennis Zhou   2017-06-19  45  		  __entry->size, __entry->align,
dee6876db0a7a4 Vasily Averin 2022-05-06  46  		  __entry->base_addr, __entry->off, __entry->ptr,
dee6876db0a7a4 Vasily Averin 2022-05-06  47  		  __entry->bytes_alloc, show_gfp_flags(__entry->gfp_flags))
df95e795a72289 Dennis Zhou   2017-06-19  48  );
df95e795a72289 Dennis Zhou   2017-06-19  49  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm] tracing: incorrect gfp_t conversion
       [not found]     ` <331d88fe-f4f7-657c-02a2-d977f15fbff6@openvz.org>
@ 2022-05-07 19:37       ` Andrew Morton
       [not found]         ` <8b1cfefa-da7d-3376-cf04-1ff77dab8170@openvz.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2022-05-07 19:37 UTC (permalink / raw)
  To: Vasily Averin; +Cc: Steven Rostedt, Ingo Molnar, kernel, linux-kernel, linux-mm

On Sat, 7 May 2022 22:02:05 +0300 Vasily Averin <vvs@openvz.org> wrote:

> Fixes the following sparse warnings:
> 
> include/trace/events/*: sparse: cast to restricted gfp_t
> include/trace/events/*: sparse: restricted gfp_t degrades to integer
> 
> gfp_t type is bitwise and requires __force attributes for any casts.
> 
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -14,43 +14,43 @@
>   */
>  
>  #define __def_gfpflag_names						\
> -	{(unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> -	{(unsigned long)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
> -	{(unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"},\
> -	{(unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
> -	{(unsigned long)GFP_USER,		"GFP_USER"},		\
> -	{(unsigned long)GFP_KERNEL_ACCOUNT,	"GFP_KERNEL_ACCOUNT"},	\
> -	{(unsigned long)__GFP_SKIP_KASAN_POISON,"__GFP_SKIP_KASAN_POISON"}\
>
> ...
>
> +	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> +	{(__force unsigned long)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
> +	{(__force unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"},\
> +	{(__force unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
> +	{(__force unsigned long)GFP_USER,		"GFP_USER"},		\
> +	{(__force unsigned long)GFP_KERNEL_ACCOUNT,	"GFP_KERNEL_ACCOUNT"},	\
> +	{(__force unsigned long)GFP_KERNEL,		"GFP_KERNEL"},		\
> +	{(__force unsigned long)GFP_NOFS,		"GFP_NOFS"},		\

This got all repetitive, line-wrappy and ugly :(

What do we think of something silly like this?



--- a/include/trace/events/mmflags.h~tracing-incorrect-gfp_t-conversion-fix
+++ a/include/trace/events/mmflags.h
@@ -13,53 +13,57 @@
  * Thus most bits set go first.
  */
 
+#define FUL __force unsigned long
+
 #define __def_gfpflag_names						\
-	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
-	{(__force unsigned long)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
-	{(__force unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"},\
-	{(__force unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
-	{(__force unsigned long)GFP_USER,		"GFP_USER"},		\
-	{(__force unsigned long)GFP_KERNEL_ACCOUNT,	"GFP_KERNEL_ACCOUNT"},	\
-	{(__force unsigned long)GFP_KERNEL,		"GFP_KERNEL"},		\
-	{(__force unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
-	{(__force unsigned long)GFP_ATOMIC,		"GFP_ATOMIC"},		\
-	{(__force unsigned long)GFP_NOIO,		"GFP_NOIO"},		\
-	{(__force unsigned long)GFP_NOWAIT,		"GFP_NOWAIT"},		\
-	{(__force unsigned long)GFP_DMA,		"GFP_DMA"},		\
-	{(__force unsigned long)__GFP_HIGHMEM,		"__GFP_HIGHMEM"},	\
-	{(__force unsigned long)GFP_DMA32,		"GFP_DMA32"},		\
-	{(__force unsigned long)__GFP_HIGH,		"__GFP_HIGH"},		\
-	{(__force unsigned long)__GFP_ATOMIC,		"__GFP_ATOMIC"},	\
-	{(__force unsigned long)__GFP_IO,		"__GFP_IO"},		\
-	{(__force unsigned long)__GFP_FS,		"__GFP_FS"},		\
-	{(__force unsigned long)__GFP_NOWARN,		"__GFP_NOWARN"},	\
-	{(__force unsigned long)__GFP_RETRY_MAYFAIL,	"__GFP_RETRY_MAYFAIL"},	\
-	{(__force unsigned long)__GFP_NOFAIL,		"__GFP_NOFAIL"},	\
-	{(__force unsigned long)__GFP_NORETRY,		"__GFP_NORETRY"},	\
-	{(__force unsigned long)__GFP_COMP,		"__GFP_COMP"},		\
-	{(__force unsigned long)__GFP_ZERO,		"__GFP_ZERO"},		\
-	{(__force unsigned long)__GFP_NOMEMALLOC,	"__GFP_NOMEMALLOC"},	\
-	{(__force unsigned long)__GFP_MEMALLOC,		"__GFP_MEMALLOC"},	\
-	{(__force unsigned long)__GFP_HARDWALL,		"__GFP_HARDWALL"},	\
-	{(__force unsigned long)__GFP_THISNODE,		"__GFP_THISNODE"},	\
-	{(__force unsigned long)__GFP_RECLAIMABLE,	"__GFP_RECLAIMABLE"},	\
-	{(__force unsigned long)__GFP_MOVABLE,		"__GFP_MOVABLE"},	\
-	{(__force unsigned long)__GFP_ACCOUNT,		"__GFP_ACCOUNT"},	\
-	{(__force unsigned long)__GFP_WRITE,		"__GFP_WRITE"},		\
-	{(__force unsigned long)__GFP_RECLAIM,		"__GFP_RECLAIM"},	\
-	{(__force unsigned long)__GFP_DIRECT_RECLAIM,	"__GFP_DIRECT_RECLAIM"},\
-	{(__force unsigned long)__GFP_KSWAPD_RECLAIM,	"__GFP_KSWAPD_RECLAIM"},\
-	{(__force unsigned long)__GFP_ZEROTAGS,		"__GFP_ZEROTAGS"}	\
+	{(FUL)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
+	{(FUL)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
+	{(FUL)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"},\
+	{(FUL)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
+	{(FUL)GFP_USER,		"GFP_USER"},		\
+	{(FUL)GFP_KERNEL_ACCOUNT,	"GFP_KERNEL_ACCOUNT"},	\
+	{(FUL)GFP_KERNEL,		"GFP_KERNEL"},		\
+	{(FUL)GFP_NOFS,		"GFP_NOFS"},		\
+	{(FUL)GFP_ATOMIC,		"GFP_ATOMIC"},		\
+	{(FUL)GFP_NOIO,		"GFP_NOIO"},		\
+	{(FUL)GFP_NOWAIT,		"GFP_NOWAIT"},		\
+	{(FUL)GFP_DMA,		"GFP_DMA"},		\
+	{(FUL)__GFP_HIGHMEM,		"__GFP_HIGHMEM"},	\
+	{(FUL)GFP_DMA32,		"GFP_DMA32"},		\
+	{(FUL)__GFP_HIGH,		"__GFP_HIGH"},		\
+	{(FUL)__GFP_ATOMIC,		"__GFP_ATOMIC"},	\
+	{(FUL)__GFP_IO,		"__GFP_IO"},		\
+	{(FUL)__GFP_FS,		"__GFP_FS"},		\
+	{(FUL)__GFP_NOWARN,		"__GFP_NOWARN"},	\
+	{(FUL)__GFP_RETRY_MAYFAIL,	"__GFP_RETRY_MAYFAIL"},	\
+	{(FUL)__GFP_NOFAIL,		"__GFP_NOFAIL"},	\
+	{(FUL)__GFP_NORETRY,		"__GFP_NORETRY"},	\
+	{(FUL)__GFP_COMP,		"__GFP_COMP"},		\
+	{(FUL)__GFP_ZERO,		"__GFP_ZERO"},		\
+	{(FUL)__GFP_NOMEMALLOC,	"__GFP_NOMEMALLOC"},	\
+	{(FUL)__GFP_MEMALLOC,		"__GFP_MEMALLOC"},	\
+	{(FUL)__GFP_HARDWALL,		"__GFP_HARDWALL"},	\
+	{(FUL)__GFP_THISNODE,		"__GFP_THISNODE"},	\
+	{(FUL)__GFP_RECLAIMABLE,	"__GFP_RECLAIMABLE"},	\
+	{(FUL)__GFP_MOVABLE,		"__GFP_MOVABLE"},	\
+	{(FUL)__GFP_ACCOUNT,		"__GFP_ACCOUNT"},	\
+	{(FUL)__GFP_WRITE,		"__GFP_WRITE"},		\
+	{(FUL)__GFP_RECLAIM,		"__GFP_RECLAIM"},	\
+	{(FUL)__GFP_DIRECT_RECLAIM,	"__GFP_DIRECT_RECLAIM"},\
+	{(FUL)__GFP_KSWAPD_RECLAIM,	"__GFP_KSWAPD_RECLAIM"},\
+	{(FUL)__GFP_ZEROTAGS,		"__GFP_ZEROTAGS"}	\
 
 #ifdef CONFIG_KASAN_HW_TAGS
 #define __def_gfpflag_names_kasan ,					       \
-	{(__force unsigned long)__GFP_SKIP_ZERO,	   "__GFP_SKIP_ZERO"},	       \
-	{(__force unsigned long)__GFP_SKIP_KASAN_POISON,   "__GFP_SKIP_KASAN_POISON"}, \
-	{(__force unsigned long)__GFP_SKIP_KASAN_UNPOISON, "__GFP_SKIP_KASAN_UNPOISON"}
+	{(FUL)__GFP_SKIP_ZERO,	   "__GFP_SKIP_ZERO"},	       \
+	{(FUL)__GFP_SKIP_KASAN_POISON,   "__GFP_SKIP_KASAN_POISON"}, \
+	{(FUL)__GFP_SKIP_KASAN_UNPOISON, "__GFP_SKIP_KASAN_UNPOISON"}
 #else
 #define __def_gfpflag_names_kasan
 #endif
 
+#undef FUL
+
 #define show_gfp_flags(flags)						\
 	(flags) ? __print_flags(flags, "|",				\
 	__def_gfpflag_names __def_gfpflag_names_kasan			\
_



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm] tracing: incorrect gfp_t conversion
       [not found]         ` <8b1cfefa-da7d-3376-cf04-1ff77dab8170@openvz.org>
@ 2022-05-07 22:48           ` Andrew Morton
  2022-05-07 23:00             ` Andrew Morton
  2022-05-08 20:51             ` Joe Perches
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2022-05-07 22:48 UTC (permalink / raw)
  To: Vasily Averin; +Cc: Steven Rostedt, Ingo Molnar, kernel, linux-kernel, linux-mm

On Sun, 8 May 2022 01:28:58 +0300 Vasily Averin <vvs@openvz.org> wrote:

> On 5/7/22 22:37, Andrew Morton wrote:
> > On Sat, 7 May 2022 22:02:05 +0300 Vasily Averin <vvs@openvz.org> wrote:
> >> +	{(__force unsigned long)GFP_KERNEL,		"GFP_KERNEL"},		\
> >> +	{(__force unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
> > 
> > This got all repetitive, line-wrappy and ugly :(
> > 
> > What do we think of something silly like this?
> 
> > --- a/include/trace/events/mmflags.h~tracing-incorrect-gfp_t-conversion-fix
> > +++ a/include/trace/events/mmflags.h
> > @@ -13,53 +13,57 @@
> >   * Thus most bits set go first.
> >   */
> >  
> > +#define FUL __force unsigned long
> > +
> >  #define __def_gfpflag_names						\
> > -	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> > -	{(__force unsigned long)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
> ...
> > +	{(FUL)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> > +	{(FUL)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
> 
> 
> I think it's a good idea, and I regret it was your idea and not mine.

heh

> Should I resend my patch with these changes or would you prefer 
> to keep your patch as a separate one?

I did the below.  I'll squash them together later.


--- a/include/trace/events/mmflags.h~tracing-incorrect-gfp_t-conversion-fix
+++ a/include/trace/events/mmflags.h
@@ -13,53 +13,57 @@
  * Thus most bits set go first.
  */
 
+#define FUL __force unsigned long
+
 #define __def_gfpflag_names						\
-	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
-	{(__force unsigned long)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
-	{(__force unsigned long)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"},\
-	{(__force unsigned long)GFP_HIGHUSER,		"GFP_HIGHUSER"},	\
-	{(__force unsigned long)GFP_USER,		"GFP_USER"},		\
-	{(__force unsigned long)GFP_KERNEL_ACCOUNT,	"GFP_KERNEL_ACCOUNT"},	\
-	{(__force unsigned long)GFP_KERNEL,		"GFP_KERNEL"},		\
-	{(__force unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
-	{(__force unsigned long)GFP_ATOMIC,		"GFP_ATOMIC"},		\
-	{(__force unsigned long)GFP_NOIO,		"GFP_NOIO"},		\
-	{(__force unsigned long)GFP_NOWAIT,		"GFP_NOWAIT"},		\
-	{(__force unsigned long)GFP_DMA,		"GFP_DMA"},		\
-	{(__force unsigned long)__GFP_HIGHMEM,		"__GFP_HIGHMEM"},	\
-	{(__force unsigned long)GFP_DMA32,		"GFP_DMA32"},		\
-	{(__force unsigned long)__GFP_HIGH,		"__GFP_HIGH"},		\
-	{(__force unsigned long)__GFP_ATOMIC,		"__GFP_ATOMIC"},	\
-	{(__force unsigned long)__GFP_IO,		"__GFP_IO"},		\
-	{(__force unsigned long)__GFP_FS,		"__GFP_FS"},		\
-	{(__force unsigned long)__GFP_NOWARN,		"__GFP_NOWARN"},	\
-	{(__force unsigned long)__GFP_RETRY_MAYFAIL,	"__GFP_RETRY_MAYFAIL"},	\
-	{(__force unsigned long)__GFP_NOFAIL,		"__GFP_NOFAIL"},	\
-	{(__force unsigned long)__GFP_NORETRY,		"__GFP_NORETRY"},	\
-	{(__force unsigned long)__GFP_COMP,		"__GFP_COMP"},		\
-	{(__force unsigned long)__GFP_ZERO,		"__GFP_ZERO"},		\
-	{(__force unsigned long)__GFP_NOMEMALLOC,	"__GFP_NOMEMALLOC"},	\
-	{(__force unsigned long)__GFP_MEMALLOC,		"__GFP_MEMALLOC"},	\
-	{(__force unsigned long)__GFP_HARDWALL,		"__GFP_HARDWALL"},	\
-	{(__force unsigned long)__GFP_THISNODE,		"__GFP_THISNODE"},	\
-	{(__force unsigned long)__GFP_RECLAIMABLE,	"__GFP_RECLAIMABLE"},	\
-	{(__force unsigned long)__GFP_MOVABLE,		"__GFP_MOVABLE"},	\
-	{(__force unsigned long)__GFP_ACCOUNT,		"__GFP_ACCOUNT"},	\
-	{(__force unsigned long)__GFP_WRITE,		"__GFP_WRITE"},		\
-	{(__force unsigned long)__GFP_RECLAIM,		"__GFP_RECLAIM"},	\
-	{(__force unsigned long)__GFP_DIRECT_RECLAIM,	"__GFP_DIRECT_RECLAIM"},\
-	{(__force unsigned long)__GFP_KSWAPD_RECLAIM,	"__GFP_KSWAPD_RECLAIM"},\
-	{(__force unsigned long)__GFP_ZEROTAGS,		"__GFP_ZEROTAGS"}	\
+	{(FUL)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},		\
+	{(FUL)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, 	\
+	{(FUL)GFP_HIGHUSER_MOVABLE,	"GFP_HIGHUSER_MOVABLE"},	\
+	{(FUL)GFP_HIGHUSER,		"GFP_HIGHUSER"},		\
+	{(FUL)GFP_USER,			"GFP_USER"},			\
+	{(FUL)GFP_KERNEL_ACCOUNT,	"GFP_KERNEL_ACCOUNT"},		\
+	{(FUL)GFP_KERNEL,		"GFP_KERNEL"},			\
+	{(FUL)GFP_NOFS,			"GFP_NOFS"},			\
+	{(FUL)GFP_ATOMIC,		"GFP_ATOMIC"},			\
+	{(FUL)GFP_NOIO,			"GFP_NOIO"},			\
+	{(FUL)GFP_NOWAIT,		"GFP_NOWAIT"},			\
+	{(FUL)GFP_DMA,			"GFP_DMA"},			\
+	{(FUL)__GFP_HIGHMEM,		"__GFP_HIGHMEM"},		\
+	{(FUL)GFP_DMA32,		"GFP_DMA32"},			\
+	{(FUL)__GFP_HIGH,		"__GFP_HIGH"},			\
+	{(FUL)__GFP_ATOMIC,		"__GFP_ATOMIC"},		\
+	{(FUL)__GFP_IO,			"__GFP_IO"},			\
+	{(FUL)__GFP_FS,			"__GFP_FS"},			\
+	{(FUL)__GFP_NOWARN,		"__GFP_NOWARN"},		\
+	{(FUL)__GFP_RETRY_MAYFAIL,	"__GFP_RETRY_MAYFAIL"},		\
+	{(FUL)__GFP_NOFAIL,		"__GFP_NOFAIL"},		\
+	{(FUL)__GFP_NORETRY,		"__GFP_NORETRY"},		\
+	{(FUL)__GFP_COMP,		"__GFP_COMP"},			\
+	{(FUL)__GFP_ZERO,		"__GFP_ZERO"},			\
+	{(FUL)__GFP_NOMEMALLOC,		"__GFP_NOMEMALLOC"},		\
+	{(FUL)__GFP_MEMALLOC,		"__GFP_MEMALLOC"},		\
+	{(FUL)__GFP_HARDWALL,		"__GFP_HARDWALL"},		\
+	{(FUL)__GFP_THISNODE,		"__GFP_THISNODE"},		\
+	{(FUL)__GFP_RECLAIMABLE,	"__GFP_RECLAIMABLE"},		\
+	{(FUL)__GFP_MOVABLE,		"__GFP_MOVABLE"},		\
+	{(FUL)__GFP_ACCOUNT,		"__GFP_ACCOUNT"},		\
+	{(FUL)__GFP_WRITE,		"__GFP_WRITE"},			\
+	{(FUL)__GFP_RECLAIM,		"__GFP_RECLAIM"},		\
+	{(FUL)__GFP_DIRECT_RECLAIM,	"__GFP_DIRECT_RECLAIM"},	\
+	{(FUL)__GFP_KSWAPD_RECLAIM,	"__GFP_KSWAPD_RECLAIM"},	\
+	{(FUL)__GFP_ZEROTAGS,		"__GFP_ZEROTAGS"}		\
 
 #ifdef CONFIG_KASAN_HW_TAGS
-#define __def_gfpflag_names_kasan ,					       \
-	{(__force unsigned long)__GFP_SKIP_ZERO,	   "__GFP_SKIP_ZERO"},	       \
-	{(__force unsigned long)__GFP_SKIP_KASAN_POISON,   "__GFP_SKIP_KASAN_POISON"}, \
-	{(__force unsigned long)__GFP_SKIP_KASAN_UNPOISON, "__GFP_SKIP_KASAN_UNPOISON"}
+#define __def_gfpflag_names_kasan ,					\
+	{(FUL)__GFP_SKIP_ZERO,		"__GFP_SKIP_ZERO"},		\
+	{(FUL)__GFP_SKIP_KASAN_POISON,	"__GFP_SKIP_KASAN_POISON"},	\
+	{(FUL)__GFP_SKIP_KASAN_UNPOISON,"__GFP_SKIP_KASAN_UNPOISON"}
 #else
 #define __def_gfpflag_names_kasan
 #endif
 
+#undef FUL
+
 #define show_gfp_flags(flags)						\
 	(flags) ? __print_flags(flags, "|",				\
 	__def_gfpflag_names __def_gfpflag_names_kasan			\
_



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm] tracing: incorrect gfp_t conversion
  2022-05-07 22:48           ` Andrew Morton
@ 2022-05-07 23:00             ` Andrew Morton
  2022-05-08 20:37               ` Matthew Wilcox
  2022-05-08 20:51             ` Joe Perches
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2022-05-07 23:00 UTC (permalink / raw)
  To: Vasily Averin, Steven Rostedt, Ingo Molnar, kernel, linux-kernel,
	linux-mm

On Sat, 7 May 2022 15:48:35 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> I did the below.
> 

Silly me, doesn't work.

> 
> --- a/include/trace/events/mmflags.h~tracing-incorrect-gfp_t-conversion-fix
> +++ a/include/trace/events/mmflags.h
> @@ -13,53 +13,57 @@
>   * Thus most bits set go first.
>   */
>  
> +#define FUL __force unsigned long
> +
>  #define __def_gfpflag_names						\
> -	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\

Can't expand FUL here within the macro definition.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm] tracing: incorrect gfp_t conversion
  2022-05-07 23:00             ` Andrew Morton
@ 2022-05-08 20:37               ` Matthew Wilcox
  0 siblings, 0 replies; 9+ messages in thread
From: Matthew Wilcox @ 2022-05-08 20:37 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Vasily Averin, Steven Rostedt, Ingo Molnar, kernel, linux-kernel,
	linux-mm

On Sat, May 07, 2022 at 04:00:10PM -0700, Andrew Morton wrote:
> On Sat, 7 May 2022 15:48:35 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > I did the below.
> > 
> 
> Silly me, doesn't work.
> 
> > 
> > --- a/include/trace/events/mmflags.h~tracing-incorrect-gfp_t-conversion-fix
> > +++ a/include/trace/events/mmflags.h
> > @@ -13,53 +13,57 @@
> >   * Thus most bits set go first.
> >   */
> >  
> > +#define FUL __force unsigned long
> > +
> >  #define __def_gfpflag_names						\
> > -	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> 
> Can't expand FUL here within the macro definition.

Can we do something even better?

#define GFP_NAME(flag) { (__force unsigned long)flag, #flag },

... with one or more layers of indirection to satisfy the arcane
rules of C macros?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm] tracing: incorrect gfp_t conversion
  2022-05-07 22:48           ` Andrew Morton
  2022-05-07 23:00             ` Andrew Morton
@ 2022-05-08 20:51             ` Joe Perches
       [not found]               ` <8b9ba8ce-7376-2ef2-95f5-30e53cb46914@openvz.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Joe Perches @ 2022-05-08 20:51 UTC (permalink / raw)
  To: Andrew Morton, Vasily Averin
  Cc: Steven Rostedt, Ingo Molnar, kernel, linux-kernel, linux-mm

On Sat, 2022-05-07 at 15:48 -0700, Andrew Morton wrote:
> On Sun, 8 May 2022 01:28:58 +0300 Vasily Averin <vvs@openvz.org> wrote:
> 
> > On 5/7/22 22:37, Andrew Morton wrote:
> > > On Sat, 7 May 2022 22:02:05 +0300 Vasily Averin <vvs@openvz.org> wrote:
> > > > +	{(__force unsigned long)GFP_KERNEL,		"GFP_KERNEL"},		\
> > > > +	{(__force unsigned long)GFP_NOFS,		"GFP_NOFS"},		\
> > > 
> > > This got all repetitive, line-wrappy and ugly :(
> > > 
> > > What do we think of something silly like this?
> > 
> > > --- a/include/trace/events/mmflags.h~tracing-incorrect-gfp_t-conversion-fix
> > > +++ a/include/trace/events/mmflags.h
> > > @@ -13,53 +13,57 @@
> > >   * Thus most bits set go first.
> > >   */
> > >  
> > > +#define FUL __force unsigned long
> > > +
> > >  #define __def_gfpflag_names						\
> > > -	{(__force unsigned long)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> > > -	{(__force unsigned long)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
> > ...
> > > +	{(FUL)GFP_TRANSHUGE,		"GFP_TRANSHUGE"},	\
> > > +	{(FUL)GFP_TRANSHUGE_LIGHT,	"GFP_TRANSHUGE_LIGHT"}, \
> > 
> > 
> > I think it's a good idea, and I regret it was your idea and not mine.
> 
> heh
> 
> > Should I resend my patch with these changes or would you prefer 
> > to keep your patch as a separate one?
> 
> I did the below.  I'll squash them together later.

Very repetitive indeed.

Why not use another stringifying macro?

Maybe something like:

#define gfpflag_string(GFP)	\
	{(__force unsigned long)GFP, #GFP)}

#define __def_gfpflag_names			\
	gfp_flag_string(GFP_TRANSHUGE),		\
	etc...




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] percpu: improve percpu_alloc_percpu event trace
       [not found]   ` <e1c09bbb-2c58-a986-c704-1db538da905a@openvz.org>
       [not found]     ` <331d88fe-f4f7-657c-02a2-d977f15fbff6@openvz.org>
@ 2022-05-09 21:06     ` Steven Rostedt
  1 sibling, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2022-05-09 21:06 UTC (permalink / raw)
  To: Vasily Averin
  Cc: kernel test robot, Ingo Molnar, kbuild-all, Shakeel Butt, kernel,
	linux-kernel, Roman Gushchin, Vlastimil Babka, Michal Hocko,
	cgroups, Andrew Morton, Linux Memory Management List,
	Dennis Zhou, Tejun Heo, Christoph Lameter

On Sat, 7 May 2022 17:51:16 +0300
Vasily Averin <vvs@openvz.org> wrote:

> The same messages are generated for any other gfp_t argument in trace events.
> As far as I understand it is not a bug per se,
> but trace macros lacks __force attribute in 'gfp_t'-> 'unsigned long' casts.
> The same thing happens with mode_t and with some other places using __print_flags()
> for __bitwise marked types.

I'm curious as to where the gfp_t to unsigned long is happening in the
macros?

-- Steve


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm v2] tracing: incorrect gfp_t conversion
       [not found]               ` <8b9ba8ce-7376-2ef2-95f5-30e53cb46914@openvz.org>
@ 2022-05-15 22:09                 ` Steven Rostedt
  2022-05-16 20:55                   ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2022-05-15 22:09 UTC (permalink / raw)
  To: Vasily Averin
  Cc: Ingo Molnar, Andrew Morton, Matthew Wilcox, Joe Perches, kernel,
	linux-kernel, linux-mm

On Wed, 11 May 2022 10:20:39 +0300
Vasily Averin <vvs@openvz.org> wrote:

> Fixes the following sparse warnings:
> 
> include/trace/events/*: sparse: cast to restricted gfp_t
> include/trace/events/*: sparse: restricted gfp_t degrades to integer
> 
> gfp_t type is bitwise and requires __force attributes for any casts.
> 
> Signed-off-by:	Vasily Averin <vvs@openvz.org>
> ---
> v2: 1) re-based to 5.18-rc6
>     2) re-defined __def_gfpflag_names array according to
> 	akpm@, willy@ and Joe Perches recommendations
> ---
>  include/linux/gfp.h               |  2 +-
>  include/trace/events/btrfs.h      |  4 +-
>  include/trace/events/compaction.h |  4 +-
>  include/trace/events/kmem.h       | 12 ++---
>  include/trace/events/mmflags.h    | 84 ++++++++++++++++---------------
>  include/trace/events/vmscan.h     | 16 +++---
>  mm/compaction.c                   |  2 +-
>  7 files changed, 63 insertions(+), 61 deletions(-)

From the tracing POV:

Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH mm v2] tracing: incorrect gfp_t conversion
  2022-05-15 22:09                 ` [PATCH mm v2] " Steven Rostedt
@ 2022-05-16 20:55                   ` Andrew Morton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2022-05-16 20:55 UTC (permalink / raw)
  To: Vasily Averin; +Cc: kernel, linux-kernel, linux-mm, Steven Rostedt

On Wed, 11 May 2022 10:20:39 +0300 Vasily Averin <vvs@openvz.org> wrote:

> Fixes the following sparse warnings:
> 
> include/trace/events/*: sparse: cast to restricted gfp_t
> include/trace/events/*: sparse: restricted gfp_t degrades to integer
> 
> gfp_t type is bitwise and requires __force attributes for any casts.

I already moved the previous version into mm-stable.  Would prefer not
to have to rebase it.

> v2: 1) re-based to 5.18-rc6
>     2) re-defined __def_gfpflag_names array according to
> 	akpm@, willy@ and Joe Perches recommendations

Please redo this against the mm-stable or mm-unstable branches against
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm, or against your
own previous version.

The new patch will simply switch to the gfpflag_string() trick, so it will
not be a "fix", so please let's position it as a "cleanup".



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-05-16 20:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <8d627f02-183f-c4e7-7c15-77b2b438536b@openvz.org>
2022-05-06 20:38 ` [PATCH] percpu: improve percpu_alloc_percpu event trace kernel test robot
     [not found]   ` <e1c09bbb-2c58-a986-c704-1db538da905a@openvz.org>
     [not found]     ` <331d88fe-f4f7-657c-02a2-d977f15fbff6@openvz.org>
2022-05-07 19:37       ` [PATCH mm] tracing: incorrect gfp_t conversion Andrew Morton
     [not found]         ` <8b1cfefa-da7d-3376-cf04-1ff77dab8170@openvz.org>
2022-05-07 22:48           ` Andrew Morton
2022-05-07 23:00             ` Andrew Morton
2022-05-08 20:37               ` Matthew Wilcox
2022-05-08 20:51             ` Joe Perches
     [not found]               ` <8b9ba8ce-7376-2ef2-95f5-30e53cb46914@openvz.org>
2022-05-15 22:09                 ` [PATCH mm v2] " Steven Rostedt
2022-05-16 20:55                   ` Andrew Morton
2022-05-09 21:06     ` [PATCH] percpu: improve percpu_alloc_percpu event trace Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox