[RFC PATCH 0/9] Export information needed by the LTTng kernel tracer

[RFC PATCH 0/9] Export information needed by the LTTng kernel tracer

[Mathieu Desnoyers]

Recently commit 0bd476e6c6 "kallsyms: unexport kallsyms_lookup_name()
and kallsyms_on_each_symbol()" was merged upstream. As discussed
previously [1], LTTng-modules (a GPL kernel tracer module) currently
uses kallsyms to lookup some missing symbols.

This patch set integrates the required changes to the kernel code
wherever those changes are generally useful, and adds a few GPL-exports
targeting the information currently missing so LTTng modules can be
entirely free of kallsyms.

Feedback is welcome,

Thanks,

Mathieu

[1] https://lore.kernel.org/r/20200302192811.n6o5645rsib44vco@localhost

Mathieu Desnoyers (9):
  tracepoint: call vmalloc_sync_mappings() on registration
  bpf: allow up to 13 arguments for tracepoints
  writeback: tracing: pass global_wb_domain as tracepoint parameter
  stacktrace: export-GPL stack_trace_save_user
  sched: export-GPL task_prio
  mm: export-GPL get_pageblock_migratetype
  block: genhd: export-GPL gendisk_name
  block: genhd: export-GPL generic disk device type
  block: genhd: export-GPL generic disk block class

 block/bio.c                      |  2 +-
 block/blk-settings.c             |  2 +-
 block/blk.h                      |  2 +-
 block/genhd.c                    | 38 ++++++++++++++++++++++++--------
 block/partitions/core.c          |  4 ++--
 drivers/base/class.c             |  2 +-
 drivers/base/core.c              | 15 +++++++------
 drivers/base/devtmpfs.c          |  2 +-
 include/linux/genhd.h            |  4 +++-
 include/linux/kernel.h           |  6 ++---
 include/linux/trace_events.h     |  3 +++
 include/trace/bpf_probe.h        |  3 ++-
 include/trace/events/writeback.h | 17 ++++++++------
 init/do_mounts.c                 |  4 ++--
 kernel/sched/core.c              |  1 +
 kernel/stacktrace.c              |  1 +
 kernel/trace/bpf_trace.c         |  8 ++++---
 kernel/tracepoint.c              |  2 ++
 mm/page-writeback.c              |  9 ++++----
 mm/page_alloc.c                  |  1 +
 20 files changed, 82 insertions(+), 44 deletions(-)

-- 
2.17.1

[RFC PATCH 1/9] tracepoint: call vmalloc_sync_mappings() on registration

[Mathieu Desnoyers]

Similarly to register_die_notifier(), tracepoints register callbacks
which can be invoked from execution contexts such as the page fault
handler and NMIs which should not generate a page fault. Those callbacks
can access virtual mappings and can be implemented in modules.

Calling vmalloc_sync_mappings() before registering a tracepoint callback
ensures that the virtual mappings are synchronized within each process
page tables, thus ensuring that no page fault will be triggered by the
callback.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/tracepoint.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 73956eaff8a9..82191a89c72e 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -14,6 +14,7 @@
 #include <linux/sched/signal.h>
 #include <linux/sched/task.h>
 #include <linux/static_key.h>
+#include <linux/vmalloc.h>
 
 extern tracepoint_ptr_t __start___tracepoints_ptrs[];
 extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
@@ -308,6 +309,7 @@ int tracepoint_probe_register_prio(struct tracepoint *tp, void *probe,
 	struct tracepoint_func tp_func;
 	int ret;
 
+	vmalloc_sync_mappings();
 	mutex_lock(&tracepoints_mutex);
 	tp_func.func = probe;
 	tp_func.data = data;
-- 
2.17.1

[RFC PATCH 2/9] bpf: allow up to 13 arguments for tracepoints

[Mathieu Desnoyers]

Adding an argument to the writeback balance_dirty_pages event requires
to bump the bpf argument count limit from 12 to 13.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 include/linux/kernel.h       | 6 +++---
 include/linux/trace_events.h | 3 +++
 include/trace/bpf_probe.h    | 3 ++-
 kernel/trace/bpf_trace.c     | 8 +++++---
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 9b7a8d74a9d6..20203f762410 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -975,9 +975,9 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
 #define swap(a, b) \
 	do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
 
-/* This counts to 12. Any more, it will return 13th argument. */
-#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
-#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+/* This counts to 13. Any more, it will return 14th argument. */
+#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _n, X...) _n
+#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
 
 #define __CONCAT(a, b) a ## b
 #define CONCATENATE(a, b) __CONCAT(a, b)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5c6943354049..1e3f4782d32f 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -776,6 +776,9 @@ void bpf_trace_run11(struct bpf_prog *prog, u64 arg1, u64 arg2,
 void bpf_trace_run12(struct bpf_prog *prog, u64 arg1, u64 arg2,
 		     u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
 		     u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12);
+void bpf_trace_run13(struct bpf_prog *prog, u64 arg1, u64 arg2,
+		     u64 arg3, u64 arg4, u64 arg5, u64 arg6, u64 arg7,
+		     u64 arg8, u64 arg9, u64 arg10, u64 arg11, u64 arg12, u64 arg13);
 void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
 			       struct trace_event_call *call, u64 count,
 			       struct pt_regs *regs, struct hlist_head *head,
diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
index 1ce3be63add1..1ca87b7a8230 100644
--- a/include/trace/bpf_probe.h
+++ b/include/trace/bpf_probe.h
@@ -52,7 +52,8 @@
 #define __CAST10(a,...) __CAST_TO_U64(a), __CAST9(__VA_ARGS__)
 #define __CAST11(a,...) __CAST_TO_U64(a), __CAST10(__VA_ARGS__)
 #define __CAST12(a,...) __CAST_TO_U64(a), __CAST11(__VA_ARGS__)
-/* tracepoints with more than 12 arguments will hit build error */
+#define __CAST13(a,...) __CAST_TO_U64(a), __CAST12(__VA_ARGS__)
+/* tracepoints with more than 13 arguments will hit build error */
 #define CAST_TO_U64(...) CONCATENATE(__CAST, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
 
 #undef DECLARE_EVENT_CLASS
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1796747a77..67354218b97f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1546,6 +1546,7 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
 #define REPEAT_10(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_9(FN, DL, __VA_ARGS__)
 #define REPEAT_11(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_10(FN, DL, __VA_ARGS__)
 #define REPEAT_12(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_11(FN, DL, __VA_ARGS__)
+#define REPEAT_13(FN, DL, X, ...)	FN(X) UNPACK DL REPEAT_12(FN, DL, __VA_ARGS__)
 #define REPEAT(X, FN, DL, ...)		REPEAT_##X(FN, DL, __VA_ARGS__)
 
 #define SARG(X)		u64 arg##X
@@ -1554,14 +1555,14 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
 #define __DL_COM	(,)
 #define __DL_SEM	(;)
 
-#define __SEQ_0_11	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
+#define __SEQ_0_12	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
 
 #define BPF_TRACE_DEFN_x(x)						\
 	void bpf_trace_run##x(struct bpf_prog *prog,			\
-			      REPEAT(x, SARG, __DL_COM, __SEQ_0_11))	\
+			      REPEAT(x, SARG, __DL_COM, __SEQ_0_12))	\
 	{								\
 		u64 args[x];						\
-		REPEAT(x, COPY, __DL_SEM, __SEQ_0_11);			\
+		REPEAT(x, COPY, __DL_SEM, __SEQ_0_12);			\
 		__bpf_trace_run(prog, args);				\
 	}								\
 	EXPORT_SYMBOL_GPL(bpf_trace_run##x)
@@ -1577,6 +1578,7 @@ BPF_TRACE_DEFN_x(9);
 BPF_TRACE_DEFN_x(10);
 BPF_TRACE_DEFN_x(11);
 BPF_TRACE_DEFN_x(12);
+BPF_TRACE_DEFN_x(13);
 
 static int __bpf_probe_register(struct bpf_raw_event_map *btp, struct bpf_prog *prog)
 {
-- 
2.17.1

[RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter

[Mathieu Desnoyers]

The symbol global_wb_domain is not GPL-exported to modules. In order
to allow kernel tracers implemented as GPL modules to access the
global writeback domain dirty limit, as used within the
global_dirty_state and balance_dirty_pages trace events, pass
a pointer to the global writeback domain as a new parameter for
those tracepoints.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bpf@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 include/trace/events/writeback.h | 17 ++++++++++-------
 mm/page-writeback.c              |  9 +++++----
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
index d94def25e4dc..8fd774108c9c 100644
--- a/include/trace/events/writeback.h
+++ b/include/trace/events/writeback.h
@@ -531,11 +531,13 @@ TRACE_EVENT(writeback_queue_io,
 
 TRACE_EVENT(global_dirty_state,
 
-	TP_PROTO(unsigned long background_thresh,
+	TP_PROTO(struct wb_domain *domain,
+		 unsigned long background_thresh,
 		 unsigned long dirty_thresh
 	),
 
-	TP_ARGS(background_thresh,
+	TP_ARGS(domain,
+		background_thresh,
 		dirty_thresh
 	),
 
@@ -558,7 +560,7 @@ TRACE_EVENT(global_dirty_state,
 		__entry->nr_written	= global_node_page_state(NR_WRITTEN);
 		__entry->background_thresh = background_thresh;
 		__entry->dirty_thresh	= dirty_thresh;
-		__entry->dirty_limit	= global_wb_domain.dirty_limit;
+		__entry->dirty_limit	= domain->dirty_limit;
 	),
 
 	TP_printk("dirty=%lu writeback=%lu unstable=%lu "
@@ -625,7 +627,8 @@ TRACE_EVENT(bdi_dirty_ratelimit,
 
 TRACE_EVENT(balance_dirty_pages,
 
-	TP_PROTO(struct bdi_writeback *wb,
+	TP_PROTO(struct wb_domain *domain,
+		 struct bdi_writeback *wb,
 		 unsigned long thresh,
 		 unsigned long bg_thresh,
 		 unsigned long dirty,
@@ -638,7 +641,7 @@ TRACE_EVENT(balance_dirty_pages,
 		 long pause,
 		 unsigned long start_time),
 
-	TP_ARGS(wb, thresh, bg_thresh, dirty, bdi_thresh, bdi_dirty,
+	TP_ARGS(domain, wb, thresh, bg_thresh, dirty, bdi_thresh, bdi_dirty,
 		dirty_ratelimit, task_ratelimit,
 		dirtied, period, pause, start_time),
 
@@ -664,8 +667,8 @@ TRACE_EVENT(balance_dirty_pages,
 		unsigned long freerun = (thresh + bg_thresh) / 2;
 		strscpy_pad(__entry->bdi, bdi_dev_name(wb->bdi), 32);
 
-		__entry->limit		= global_wb_domain.dirty_limit;
-		__entry->setpoint	= (global_wb_domain.dirty_limit +
+		__entry->limit		= domain->dirty_limit;
+		__entry->setpoint	= (domain->dirty_limit +
 						freerun) / 2;
 		__entry->dirty		= dirty;
 		__entry->bdi_setpoint	= __entry->setpoint *
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 7326b54ab728..c76aae305360 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -443,9 +443,8 @@ static void domain_dirty_limits(struct dirty_throttle_control *dtc)
 	dtc->thresh = thresh;
 	dtc->bg_thresh = bg_thresh;
 
-	/* we should eventually report the domain in the TP */
 	if (!gdtc)
-		trace_global_dirty_state(bg_thresh, thresh);
+		trace_global_dirty_state(&global_wb_domain, bg_thresh, thresh);
 }
 
 /**
@@ -1736,7 +1735,8 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 		 * do a reset, as it may be a light dirtier.
 		 */
 		if (pause < min_pause) {
-			trace_balance_dirty_pages(wb,
+			trace_balance_dirty_pages(&global_wb_domain,
+						  wb,
 						  sdtc->thresh,
 						  sdtc->bg_thresh,
 						  sdtc->dirty,
@@ -1765,7 +1765,8 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
 		}
 
 pause:
-		trace_balance_dirty_pages(wb,
+		trace_balance_dirty_pages(&global_wb_domain,
+					  wb,
 					  sdtc->thresh,
 					  sdtc->bg_thresh,
 					  sdtc->dirty,
-- 
2.17.1

Re: [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter

[Alexei Starovoitov]

On Thu, Apr 09, 2020 at 03:35:37PM -0400, Mathieu Desnoyers wrote:
>  		if (pause < min_pause) {
> -			trace_balance_dirty_pages(wb,
> +			trace_balance_dirty_pages(&global_wb_domain,
> +						  wb,
>  						  sdtc->thresh,
>  						  sdtc->bg_thresh,
>  						  sdtc->dirty,

argh. 13 arguments to single function ?!
Currently the call site looks like:
                        trace_balance_dirty_pages(wb,
                                                  sdtc->thresh,
                                                  sdtc->bg_thresh,
                                                  sdtc->dirty,
                                                  sdtc->wb_thresh,
                                                  sdtc->wb_dirty,
                                                  dirty_ratelimit,
                                                  task_ratelimit,
                                                  pages_dirtied,
                                                  period,
                                                  min(pause, 0L),
                                                  start_time);
Just pass sdtc as a pointer instead.
Then another wb argument will be fine.

Re: [RFC PATCH 3/9] writeback: tracing: pass global_wb_domain as tracepoint parameter

[Mathieu Desnoyers]

----- On Apr 9, 2020, at 6:33 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:

> On Thu, Apr 09, 2020 at 03:35:37PM -0400, Mathieu Desnoyers wrote:
>>  		if (pause < min_pause) {
>> -			trace_balance_dirty_pages(wb,
>> +			trace_balance_dirty_pages(&global_wb_domain,
>> +						  wb,
>>  						  sdtc->thresh,
>>  						  sdtc->bg_thresh,
>>  						  sdtc->dirty,
> 
> argh. 13 arguments to single function ?!
> Currently the call site looks like:
>                        trace_balance_dirty_pages(wb,
>                                                  sdtc->thresh,
>                                                  sdtc->bg_thresh,
>                                                  sdtc->dirty,
>                                                  sdtc->wb_thresh,
>                                                  sdtc->wb_dirty,
>                                                  dirty_ratelimit,
>                                                  task_ratelimit,
>                                                  pages_dirtied,
>                                                  period,
>                                                  min(pause, 0L),
>                                                  start_time);
> Just pass sdtc as a pointer instead.
> Then another wb argument will be fine.

Good point! In order to do that I would need to move the declaration
of struct dirty_throttle_control from mm/page-writeback.c to
include/linux/writeback.h so it can be used from the tracepoint.

Seems fair ?

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

[RFC PATCH 4/9] stacktrace: export-GPL stack_trace_save_user

[Mathieu Desnoyers]

The stack_trace_save symbol is already exported GPL. Add a GPL export
for the stack_trace_save_user symbol as well.

This is useful for tracers implemented as kernel modules.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
---
 kernel/stacktrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
index 2af66e449aa6..c8c48444bd39 100644
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -240,6 +240,7 @@ unsigned int stack_trace_save_user(unsigned long *store, unsigned int size)
 
 	return c.len;
 }
+EXPORT_SYMBOL_GPL(stack_trace_save_user);
 #endif
 
 #else /* CONFIG_ARCH_STACKWALK */
-- 
2.17.1

Re: [RFC PATCH 4/9] stacktrace: export-GPL stack_trace_save_user

[Christoph Hellwig]

On Thu, Apr 09, 2020 at 03:35:38PM -0400, Mathieu Desnoyers wrote:
> The stack_trace_save symbol is already exported GPL. Add a GPL export
> for the stack_trace_save_user symbol as well.
> 
> This is useful for tracers implemented as kernel modules.

Which part of every added export needs an in-tree user did you not get?

[RFC PATCH 5/9] sched: export-GPL task_prio

[Mathieu Desnoyers]

Exporting the task_prio() symbol to GPL modules is useful for kernel
tracers implemented as kernel modules. It exports information which is
already available through /proc in a way which allows it to be sampled
by kernel tracers with low overhead.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/sched/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a2694ba82874..f87cffef317a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4629,6 +4629,7 @@ int task_prio(const struct task_struct *p)
 {
 	return p->prio - MAX_RT_PRIO;
 }
+EXPORT_SYMBOL_GPL(task_prio);
 
 /**
  * idle_cpu - is a given CPU idle currently?
-- 
2.17.1

[RFC PATCH 6/9] mm: export-GPL get_pageblock_migratetype

[Mathieu Desnoyers]

The macro include/linux/mmzone.h:get_pageblock_migratetype() uses
the symbol get_pfnblock_flags_mask, which is not exported. That macro is
used within the kmem and page_ref trace events to trace the migrate type
of the target page.

Exporting this symbol to GPL modules allows GPL kernel tracers to be
implemented as modules.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-mm@kvack.org
---
 mm/page_alloc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 114c56c3685d..390febb028a0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -501,6 +501,7 @@ unsigned long get_pfnblock_flags_mask(struct page *page, unsigned long pfn,
 {
 	return __get_pfnblock_flags_mask(page, pfn, end_bitidx, mask);
 }
+EXPORT_SYMBOL_GPL(get_pfnblock_flags_mask);
 
 static __always_inline int get_pfnblock_migratetype(struct page *page, unsigned long pfn)
 {
-- 
2.17.1

Re: [RFC PATCH 6/9] mm: export-GPL get_pageblock_migratetype

[Christoph Hellwig]

On Thu, Apr 09, 2020 at 03:35:40PM -0400, Mathieu Desnoyers wrote:
> The macro include/linux/mmzone.h:get_pageblock_migratetype() uses
> the symbol get_pfnblock_flags_mask, which is not exported. That macro is
> used within the kmem and page_ref trace events to trace the migrate type
> of the target page.
> 
> Exporting this symbol to GPL modules allows GPL kernel tracers to be
> implemented as modules.

Which might make some sense if we had such intree modules (and then even
some as we could have higher level functions).  Without that is
obviously is a no-go.

[RFC PATCH 7/9] block: genhd: export-GPL gendisk_name

[Mathieu Desnoyers]

Iteration on class devices is exported for use by GPL modules, but the
helper for formatting generic disk name is not.

The symbol "disk_name" used for this helper could benefit from
a more specific namespacing. Rename it to "gendisk_name" and adapt all
users across the kernel tree.

Export the generic disk name formatting helper for use by GPL modules.
This is useful for tracing a meaningful list of block devices from
tracers implemented as GPL modules.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 block/bio.c             |  2 +-
 block/blk-settings.c    |  2 +-
 block/blk.h             |  2 +-
 block/genhd.c           | 13 +++++++------
 block/partitions/core.c |  2 +-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 21cbaa6a1c20..d6665bab1fef 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -725,7 +725,7 @@ EXPORT_SYMBOL(bio_clone_fast);
 
 const char *bio_devname(struct bio *bio, char *buf)
 {
-	return disk_name(bio->bi_disk, bio->bi_partno, buf);
+	return gendisk_name(bio->bi_disk, bio->bi_partno, buf);
 }
 EXPORT_SYMBOL(bio_devname);
 
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 14397b4c4b53..0b2eed0ed556 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -622,7 +622,7 @@ void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
 	if (bdev_stack_limits(&t->limits, bdev, offset >> 9) < 0) {
 		char top[BDEVNAME_SIZE], bottom[BDEVNAME_SIZE];
 
-		disk_name(disk, 0, top);
+		gendisk_name(disk, 0, top);
 		bdevname(bdev, bottom);
 
 		printk(KERN_NOTICE "%s: Warning: Device %s is misaligned\n",
diff --git a/block/blk.h b/block/blk.h
index 0a94ec68af32..21978defa4b5 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -385,7 +385,7 @@ struct hd_struct *disk_map_sector_rcu(struct gendisk *disk, sector_t sector);
 int blk_alloc_devt(struct hd_struct *part, dev_t *devt);
 void blk_free_devt(dev_t devt);
 void blk_invalidate_devt(dev_t devt);
-char *disk_name(struct gendisk *hd, int partno, char *buf);
+char *gendisk_name(struct gendisk *hd, int partno, char *buf);
 #define ADDPART_FLAG_NONE	0
 #define ADDPART_FLAG_RAID	1
 #define ADDPART_FLAG_WHOLEDISK	2
diff --git a/block/genhd.c b/block/genhd.c
index 06b642b23a07..409c5a92b0b9 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -74,7 +74,7 @@ EXPORT_SYMBOL_GPL(set_capacity_revalidate_and_notify);
  * Format the device name of the indicated disk into the supplied buffer and
  * return a pointer to that same buffer for convenience.
  */
-char *disk_name(struct gendisk *hd, int partno, char *buf)
+char *gendisk_name(struct gendisk *hd, int partno, char *buf)
 {
 	if (!partno)
 		snprintf(buf, BDEVNAME_SIZE, "%s", hd->disk_name);
@@ -85,10 +85,11 @@ char *disk_name(struct gendisk *hd, int partno, char *buf)
 
 	return buf;
 }
+EXPORT_SYMBOL_GPL(gendisk_name);
 
 const char *bdevname(struct block_device *bdev, char *buf)
 {
-	return disk_name(bdev->bd_disk, bdev->bd_part->partno, buf);
+	return gendisk_name(bdev->bd_disk, bdev->bd_part->partno, buf);
 }
 EXPORT_SYMBOL(bdevname);
 
@@ -1081,8 +1082,8 @@ void __init printk_all_partitions(void)
 
 			printk("%s%s %10llu %s %s", is_part0 ? "" : "  ",
 			       bdevt_str(part_devt(part), devt_buf),
-			       (unsigned long long)part_nr_sects_read(part) >> 1
-			       , disk_name(disk, part->partno, name_buf),
+			       (unsigned long long)part_nr_sects_read(part) >> 1,
+			       gendisk_name(disk, part->partno, name_buf),
 			       part->info ? part->info->uuid : "");
 			if (is_part0) {
 				if (dev->parent && dev->parent->driver)
@@ -1175,7 +1176,7 @@ static int show_partition(struct seq_file *seqf, void *v)
 		seq_printf(seqf, "%4d  %7d %10llu %s\n",
 			   MAJOR(part_devt(part)), MINOR(part_devt(part)),
 			   (unsigned long long)part_nr_sects_read(part) >> 1,
-			   disk_name(sgp, part->partno, buf));
+			   gendisk_name(sgp, part->partno, buf));
 	disk_part_iter_exit(&piter);
 
 	return 0;
@@ -1583,7 +1584,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 			   "%lu %u"
 			   "\n",
 			   MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
-			   disk_name(gp, hd->partno, buf),
+			   gendisk_name(gp, hd->partno, buf),
 			   stat.ios[STAT_READ],
 			   stat.merges[STAT_READ],
 			   stat.sectors[STAT_READ],
diff --git a/block/partitions/core.c b/block/partitions/core.c
index b79c4513629b..56f0b730a2a0 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -129,7 +129,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd,
 	state->pp_buf[0] = '\0';
 
 	state->bdev = bdev;
-	disk_name(hd, 0, state->name);
+	gendisk_name(hd, 0, state->name);
 	snprintf(state->pp_buf, PAGE_SIZE, " %s:", state->name);
 	if (isdigit(state->name[strlen(state->name)-1]))
 		sprintf(state->name, "p");
-- 
2.17.1

[RFC PATCH 8/9] block: genhd: export-GPL generic disk device type

[Mathieu Desnoyers]

Iteration on class devices is exported for use by GPL modules, but
there is no exported function for getting the generic disk device type
which is required to perform iteration on the generic disks.

Export a new getter for disk device type for use by GPL modules. This is
useful for tracing a meaningful list of block devices from tracers
implemented as GPL modules.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 block/genhd.c         | 9 +++++++++
 include/linux/genhd.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 409c5a92b0b9..e104b696002f 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1546,6 +1546,15 @@ static const struct device_type disk_type = {
 	.devnode	= block_devnode,
 };
 
+/*
+ * Return the generic disk device type.
+ */
+const struct device_type *gendisk_device_type(void)
+{
+	return &disk_type;
+}
+EXPORT_SYMBOL_GPL(gendisk_device_type);
+
 #ifdef CONFIG_PROC_FS
 /*
  * aggregate disk stat collector.  Uses the same stats that the sysfs
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 9b3fffdf4011..3307dfae72cb 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -353,6 +353,8 @@ extern void blk_register_region(dev_t devt, unsigned long range,
 			void *data);
 extern void blk_unregister_region(dev_t devt, unsigned long range);
 
+extern const struct device_type *gendisk_device_type(void);
+
 #define alloc_disk_node(minors, node_id)				\
 ({									\
 	static struct lock_class_key __key;				\
-- 
2.17.1

Re: [RFC PATCH 8/9] block: genhd: export-GPL generic disk device type

[Greg Kroah-Hartman]

On Thu, Apr 09, 2020 at 03:35:42PM -0400, Mathieu Desnoyers wrote:
> Iteration on class devices is exported for use by GPL modules, but
> there is no exported function for getting the generic disk device type
> which is required to perform iteration on the generic disks.
> 
> Export a new getter for disk device type for use by GPL modules. This is
> useful for tracing a meaningful list of block devices from tracers
> implemented as GPL modules.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  block/genhd.c         | 9 +++++++++
>  include/linux/genhd.h | 2 ++
>  2 files changed, 11 insertions(+)

I understand your need here, however we do not export things for
modules, when there are no in-kernel module users, sorry.

I have your last thread somewhere in my todo pile, to try to respond as
to how to make this not be an issue for you, sorry I haven't gotten to
it.

Why can't you just add a tracepoint instead of having to dig through
this mess?  Wouldn't that solve a lot of these issues for block devices?

thanks,

greg k-h

Re: [RFC PATCH 8/9] block: genhd: export-GPL generic disk device type

[Mathieu Desnoyers]

----- On Apr 10, 2020, at 2:33 AM, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:

> On Thu, Apr 09, 2020 at 03:35:42PM -0400, Mathieu Desnoyers wrote:
>> Iteration on class devices is exported for use by GPL modules, but
>> there is no exported function for getting the generic disk device type
>> which is required to perform iteration on the generic disks.
>> 
>> Export a new getter for disk device type for use by GPL modules. This is
>> useful for tracing a meaningful list of block devices from tracers
>> implemented as GPL modules.
>> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Tejun Heo <tj@kernel.org>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> ---
>>  block/genhd.c         | 9 +++++++++
>>  include/linux/genhd.h | 2 ++
>>  2 files changed, 11 insertions(+)
> 
> I understand your need here, however we do not export things for
> modules, when there are no in-kernel module users, sorry.
> 
> I have your last thread somewhere in my todo pile, to try to respond as
> to how to make this not be an issue for you, sorry I haven't gotten to
> it.

Thanks for taking time to look into this.

Unfortunately, having commit 0bd476e6c6 "kallsyms: unexport kallsyms_lookup_name()
and kallsyms_on_each_symbol()" now merged into mainline before that discussion
was completed now makes it an immediate issue. I would have preferred to have more
time to discuss the possible solutions before seeing that commit upstream.

> Why can't you just add a tracepoint instead of having to dig through
> this mess?  Wouldn't that solve a lot of these issues for block devices?

The problem here is that it's not something which can be exported with "just"
a tracepoint. This is really a "kernel state dumping" facility meant to be
used by kernel tracers. We can indeed use tracepoints as data-export hooking
mechanism (I actually do this already within the LTTng statedump module), but
we need a function symbol which can be called by the tracer to trigger a dump
of the relevant kernel data structures.

I'm very much open to contribute any parts of the LTTng statedump facility
to the kernel, but last time this was discussed (a few years ago), I recall
that dumping kernel state was not deemed useful for the use-cases targeted
by perf and ftrace maintainers. But maybe the situation has changed now ?

For LTTng, this statedump facility is the underlying foundation which allows
doing stateful analyses at post-processing.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [RFC PATCH 8/9] block: genhd: export-GPL generic disk device type

[Steven Rostedt]

On Fri, 10 Apr 2020 08:33:57 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> I understand your need here, however we do not export things for
> modules, when there are no in-kernel module users, sorry.

This "we don't cater to out-of-tree modules" even when they are GPL seems
to always baffle me. Especially since we have a high bar of accepting out
of tree modules especially if they duplicate some functionality of an
existing infrastructure of the kernel. I like choice, and coming from
someone that spent over a decade working on code that has been out of tree,
I'm a little sympathetic to the cause ;-)

I guess we should be open to allowing LTTng modules in the kernel as well,
even though it is yet another tracing framework. It's not like its going
away. And perhaps by doing so, ftrace and perf could start taking advantage
of anything that LTTng brings.

-- Steve

Re: [RFC PATCH 8/9] block: genhd: export-GPL generic disk device type

[Greg Kroah-Hartman]

On Fri, Apr 10, 2020 at 11:44:44AM -0400, Steven Rostedt wrote:
> On Fri, 10 Apr 2020 08:33:57 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > I understand your need here, however we do not export things for
> > modules, when there are no in-kernel module users, sorry.
> 
> This "we don't cater to out-of-tree modules" even when they are GPL seems
> to always baffle me. Especially since we have a high bar of accepting out
> of tree modules especially if they duplicate some functionality of an
> existing infrastructure of the kernel. I like choice, and coming from
> someone that spent over a decade working on code that has been out of tree,
> I'm a little sympathetic to the cause ;-)

We can't do anything for out-of-tree modules as they suddenly become
"higher priority" than in-tree code if you have to not do specific
changes or extra work for them.  Which is not fair at all to the in-tree
code developers at all.

With drivers/staging/ we removed the barrier for accepting any license
compliant driver, so that solved the huge majority of these issues.

> I guess we should be open to allowing LTTng modules in the kernel as well,
> even though it is yet another tracing framework. It's not like its going
> away. And perhaps by doing so, ftrace and perf could start taking advantage
> of anything that LTTng brings.

That is up to you all, as you are the one preventing this from being
merged in the tree, not me :)

Again, don't make us do _more_ work for out-of-tree modules than we do
for in-tree modules, that's just crazy to expect.

thanks,

greg k-h

[RFC PATCH 9/9] block: genhd: export-GPL generic disk block class

[Mathieu Desnoyers]

The "block_class" symbol is currently used across the kernel tree
without a clear namespace. It is also unavailable for GPL modules for
iteration over generic disks.

Introduce a GPL-exported gendisk_block_class() to get the generic disk
block class. Make "block_class" static, therefore limiting its scope to
genhd.c.

Replace all use of "block_class" across the kernel tree by use of the
new getter function.

This is useful for tracing a meaningful list of block devices from
tracers implemented as GPL modules.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 block/genhd.c           | 16 +++++++++++++---
 block/partitions/core.c |  2 +-
 drivers/base/class.c    |  2 +-
 drivers/base/core.c     | 15 ++++++++-------
 drivers/base/devtmpfs.c |  2 +-
 include/linux/genhd.h   |  2 +-
 init/do_mounts.c        |  4 ++--
 7 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index e104b696002f..c87c83983d72 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -40,6 +40,10 @@ static DEFINE_IDR(ext_devt_idr);
 
 static const struct device_type disk_type;
 
+static struct class block_class = {
+	.name		= "block",
+};
+
 static void disk_check_events(struct disk_events *ev,
 			      unsigned int *clearing_ptr);
 static void disk_alloc_events(struct gendisk *disk);
@@ -1525,9 +1529,15 @@ static void disk_release(struct device *dev)
 		blk_put_queue(disk->queue);
 	kfree(disk);
 }
-struct class block_class = {
-	.name		= "block",
-};
+
+/*
+ * Return the generic disk block class.
+ */
+struct class *gendisk_block_class(void)
+{
+	return &block_class;
+}
+EXPORT_SYMBOL_GPL(gendisk_block_class);
 
 static char *block_devnode(struct device *dev, umode_t *mode,
 			   kuid_t *uid, kgid_t *gid)
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 56f0b730a2a0..419dcdb811b3 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -413,7 +413,7 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno,
 		dev_set_name(pdev, "%s%d", dname, partno);
 
 	device_initialize(pdev);
-	pdev->class = &block_class;
+	pdev->class = gendisk_block_class();
 	pdev->type = &part_type;
 	pdev->parent = ddev;
 
diff --git a/drivers/base/class.c b/drivers/base/class.c
index bcd410e6d70a..a6dea6d8bfd6 100644
--- a/drivers/base/class.c
+++ b/drivers/base/class.c
@@ -176,7 +176,7 @@ int __class_register(struct class *cls, struct lock_class_key *key)
 
 #if defined(CONFIG_BLOCK)
 	/* let the block class directory show up in the root of sysfs */
-	if (!sysfs_deprecated || cls != &block_class)
+	if (!sysfs_deprecated || cls != gendisk_block_class())
 		cp->subsys.kobj.kset = class_kset;
 #else
 	cp->subsys.kobj.kset = class_kset;
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 139cdf7e7327..84657fca67f5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2062,13 +2062,14 @@ static struct kobject *get_device_parent(struct device *dev,
 		struct kobject *kobj = NULL;
 		struct kobject *parent_kobj;
 		struct kobject *k;
-
 #ifdef CONFIG_BLOCK
+		struct class *block_class = gendisk_block_class();
+
 		/* block disks show up in /sys/block */
-		if (sysfs_deprecated && dev->class == &block_class) {
-			if (parent && parent->class == &block_class)
+		if (sysfs_deprecated && dev->class == block_class) {
+			if (parent && parent->class == block_class)
 				return &parent->kobj;
-			return &block_class.p->subsys.kobj;
+			return &block_class->p->subsys.kobj;
 		}
 #endif
 
@@ -2228,7 +2229,7 @@ static int device_add_class_symlinks(struct device *dev)
 
 #ifdef CONFIG_BLOCK
 	/* /sys/block has directories and does not need symlinks */
-	if (sysfs_deprecated && dev->class == &block_class)
+	if (sysfs_deprecated && dev->class == gendisk_block_class())
 		return 0;
 #endif
 
@@ -2262,7 +2263,7 @@ static void device_remove_class_symlinks(struct device *dev)
 		sysfs_remove_link(&dev->kobj, "device");
 	sysfs_remove_link(&dev->kobj, "subsystem");
 #ifdef CONFIG_BLOCK
-	if (sysfs_deprecated && dev->class == &block_class)
+	if (sysfs_deprecated && dev->class == gendisk_block_class())
 		return;
 #endif
 	sysfs_delete_link(&dev->class->p->subsys.kobj, &dev->kobj, dev_name(dev));
@@ -3603,7 +3604,7 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
 		goto out;
 
 #ifdef CONFIG_BLOCK
-	if (sysfs_deprecated && dev->class == &block_class)
+	if (sysfs_deprecated && dev->class == gendisk_block_class())
 		goto out;
 #endif
 
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index c9017e0584c0..05cbc8ead4d8 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -83,7 +83,7 @@ static struct file_system_type dev_fs_type = {
 #ifdef CONFIG_BLOCK
 static inline int is_blockdev(struct device *dev)
 {
-	return dev->class == &block_class;
+	return dev->class == gendisk_block_class();
 }
 #else
 static inline int is_blockdev(struct device *dev) { return 0; }
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 3307dfae72cb..953c03da3ed0 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -27,7 +27,6 @@
 #define part_to_dev(part)	(&((part)->__dev))
 
 extern struct device_type part_type;
-extern struct class block_class;
 
 #define DISK_MAX_PARTS			256
 #define DISK_NAME_LEN			32
@@ -354,6 +353,7 @@ extern void blk_register_region(dev_t devt, unsigned long range,
 extern void blk_unregister_region(dev_t devt, unsigned long range);
 
 extern const struct device_type *gendisk_device_type(void);
+extern struct class *gendisk_block_class(void);
 
 #define alloc_disk_node(minors, node_id)				\
 ({									\
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 29d326b6c29d..b94a47589cee 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -138,7 +138,7 @@ static dev_t devt_from_partuuid(const char *uuid_str)
 		goto done;
 	}
 
-	dev = class_find_device(&block_class, NULL, &cmp,
+	dev = class_find_device(gendisk_block_class(), NULL, &cmp,
 				&match_dev_by_uuid);
 	if (!dev)
 		goto done;
@@ -237,7 +237,7 @@ dev_t name_to_dev_t(const char *name)
 	} else if (strncmp(name, "PARTLABEL=", 10) == 0) {
 		struct device *dev;
 
-		dev = class_find_device(&block_class, NULL, name + 10,
+		dev = class_find_device(gendisk_block_class(), NULL, name + 10,
 					&match_dev_by_label);
 		if (!dev)
 			goto fail;
-- 
2.17.1