Re: [PATCH v2 6/9] mm, debug: introduce dump_gfpflag_names() for symbolic printing of gfp_flags

yalin wang
2015-11-27T03:40:51Z

> On Nov 25, 2015, at 18:28, Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 11/25/2015 09:16 AM, Joonsoo Kim wrote:
>> On Tue, Nov 24, 2015 at 01:36:18PM +0100, Vlastimil Babka wrote:
>>> --- a/include/trace/events/gfpflags.h
>>> +++ b/include/trace/events/gfpflags.h
>>> @@ -8,8 +8,8 @@
>>> *
>>> * Thus most bits set go first.
>>> */
>>> -#define show_gfp_flags(flags) \
>>> - (flags) ? __print_flags(flags, "|", \
>>> +
>>> +#define __def_gfpflag_names \
>>> {(unsigned long)GFP_TRANSHUGE, "GFP_TRANSHUGE"}, \
>>> {(unsigned long)GFP_HIGHUSER_MOVABLE, "GFP_HIGHUSER_MOVABLE"}, \
>>> {(unsigned long)GFP_HIGHUSER, "GFP_HIGHUSER"}, \
>>> @@ -19,9 +19,13 @@
>>> {(unsigned long)GFP_NOFS, "GFP_NOFS"}, \
>>> {(unsigned long)GFP_ATOMIC, "GFP_ATOMIC"}, \
>>> {(unsigned long)GFP_NOIO, "GFP_NOIO"}, \
>>> + {(unsigned long)GFP_NOWAIT, "GFP_NOWAIT"}, \
>>> + {(unsigned long)__GFP_DMA, "GFP_DMA"}, \
>>> + {(unsigned long)__GFP_DMA32, "GFP_DMA32"}, \
>>> {(unsigned long)__GFP_HIGH, "GFP_HIGH"}, \
>>> {(unsigned long)__GFP_ATOMIC, "GFP_ATOMIC"}, \
>>> {(unsigned long)__GFP_IO, "GFP_IO"}, \
>>> + {(unsigned long)__GFP_FS, "GFP_FS"}, \
>>> {(unsigned long)__GFP_COLD, "GFP_COLD"}, \
>>> {(unsigned long)__GFP_NOWARN, "GFP_NOWARN"}, \
>>> {(unsigned long)__GFP_REPEAT, "GFP_REPEAT"}, \
>>> @@ -36,8 +40,12 @@
>>> {(unsigned long)__GFP_RECLAIMABLE, "GFP_RECLAIMABLE"}, \
>>> {(unsigned long)__GFP_MOVABLE, "GFP_MOVABLE"}, \
>>> {(unsigned long)__GFP_NOTRACK, "GFP_NOTRACK"}, \
>>> + {(unsigned long)__GFP_WRITE, "GFP_WRITE"}, \
>>> {(unsigned long)__GFP_DIRECT_RECLAIM, "GFP_DIRECT_RECLAIM"}, \
>>> {(unsigned long)__GFP_KSWAPD_RECLAIM, "GFP_KSWAPD_RECLAIM"}, \
>>> {(unsigned long)__GFP_OTHER_NODE, "GFP_OTHER_NODE"} \
>>> - ) : "GFP_NOWAIT"
>>>
>>> +#define show_gfp_flags(flags) \
>>> + (flags) ? __print_flags(flags, "|", \
>>> + __def_gfpflag_names \
>>> + ) : "none"
>>
>> How about moving this to gfp.h or something?
>> Now, we use it in out of tracepoints so there is no need to keep it
>> in include/trace/events/xxx.
>
> Hm I didn't want to pollute such widely included header with such defines. And
> show_gfp_flags shouldn't be there definitely as it depends on __print_flags.
> What do others think?
how about add this into standard printk() format ?
like cpu mask print in printk use %*pb[l] ,
it define a macro cpumask_pr_args to print cpumask .

we can also define a new format like %pG means print flag ,
then it will be useful for other code to use , like dump vma / mm flags ..

Thanks