Re: [PATCH 1/2] mm, printk: introduce new format string for flags

[Vlastimil Babka]

On 12/03/2015 01:37 PM, Rasmus Villemoes wrote:
> On Wed, Dec 02 2015, Vlastimil Babka <vbabka@xxxxxxx> wrote:
>> --- a/include/linux/mmdebug.h
>> +++ b/include/linux/mmdebug.h
>> @@ -7,6 +7,9 @@
>>  struct page;
>>  struct vm_area_struct;
>>  struct mm_struct;
>> +struct trace_print_flags; // can't include trace_events.h here
>> +
>> +extern const struct trace_print_flags *pageflag_names;
>>
>>  extern void dump_page(struct page *page, const char *reason);
>>  extern void dump_page_badflags(struct page *page, const char *reason,
>> diff --git a/mm/debug.c b/mm/debug.c
>> index a092111920e7..1cbc60544b87 100644
>> --- a/mm/debug.c
>> +++ b/mm/debug.c
>> @@ -23,7 +23,7 @@ char *migrate_reason_names[MR_TYPES] = {
>>  	"cma",
>>  };
>>
>> -static const struct trace_print_flags pageflag_names[] = {
>> +const struct trace_print_flags __pageflag_names[] = {
>>  	{1UL << PG_locked,		"locked"	},
>>  	{1UL << PG_error,		"error"		},
>>  	{1UL << PG_referenced,		"referenced"	},
>> @@ -59,6 +59,8 @@ static const struct trace_print_flags pageflag_names[] = {
>>  #endif
>>  };
>>
>> +const struct trace_print_flags *pageflag_names = &__pageflag_names[0];
> 
> Ugh. I think it would be better if either the definition of struct
> trace_print_flags is moved somewhere where everybody can see it or to
> make our own identical type definition. For now I'd go with the latter,
> also since this doesn't really have anything to do with the tracing
> subsystem. Then just declare the array in the header
> 
> extern const struct print_flags pageflag_names[];

Ugh so yesterday I copy/pasted the definition and still got an error, which I
probably didn't read closely enough. I assumed that if it needs the full
definition of "struct trace_print_flags" here to know the size, it would also
need to know the lenght of the array as well.

But now it works. Well, copy/pasting the definition fails as long as both
headers are included and it's redefining the struct (even though it's the same
thing). But looks like I can move it from trace_events.h to tracepoint.h and it
won't blow off (knock knock).

I suck at C.

> (If you do the extra indirection thing, __pageflag_names could still be
> static, and it would be best to declare the pointer itself const as
> well, but I'd rather we don't go that way.)
> 
> Rasmus
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/