[PATCH v4] tracing: Export live module tracepoint strings in printk_formats

@@ -509,7 +509,9 @@ struct module {
 #endif
 #ifdef CONFIG_TRACING
 	unsigned int num_trace_bprintk_fmt;
+	unsigned int num_tracepoint_strings;
 	const char **trace_bprintk_fmt_start;
+	const char **tracepoint_strings_start;
 #endif
 #ifdef CONFIG_EVENT_TRACING
 	struct trace_event_call **trace_events;

@@ -508,11 +508,10 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
  * the ASCII strings they represent.
  *
  * The @str used must be a constant string and persistent as it would not
- * make sense to show a string that no longer exists. But it is still fine
- * to be used with modules, because when modules are unloaded, if they
- * had tracepoints, the ring buffers are cleared too. As long as the string
- * does not change during the life of the module, it is fine to use
- * tracepoint_string() within a module.
+ * make sense to show a string that no longer exists. For modules, the string
+ * mapping is exported while the module is loaded and removed on unload.
+ * Exporting the mapping does not extend the lifetime of the string. The
+ * existing lifetime requirements for strings referenced by events still apply.
  */
 #define tracepoint_string(str)						\
 	({								\

@@ -2740,6 +2740,10 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 	mod->trace_bprintk_fmt_start = section_objs(info, "__trace_printk_fmt",
 					 sizeof(*mod->trace_bprintk_fmt_start),
 					 &mod->num_trace_bprintk_fmt);
+	mod->tracepoint_strings_start =
+		section_objs(info, "__tracepoint_str",
+			     sizeof(*mod->tracepoint_strings_start),
+			     &mod->num_tracepoint_strings);
 #endif
 #ifdef CONFIG_DYNAMIC_FTRACE
 	/* sechdrs[0].sh_size is always zero */

@@ -19,15 +19,23 @@
 
 #include "trace.h"
 
+/* Counts unique retained formats; module unload does not decrement this. */
+static int trace_bprintk_fmt_cnt;
+
 #ifdef CONFIG_MODULES
 
 /*
  * modules trace_printk()'s formats are autosaved in struct trace_bprintk_fmt
  * which are queued on trace_bprintk_fmt_list.
+ *
+ * modules tracepoint_string() entries are kept as ranges into the owning
+ * module's __tracepoint_str section and are removed again when the module
+ * goes away.
  */
 static LIST_HEAD(trace_bprintk_fmt_list);
+static LIST_HEAD(tracepoint_str_list);
 
-/* serialize accesses to trace_bprintk_fmt_list */
+/* Serialize module list updates and the full printk_formats iteration. */
 static DEFINE_MUTEX(btrace_mutex);
 
 struct trace_bprintk_fmt {

@@ -35,6 +43,13 @@ struct trace_bprintk_fmt {
 	const char *fmt;
 };
 
+struct tracepoint_mod_str {
+	struct list_head list;
+	struct module *mod;
+	const char **start;
+	unsigned int num;
+};
+
 static inline struct trace_bprintk_fmt *lookup_format(const char *fmt)
 {
 	struct trace_bprintk_fmt *pos;

@@ -75,6 +90,7 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
 			if (fmt) {
 				list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
 				tb_fmt->fmt = fmt;
+				trace_bprintk_fmt_cnt++;
 			} else
 				kfree(tb_fmt);
 		}

@@ -84,16 +100,61 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
 	mutex_unlock(&btrace_mutex);
 }
 
+static void hold_module_tracepoint_strings(struct module *mod)
+{
+	struct tracepoint_mod_str *tp_str;
+
+	if (!mod->num_tracepoint_strings)
+		return;
+
+	tp_str = kmalloc_obj(*tp_str);
+	if (!tp_str)
+		return;
+
+	tp_str->mod = mod;
+	tp_str->start = mod->tracepoint_strings_start;
+	tp_str->num = mod->num_tracepoint_strings;
+
+	guard(mutex)(&btrace_mutex);
+	list_add_tail(&tp_str->list, &tracepoint_str_list);
+}
+
+static void release_module_tracepoint_strings(struct module *mod)
+{
+	struct tracepoint_mod_str *tp_str, *next;
+
+	if (!mod->num_tracepoint_strings)
+		return;
+
+	guard(mutex)(&btrace_mutex);
+	list_for_each_entry_safe(tp_str, next, &tracepoint_str_list, list) {
+		if (tp_str->mod != mod)
+			continue;
+
+		list_del(&tp_str->list);
+		kfree(tp_str);
+		return;
+	}
+}
+
 static int module_trace_bprintk_format_notify(struct notifier_block *self,
 		unsigned long val, void *data)
 {
 	struct module *mod = data;
-	if (mod->num_trace_bprintk_fmt) {
-		const char **start = mod->trace_bprintk_fmt_start;
-		const char **end = start + mod->num_trace_bprintk_fmt;
 
-		if (val == MODULE_STATE_COMING)
+	switch (val) {
+	case MODULE_STATE_COMING:
+		if (mod->num_trace_bprintk_fmt) {
+			const char **start = mod->trace_bprintk_fmt_start;
+			const char **end = start + mod->num_trace_bprintk_fmt;
+
 			hold_module_trace_bprintk_format(start, end);
+		}
+		hold_module_tracepoint_strings(mod);
+		break;
+	case MODULE_STATE_GOING:
+		release_module_tracepoint_strings(mod);
+		break;
 	}
 	return NOTIFY_OK;
 }

@@ -119,7 +180,7 @@ static int module_trace_bprintk_format_notify(struct notifier_block *self,
  * next format in the list.
  */
 static const char **
-find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+find_next_mod_format(loff_t start_index, void *v, const char **fmt, loff_t *pos)
 {
 	struct trace_bprintk_fmt *mod_fmt;
 

@@ -158,6 +219,25 @@ find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
 	return &mod_fmt->fmt;
 }
 
+static const char **
+find_next_mod_tracepoint_str(loff_t start_index, loff_t *pos)
+{
+	struct tracepoint_mod_str *tp_str;
+	loff_t offset = *pos - start_index;
+
+	if (offset < 0)
+		return NULL;
+
+	list_for_each_entry(tp_str, &tracepoint_str_list, list) {
+		if (offset < (loff_t)tp_str->num)
+			return tp_str->start + offset;
+
+		offset -= tp_str->num;
+	}
+
+	return NULL;
+}
+
 static void format_mod_start(void)
 {
 	mutex_lock(&btrace_mutex);

@@ -176,10 +256,17 @@ module_trace_bprintk_format_notify(struct notifier_block *self,
 	return NOTIFY_OK;
 }
 static inline const char **
-find_next_mod_format(int start_index, void *v, const char **fmt, loff_t *pos)
+find_next_mod_format(loff_t start_index, void *v, const char **fmt, loff_t *pos)
 {
 	return NULL;
 }
+
+static inline const char **
+find_next_mod_tracepoint_str(loff_t start_index, loff_t *pos)
+{
+	return NULL;
+}
+
 static inline void format_mod_start(void) { }
 static inline void format_mod_stop(void) { }
 #endif /* CONFIG_MODULES */

@@ -265,8 +352,8 @@ bool trace_is_tracepoint_string(const char *str)
 static const char **find_next(void *v, loff_t *pos)
 {
 	const char **fmt = v;
-	int start_index;
-	int last_index;
+	loff_t start_index;
+	loff_t last_index;
 
 	start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
 

@@ -292,7 +379,11 @@ static const char **find_next(void *v, loff_t *pos)
 		return __start___tracepoint_str + (*pos - last_index);
 
 	start_index += last_index;
-	return find_next_mod_format(start_index, v, fmt, pos);
+	if (*pos - start_index < trace_bprintk_fmt_cnt)
+		return find_next_mod_format(start_index, v, fmt, pos);
+
+	start_index += trace_bprintk_fmt_cnt;
+	return find_next_mod_tracepoint_str(start_index, pos);
 }
 
 static void *

@@ -51,6 +51,9 @@ static void do_simple_thread_func(int cnt, const char *fmt, ...)
 	trace_foo_with_template_print("I have to be different", cnt);
 
 	trace_foo_rel_loc("Hello __rel_loc", cnt, bitmask, current->cpus_ptr);
+
+	trace_foo_tracepoint_str(tracepoint_string("tracepoint_string_test_module_string_1"));
+	trace_foo_tracepoint_str(tracepoint_string("tracepoint_string_test_module_string_2"));
 }
 
 static void simple_thread_func(int cnt)

@@ -623,6 +623,23 @@ TRACE_EVENT(foo_timer_fn,
 
 	TP_printk("name=%s count=%d", __get_str(name), __entry->count)
 );
+
+TRACE_EVENT(foo_tracepoint_str,
+
+	TP_PROTO(const char *str),
+
+	TP_ARGS(str),
+
+	TP_STRUCT__entry(
+		__field(const char *, str)
+	),
+
+	TP_fast_assign(
+		__entry->str = str;
+	),
+
+	TP_printk("str=%s", __entry->str)
+);
 #endif
 
 /***** NOTICE! The #if protection ends here. *****/

@@ -0,0 +1,47 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Test tracepoint strings from a module
+# requires: printk_formats
+
+string1=tracepoint_string_test_module_string_1
+string2=tracepoint_string_test_module_string_2
+event=events/sample-trace/foo_tracepoint_str
+
+rmmod trace-events-sample 2>/dev/null ||:
+if ! modprobe trace-events-sample; then
+    echo "No trace-events-sample module - please make CONFIG_SAMPLE_TRACE_EVENTS=m"
+    exit_unresolved
+fi
+trap 'rmmod trace-events-sample >/dev/null 2>&1 ||:' EXIT
+
+grep -Fq "$string1" printk_formats || exit_fail
+grep -Fq "$string2" printk_formats || exit_fail
+
+echo 1 > "$event/enable"
+echo 1 > tracing_on
+
+# The sample thread emits both strings once per second.
+attempts=100
+while [ "$attempts" -gt 0 ]; do
+    if grep -Fq "foo_tracepoint_str: str=$string1" trace &&
+       grep -Fq "foo_tracepoint_str: str=$string2" trace; then
+        break
+    fi
+    sleep 0.1
+    attempts=$((attempts - 1))
+done
+[ "$attempts" -gt 0 ] || exit_fail
+
+echo 0 > "$event/enable"
+rmmod trace-events-sample || exit_fail
+trap - EXIT
+
+for string in "$string1" "$string2"; do
+    if grep -Fq "$string" printk_formats; then
+        echo "Module string remains after unload: $string"
+        exit_fail
+    else
+        # grep returns 1 for no match; a read error must also fail the test.
+        [ "$?" -eq 1 ] || exit_fail
+    fi
+done