| Message ID | 20221212130352.606d7ce2@gandalf.local.home (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tracing: Remove pointer (asterisk) from cpumask_t field | expand |
On Mon, 12 Dec 2022 13:03:52 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > To differentiate between long arrays and cpumasks, the __cpumask() field > was created. Part of the TRACE_EVENT() macros test if the type is signed > or not by using the is_signed_type() macro. The __cpumask() field used the > __dynamic_array() helper but because cpumask_t is a structure, it could > not be used in the is_signed_type() macro as that would fail to build, so > instead it passed in the pointer to cpumask_t. > > Unfortunately, that creates in the format file: > > field:__data_loc cpumask_t *[] mask; offset:36; size:4; signed:0; > > Which looks like an array of pointers to cpumask_t and not a cpumask_t > type, which is misleading to user space parsers. > > Instead, create another helper called __dynamic_array_sign() that can be > used directly by the __cpumaks() field macro where it passes in an > unsigned value to avoid using the is_signed_type() and have the > __dynamic_array() macro call it by passing it the is_signed_type() on the > type. What about renaming it as __dynamic_array_struct(), because the root issue is that we can not pass the data structure to is_signed_tyep() macro? Thank you, > > This now produces: > > field:__data_loc cpumask_t[] mask; offset:36; size:4; signed:0; > > Which is the correct type of the field. > > Link: https://lore.kernel.org/lkml/6dda5e1d-9416-b55e-88f3-31d148bc925f@arm.com/ > > Fixes: 8230f27b1ccc ("tracing: Add __cpumask to denote a trace event field that is a cpumask_t") > Reported-by: Douglas Raillard <douglas.raillard@arm.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > include/trace/stages/stage4_event_fields.h | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/include/trace/stages/stage4_event_fields.h b/include/trace/stages/stage4_event_fields.h > index f2990d22313c..9f0049c309f0 100644 > --- a/include/trace/stages/stage4_event_fields.h > +++ b/include/trace/stages/stage4_event_fields.h > @@ -28,11 +28,15 @@ > .size = sizeof(_type[_len]), .align = ALIGN_STRUCTFIELD(_type), \ > .is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER }, > > -#undef __dynamic_array > -#define __dynamic_array(_type, _item, _len) { \ > +#undef __dynamic_array_sign > +#define __dynamic_array_sign(_type, _item, _len, _sign) { \ > .type = "__data_loc " #_type "[]", .name = #_item, \ > .size = 4, .align = 4, \ > - .is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER }, > + .is_signed = _sign, .filter_type = FILTER_OTHER }, > + > +#undef __dynamic_array > +#define __dynamic_array(_type, _item, _len) \ > + __dynamic_array_sign(_type, item, len, is_signed_type(_type)) > > #undef __string > #define __string(item, src) __dynamic_array(char, item, -1) > @@ -47,7 +51,7 @@ > #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1) > > #undef __cpumask > -#define __cpumask(item) __dynamic_array(cpumask_t *, item, -1) > +#define __cpumask(item) __dynamic_array_sign(cpumask_t, item, -1, 0) > > #undef __sockaddr > #define __sockaddr(field, len) __dynamic_array(u8, field, len) > -- > 2.35.1 >
On Tue, 13 Dec 2022 09:04:11 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > What about renaming it as __dynamic_array_struct(), because the root issue > is that we can not pass the data structure to is_signed_tyep() macro? Actually, I updated this patch where __cpumask() will be its own macro. Will post it soon. -- Steve
diff --git a/include/trace/stages/stage4_event_fields.h b/include/trace/stages/stage4_event_fields.h index f2990d22313c..9f0049c309f0 100644 --- a/include/trace/stages/stage4_event_fields.h +++ b/include/trace/stages/stage4_event_fields.h @@ -28,11 +28,15 @@ .size = sizeof(_type[_len]), .align = ALIGN_STRUCTFIELD(_type), \ .is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER }, -#undef __dynamic_array -#define __dynamic_array(_type, _item, _len) { \ +#undef __dynamic_array_sign +#define __dynamic_array_sign(_type, _item, _len, _sign) { \ .type = "__data_loc " #_type "[]", .name = #_item, \ .size = 4, .align = 4, \ - .is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER }, + .is_signed = _sign, .filter_type = FILTER_OTHER }, + +#undef __dynamic_array +#define __dynamic_array(_type, _item, _len) \ + __dynamic_array_sign(_type, item, len, is_signed_type(_type)) #undef __string #define __string(item, src) __dynamic_array(char, item, -1) @@ -47,7 +51,7 @@ #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1) #undef __cpumask -#define __cpumask(item) __dynamic_array(cpumask_t *, item, -1) +#define __cpumask(item) __dynamic_array_sign(cpumask_t, item, -1, 0) #undef __sockaddr #define __sockaddr(field, len) __dynamic_array(u8, field, len)