lkml.org 
[lkml]   [2021]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[RFC][PATCH] tracing: Define "fake" struct trace_pid_list

[ Note, this is on top of my tree in ftrace/core, but wanted to ask if
this is the proper "fix". I moved the struct trace_pid_list into a
separate file to have more control over it, and only declare the
structure to be passed by pointers. It is protected by RCU sched, and
uses the rcu_dereference_sched() to retrieve the pointer, but while
testing it against gcc 8, it gave the error below. It compiles fine
on gcc 10. The issue is that on gcc 8, the "typeof(*p)" used in
rcu_dereference_sched() causes the "incomplete type" error, because
it does a "*p" where p is a pointer to the undefined struct
trace_pid_list. To get around this error, I declared struct
trace_pid_list as the following:

struct trace_pid_list {
volatile void *ignore;
};

With a #ifdef around it to allow it to be declared properly where it
is modified, but all other uses has this fake structure pointer.
This is obviously a hack workaround. But since we support gcc 8, and
I don't want to expose this structure for anything else, is this OK
to do?
]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Some compilers give this error:

kernel/trace/ftrace.c: In function 'ftrace_filter_pid_sched_switch_probe':
include/linux/rcupdate.h:389:9: error: dereferencing pointer to incomplete type 'struct trace_pid_list'
typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
^
include/linux/rcupdate.h:558:2: note: in expansion of macro '__rcu_dereference_check'
__rcu_dereference_check((p), (c) || rcu_read_lock_sched_held(), \
^~~~~~~~~~~~~~~~~~~~~~~
include/linux/rcupdate.h:612:34: note: in expansion of macro 'rcu_dereference_sched_check'
#define rcu_dereference_sched(p) rcu_dereference_sched_check(p, 0)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
kernel/trace/ftrace.c:7101:13: note: in expansion of macro 'rcu_dereference_sched'
pid_list = rcu_dereference_sched(tr->function_pids);
^~~~~~~~~~~~~~~~~~~~~

The reason is that rcu_dereference_sched() has a check that uses
typeof(*p) of the pointer passed to it. But here, the pointer is of type
"struct trace_pid_list *" which is abstracted out, and nothing outside of
pid_list.c should care what the content of it is. But the check uses
typeof(*p) and on some (not all) compilers, it errors with the
dereferencing pointer to incomplete type, which is totally bogus here.

Instead of just declaring "struct trace_pid_list", define it as a
structure with a volatile pointer (just to keep the compiler from doing any
optimization tricks).

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/pid_list.c | 1 +
kernel/trace/trace.h | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/pid_list.c b/kernel/trace/pid_list.c
index 6d1e6cda6973..c4b1bbc59b7b 100644
--- a/kernel/trace/pid_list.c
+++ b/kernel/trace/pid_list.c
@@ -5,6 +5,7 @@
#include <linux/spinlock.h>
#include <linux/irq_work.h>
#include <linux/slab.h>
+#define DEFINED_PID_LIST
#include "trace.h"

/*
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index fe13a0542486..46323ceed8e8 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -188,7 +188,19 @@ struct trace_options {
struct trace_option_dentry *topts;
};

-struct trace_pid_list;
+#ifndef DEFINED_PID_LIST
+/*
+ * rcu_dereference_sched() does typeof(*p) on struct trace_pid_list *, and that
+ * causes some compiler versions to error, with "dereferencing pointer to incomplete type"
+ * because the "*p" of "typeof(*p)" dereferences the pointer to trace_pid_list.
+ * As nothing should need to know that structure, and it should remain
+ * abstracted, define a fake structure for all uses, and define it where it is
+ * actually updated.
+ */
+struct trace_pid_list {
+ volatile void *ignore; /* Add volatile just to keep from any tricky optimizations */
+};
+#endif

struct trace_pid_list *trace_pid_list_alloc(void);
void trace_pid_list_free(struct trace_pid_list *pid_list);
--
2.31.1

--
Sent from my Android device with K-9 Mail. Please excuse my brevity and top posting.

\
 
 \ /
  Last update: 2021-10-02 22:04    [W:0.041 / U:5.848 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site