Messages in this thread | | | From | William Kucharski <> | Subject | Re: [PATCH] cgroup: Trace event cgroup id fields should be u64 | Date | Wed, 1 Dec 2021 15:59:21 +0000 |
| |
Thanks, V2 will be out shortly.
> On Dec 1, 2021, at 8:45 AM, Steven Rostedt <rostedt@goodmis.org> wrote: > > On Wed, 1 Dec 2021 07:40:43 -0700 > William Kucharski <william.kucharski@oracle.com> wrote: > >> Various trace event fields that store cgroup IDs were declared >> as ints, but cgroup_id(() returns a u64 and the conversion was >> not intended. >> >> Also remove extraneous spaces in fields that are no longer proper C style. > > Um, actually, please do not do this! > >> >> Fixes: 743210386c03 ("cgroup: use cgrp->kn->id as the cgroup ID") >> Signed-off-by: William Kucharski <william.kucharski@oracle.com> >> --- >> include/trace/events/cgroup.h | 42 +++++++++++++++++------------------ >> 1 file changed, 21 insertions(+), 21 deletions(-) >> >> diff --git a/include/trace/events/cgroup.h b/include/trace/events/cgroup.h >> index 7f42a3de59e6..a85b01aa1fff 100644 >> --- a/include/trace/events/cgroup.h >> +++ b/include/trace/events/cgroup.h >> @@ -15,9 +15,9 @@ DECLARE_EVENT_CLASS(cgroup_root, >> TP_ARGS(root), >> >> TP_STRUCT__entry( >> - __field( int, root ) >> - __field( u16, ss_mask ) >> - __string( name, root->name ) >> + __field(int, root) >> + __field(u16, ss_mask) >> + __string(name, root->name) > > The spaces are there for a reason. These are special macros and not C code. > It's defining a structure, and the spacing is to imitate just that! > > struct __entry { > int root; > u16 ss_mask; > name root->name; > }; > > > See the difference. You just made it less readable than the original. > > So leave it alone. > > NACK on the style changes. > > -- Steve > > >> ), >> >> TP_fast_assign( >> @@ -58,10 +58,10 @@ DECLARE_EVENT_CLASS(cgroup, >> TP_ARGS(cgrp, path), >> >> TP_STRUCT__entry( >> - __field( int, root ) >> - __field( int, id ) >> - __field( int, level ) >> - __string( path, path ) >> + __field(int, root) >> + __field(u64, id) >> + __field(int, level) >> + __string(path, path) >> ), >> >> TP_fast_assign( >> @@ -71,7 +71,7 @@ DECLARE_EVENT_CLASS(cgroup, >> __assign_str(path, path); >> ), >> >> - TP_printk("root=%d id=%d level=%d path=%s", >> + TP_printk("root=%d id=%llu level=%d path=%s", >> __entry->root, __entry->id, __entry->level, __get_str(path)) >> ); >> >> @@ -125,12 +125,12 @@ DECLARE_EVENT_CLASS(cgroup_migrate, >> TP_ARGS(dst_cgrp, path, task, threadgroup), >> >> TP_STRUCT__entry( >> - __field( int, dst_root ) >> - __field( int, dst_id ) >> - __field( int, dst_level ) >> - __field( int, pid ) >> - __string( dst_path, path ) >> - __string( comm, task->comm ) >> + __field(int, dst_root) >> + __field(u64, dst_id) >> + __field(int, dst_level) >> + __field(int, pid) >> + __string(dst_path, path) >> + __string(comm, task->comm) >> ), >> >> TP_fast_assign( >> @@ -142,7 +142,7 @@ DECLARE_EVENT_CLASS(cgroup_migrate, >> __assign_str(comm, task->comm); >> ), >> >> - TP_printk("dst_root=%d dst_id=%d dst_level=%d dst_path=%s pid=%d comm=%s", >> + TP_printk("dst_root=%d dst_id=%llu dst_level=%d dst_path=%s pid=%d comm=%s", >> __entry->dst_root, __entry->dst_id, __entry->dst_level, >> __get_str(dst_path), __entry->pid, __get_str(comm)) >> ); >> @@ -170,11 +170,11 @@ DECLARE_EVENT_CLASS(cgroup_event, >> TP_ARGS(cgrp, path, val), >> >> TP_STRUCT__entry( >> - __field( int, root ) >> - __field( int, id ) >> - __field( int, level ) >> - __string( path, path ) >> - __field( int, val ) >> + __field(int, root) >> + __field(u64, id) >> + __field(int, level) >> + __string(path, path) >> + __field(int, val) >> ), >> >> TP_fast_assign( >> @@ -185,7 +185,7 @@ DECLARE_EVENT_CLASS(cgroup_event, >> __entry->val = val; >> ), >> >> - TP_printk("root=%d id=%d level=%d path=%s val=%d", >> + TP_printk("root=%d id=%llu level=%d path=%s val=%d", >> __entry->root, __entry->id, __entry->level, __get_str(path), >> __entry->val) >> ); >
| |