linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs
@ 2022-07-25  6:23 Jianlin Lv
  2022-09-06 15:12 ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Jianlin Lv @ 2022-07-25  6:23 UTC (permalink / raw)
  To: corbet, rostedt, mingo; +Cc: iecedge, jianlv, linux-kernel, linux-doc

The private kprobes are not added to the global list dyn_event_list,
so there is a missing interface to show probe hit and probe miss.
This patch adds a profiling interface to check the number of hits or
misses for private kprobes.

Signed-off-by: Jianlin Lv <iecedge@gmail.com>
---
v2: update commit message
---
 Documentation/trace/kprobetrace.rst |  6 +++-
 kernel/trace/trace_dynevent.c       | 20 +++++++++++
 kernel/trace/trace_dynevent.h       | 37 ++++++++++++++++++++
 kernel/trace/trace_kprobe.c         | 54 +++++++++++++++++++++++++++++
 4 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index b175d88f31eb..8815d64dd8a6 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -146,7 +146,11 @@ trigger:
 Event Profiling
 ---------------
 You can check the total number of probe hits and probe miss-hits via
-/sys/kernel/debug/tracing/kprobe_profile.
+/sys/kernel/debug/tracing/kprobe_profile or
+/sys/kernel/debug/tracing/kprobe_local_profile.
+All kprobe events created by kprobe_events will be added to the global
+list, you can get their profiling via kprobe_profile; kprobe_local_profile
+shows profiling for private kprobe events created by perf_kprobe pmu.
 The first column is event name, the second is the number of probe hits,
 the third is the number of probe miss-hits.
 
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 076b447a1b88..70ec99cd9c53 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -181,6 +181,26 @@ static const struct seq_operations dyn_event_seq_op = {
 	.show	= dyn_event_seq_show
 };
 
+#ifdef CONFIG_KPROBE_EVENTS
+LIST_HEAD(local_event_list);
+
+void *local_event_seq_start(struct seq_file *m, loff_t *pos)
+{
+	mutex_lock(&event_mutex);
+	return seq_list_start(&local_event_list, *pos);
+}
+
+void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &local_event_list, pos);
+}
+
+void local_event_seq_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&event_mutex);
+}
+#endif /* CONFIG_KPROBE_EVENTS */
+
 /*
  * dyn_events_release_all - Release all specific events
  * @type:	the dyn_event_operations * which filters releasing events
diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
index 936477a111d3..e30193470295 100644
--- a/kernel/trace/trace_dynevent.h
+++ b/kernel/trace/trace_dynevent.h
@@ -101,6 +101,43 @@ void dyn_event_seq_stop(struct seq_file *m, void *v);
 int dyn_events_release_all(struct dyn_event_operations *type);
 int dyn_event_release(const char *raw_command, struct dyn_event_operations *type);
 
+#ifdef CONFIG_KPROBE_EVENTS
+extern struct list_head local_event_list;
+
+static inline
+int local_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
+{
+	if (!ev || !ops)
+		return -EINVAL;
+
+	INIT_LIST_HEAD(&ev->list);
+	ev->ops = ops;
+	return 0;
+}
+
+static inline int local_event_add(struct dyn_event *ev)
+{
+	lockdep_assert_held(&event_mutex);
+
+	if (!ev || !ev->ops)
+		return -EINVAL;
+
+	list_add_tail(&ev->list, &local_event_list);
+	return 0;
+}
+
+static inline void local_event_remove(struct dyn_event *ev)
+{
+	lockdep_assert_held(&event_mutex);
+	list_del_init(&ev->list);
+}
+
+void *local_event_seq_start(struct seq_file *m, loff_t *pos);
+void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
+void local_event_seq_stop(struct seq_file *m, void *v);
+
+#endif /* CONFIG_KPROBE_EVENTS */
+
 /*
  * for_each_dyn_event	-	iterate over the dyn_event list
  * @pos:	the struct dyn_event * to use as a loop cursor
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a245ea673715..76f500b17b46 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1213,6 +1213,52 @@ static const struct file_operations kprobe_profile_ops = {
 	.release        = seq_release,
 };
 
+#ifdef CONFIG_KPROBE_EVENTS
+/* kprobe Local profile  */
+static int local_probes_profile_seq_show(struct seq_file *m, void *v)
+{
+	struct dyn_event *ev = v;
+	struct trace_kprobe *tk;
+
+	if (!is_trace_kprobe(ev))
+		return 0;
+
+	tk = to_trace_kprobe(ev);
+	seq_printf(m, "  %-44s %15lu %15lu\n",
+		trace_probe_name(&tk->tp),
+		trace_kprobe_nhit(tk),
+		tk->rp.kp.nmissed);
+
+	return 0;
+}
+
+static const struct seq_operations local_profile_seq_op = {
+	.start  = local_event_seq_start,
+	.next   = local_event_seq_next,
+	.stop   = local_event_seq_stop,
+	.show   = local_probes_profile_seq_show
+};
+
+static int local_profile_open(struct inode *inode, struct file *file)
+{
+	int ret;
+
+	ret = security_locked_down(LOCKDOWN_TRACEFS);
+	if (ret)
+		return ret;
+
+	return seq_open(file, &local_profile_seq_op);
+}
+
+static const struct file_operations kprobe_local_profile_ops = {
+	.owner          = THIS_MODULE,
+	.open           = local_profile_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = seq_release,
+};
+#endif /* CONFIG_KPROBE_EVENTS */
+
 /* Kprobe specific fetch functions */
 
 /* Return the length of string -- including null terminal byte */
@@ -1830,6 +1876,7 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
 	if (ret < 0)
 		goto error;
 
+	local_event_add(&tk->devent);
 	return trace_probe_event_call(&tk->tp);
 error:
 	free_trace_kprobe(tk);
@@ -1849,6 +1896,7 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
 		return;
 	}
 
+	local_event_remove(&tk->devent);
 	__unregister_trace_kprobe(tk);
 
 	free_trace_kprobe(tk);
@@ -1929,6 +1977,12 @@ static __init int init_kprobe_trace(void)
 	trace_create_file("kprobe_profile", TRACE_MODE_READ,
 			  NULL, NULL, &kprobe_profile_ops);
 
+#ifdef CONFIG_KPROBE_EVENTS
+	/* kprobe Local profile */
+	tracefs_create_file("kprobe_local_profile", TRACE_MODE_READ,
+			  NULL, NULL, &kprobe_local_profile_ops);
+#endif /* CONFIG_KPROBE_EVENTS */
+
 	setup_boot_kprobe_events();
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs
  2022-07-25  6:23 [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs Jianlin Lv
@ 2022-09-06 15:12 ` Steven Rostedt
  2022-09-07  8:51   ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2022-09-06 15:12 UTC (permalink / raw)
  To: Jianlin Lv
  Cc: corbet, mingo, jianlv, linux-kernel, linux-doc, Masami Hiramatsu,
	Tom Zanussi


[ Adding Masami and Tom ]

On Mon, 25 Jul 2022 06:23:34 +0000
Jianlin Lv <iecedge@gmail.com> wrote:

> The private kprobes are not added to the global list dyn_event_list,
> so there is a missing interface to show probe hit and probe miss.
> This patch adds a profiling interface to check the number of hits or
> misses for private kprobes.

Masami, what do you think of this patch?

-- Steve

> 
> Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> ---
> v2: update commit message
> ---
>  Documentation/trace/kprobetrace.rst |  6 +++-
>  kernel/trace/trace_dynevent.c       | 20 +++++++++++
>  kernel/trace/trace_dynevent.h       | 37 ++++++++++++++++++++
>  kernel/trace/trace_kprobe.c         | 54 +++++++++++++++++++++++++++++
>  4 files changed, 116 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> index b175d88f31eb..8815d64dd8a6 100644
> --- a/Documentation/trace/kprobetrace.rst
> +++ b/Documentation/trace/kprobetrace.rst
> @@ -146,7 +146,11 @@ trigger:
>  Event Profiling
>  ---------------
>  You can check the total number of probe hits and probe miss-hits via
> -/sys/kernel/debug/tracing/kprobe_profile.
> +/sys/kernel/debug/tracing/kprobe_profile or
> +/sys/kernel/debug/tracing/kprobe_local_profile.
> +All kprobe events created by kprobe_events will be added to the global
> +list, you can get their profiling via kprobe_profile; kprobe_local_profile
> +shows profiling for private kprobe events created by perf_kprobe pmu.
>  The first column is event name, the second is the number of probe hits,
>  the third is the number of probe miss-hits.
>  
> diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
> index 076b447a1b88..70ec99cd9c53 100644
> --- a/kernel/trace/trace_dynevent.c
> +++ b/kernel/trace/trace_dynevent.c
> @@ -181,6 +181,26 @@ static const struct seq_operations dyn_event_seq_op = {
>  	.show	= dyn_event_seq_show
>  };
>  
> +#ifdef CONFIG_KPROBE_EVENTS
> +LIST_HEAD(local_event_list);
> +
> +void *local_event_seq_start(struct seq_file *m, loff_t *pos)
> +{
> +	mutex_lock(&event_mutex);
> +	return seq_list_start(&local_event_list, *pos);
> +}
> +
> +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> +	return seq_list_next(v, &local_event_list, pos);
> +}
> +
> +void local_event_seq_stop(struct seq_file *m, void *v)
> +{
> +	mutex_unlock(&event_mutex);
> +}
> +#endif /* CONFIG_KPROBE_EVENTS */
> +
>  /*
>   * dyn_events_release_all - Release all specific events
>   * @type:	the dyn_event_operations * which filters releasing events
> diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
> index 936477a111d3..e30193470295 100644
> --- a/kernel/trace/trace_dynevent.h
> +++ b/kernel/trace/trace_dynevent.h
> @@ -101,6 +101,43 @@ void dyn_event_seq_stop(struct seq_file *m, void *v);
>  int dyn_events_release_all(struct dyn_event_operations *type);
>  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type);
>  
> +#ifdef CONFIG_KPROBE_EVENTS
> +extern struct list_head local_event_list;
> +
> +static inline
> +int local_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
> +{
> +	if (!ev || !ops)
> +		return -EINVAL;
> +
> +	INIT_LIST_HEAD(&ev->list);
> +	ev->ops = ops;
> +	return 0;
> +}
> +
> +static inline int local_event_add(struct dyn_event *ev)
> +{
> +	lockdep_assert_held(&event_mutex);
> +
> +	if (!ev || !ev->ops)
> +		return -EINVAL;
> +
> +	list_add_tail(&ev->list, &local_event_list);
> +	return 0;
> +}
> +
> +static inline void local_event_remove(struct dyn_event *ev)
> +{
> +	lockdep_assert_held(&event_mutex);
> +	list_del_init(&ev->list);
> +}
> +
> +void *local_event_seq_start(struct seq_file *m, loff_t *pos);
> +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
> +void local_event_seq_stop(struct seq_file *m, void *v);
> +
> +#endif /* CONFIG_KPROBE_EVENTS */
> +
>  /*
>   * for_each_dyn_event	-	iterate over the dyn_event list
>   * @pos:	the struct dyn_event * to use as a loop cursor
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index a245ea673715..76f500b17b46 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1213,6 +1213,52 @@ static const struct file_operations kprobe_profile_ops = {
>  	.release        = seq_release,
>  };
>  
> +#ifdef CONFIG_KPROBE_EVENTS
> +/* kprobe Local profile  */
> +static int local_probes_profile_seq_show(struct seq_file *m, void *v)
> +{
> +	struct dyn_event *ev = v;
> +	struct trace_kprobe *tk;
> +
> +	if (!is_trace_kprobe(ev))
> +		return 0;
> +
> +	tk = to_trace_kprobe(ev);
> +	seq_printf(m, "  %-44s %15lu %15lu\n",
> +		trace_probe_name(&tk->tp),
> +		trace_kprobe_nhit(tk),
> +		tk->rp.kp.nmissed);
> +
> +	return 0;
> +}
> +
> +static const struct seq_operations local_profile_seq_op = {
> +	.start  = local_event_seq_start,
> +	.next   = local_event_seq_next,
> +	.stop   = local_event_seq_stop,
> +	.show   = local_probes_profile_seq_show
> +};
> +
> +static int local_profile_open(struct inode *inode, struct file *file)
> +{
> +	int ret;
> +
> +	ret = security_locked_down(LOCKDOWN_TRACEFS);
> +	if (ret)
> +		return ret;
> +
> +	return seq_open(file, &local_profile_seq_op);
> +}
> +
> +static const struct file_operations kprobe_local_profile_ops = {
> +	.owner          = THIS_MODULE,
> +	.open           = local_profile_open,
> +	.read           = seq_read,
> +	.llseek         = seq_lseek,
> +	.release        = seq_release,
> +};
> +#endif /* CONFIG_KPROBE_EVENTS */
> +
>  /* Kprobe specific fetch functions */
>  
>  /* Return the length of string -- including null terminal byte */
> @@ -1830,6 +1876,7 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
>  	if (ret < 0)
>  		goto error;
>  
> +	local_event_add(&tk->devent);
>  	return trace_probe_event_call(&tk->tp);
>  error:
>  	free_trace_kprobe(tk);
> @@ -1849,6 +1896,7 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
>  		return;
>  	}
>  
> +	local_event_remove(&tk->devent);
>  	__unregister_trace_kprobe(tk);
>  
>  	free_trace_kprobe(tk);
> @@ -1929,6 +1977,12 @@ static __init int init_kprobe_trace(void)
>  	trace_create_file("kprobe_profile", TRACE_MODE_READ,
>  			  NULL, NULL, &kprobe_profile_ops);
>  
> +#ifdef CONFIG_KPROBE_EVENTS
> +	/* kprobe Local profile */
> +	tracefs_create_file("kprobe_local_profile", TRACE_MODE_READ,
> +			  NULL, NULL, &kprobe_local_profile_ops);
> +#endif /* CONFIG_KPROBE_EVENTS */
> +
>  	setup_boot_kprobe_events();
>  
>  	return 0;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs
  2022-09-06 15:12 ` Steven Rostedt
@ 2022-09-07  8:51   ` Masami Hiramatsu
  2022-09-09  1:26     ` Jianlin Lv
  0 siblings, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2022-09-07  8:51 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Jianlin Lv, corbet, mingo, jianlv, linux-kernel, linux-doc,
	Masami Hiramatsu, Tom Zanussi, bpf

[Adding bpf ML]

On Tue, 6 Sep 2022 11:12:14 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> [ Adding Masami and Tom ]
> 
> On Mon, 25 Jul 2022 06:23:34 +0000
> Jianlin Lv <iecedge@gmail.com> wrote:
> 
> > The private kprobes are not added to the global list dyn_event_list,
> > so there is a missing interface to show probe hit and probe miss.
> > This patch adds a profiling interface to check the number of hits or
> > misses for private kprobes.
> 
> Masami, what do you think of this patch?

I discussed it with BPF people when it was introduced and they didn't
want to show up it on tracefs because it is a private one. I agreed that.

So I think this kind of interface must be managed by BPF subsystem.
Is there any API to manage the BPF probe points in BPF subsystem?

Thank you,

> 
> -- Steve
> 
> > 
> > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > ---
> > v2: update commit message
> > ---
> >  Documentation/trace/kprobetrace.rst |  6 +++-
> >  kernel/trace/trace_dynevent.c       | 20 +++++++++++
> >  kernel/trace/trace_dynevent.h       | 37 ++++++++++++++++++++
> >  kernel/trace/trace_kprobe.c         | 54 +++++++++++++++++++++++++++++
> >  4 files changed, 116 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> > index b175d88f31eb..8815d64dd8a6 100644
> > --- a/Documentation/trace/kprobetrace.rst
> > +++ b/Documentation/trace/kprobetrace.rst
> > @@ -146,7 +146,11 @@ trigger:
> >  Event Profiling
> >  ---------------
> >  You can check the total number of probe hits and probe miss-hits via
> > -/sys/kernel/debug/tracing/kprobe_profile.
> > +/sys/kernel/debug/tracing/kprobe_profile or
> > +/sys/kernel/debug/tracing/kprobe_local_profile.
> > +All kprobe events created by kprobe_events will be added to the global
> > +list, you can get their profiling via kprobe_profile; kprobe_local_profile
> > +shows profiling for private kprobe events created by perf_kprobe pmu.
> >  The first column is event name, the second is the number of probe hits,
> >  the third is the number of probe miss-hits.
> >  
> > diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
> > index 076b447a1b88..70ec99cd9c53 100644
> > --- a/kernel/trace/trace_dynevent.c
> > +++ b/kernel/trace/trace_dynevent.c
> > @@ -181,6 +181,26 @@ static const struct seq_operations dyn_event_seq_op = {
> >  	.show	= dyn_event_seq_show
> >  };
> >  
> > +#ifdef CONFIG_KPROBE_EVENTS
> > +LIST_HEAD(local_event_list);
> > +
> > +void *local_event_seq_start(struct seq_file *m, loff_t *pos)
> > +{
> > +	mutex_lock(&event_mutex);
> > +	return seq_list_start(&local_event_list, *pos);
> > +}
> > +
> > +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
> > +{
> > +	return seq_list_next(v, &local_event_list, pos);
> > +}
> > +
> > +void local_event_seq_stop(struct seq_file *m, void *v)
> > +{
> > +	mutex_unlock(&event_mutex);
> > +}
> > +#endif /* CONFIG_KPROBE_EVENTS */
> > +
> >  /*
> >   * dyn_events_release_all - Release all specific events
> >   * @type:	the dyn_event_operations * which filters releasing events
> > diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
> > index 936477a111d3..e30193470295 100644
> > --- a/kernel/trace/trace_dynevent.h
> > +++ b/kernel/trace/trace_dynevent.h
> > @@ -101,6 +101,43 @@ void dyn_event_seq_stop(struct seq_file *m, void *v);
> >  int dyn_events_release_all(struct dyn_event_operations *type);
> >  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type);
> >  
> > +#ifdef CONFIG_KPROBE_EVENTS
> > +extern struct list_head local_event_list;
> > +
> > +static inline
> > +int local_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
> > +{
> > +	if (!ev || !ops)
> > +		return -EINVAL;
> > +
> > +	INIT_LIST_HEAD(&ev->list);
> > +	ev->ops = ops;
> > +	return 0;
> > +}
> > +
> > +static inline int local_event_add(struct dyn_event *ev)
> > +{
> > +	lockdep_assert_held(&event_mutex);
> > +
> > +	if (!ev || !ev->ops)
> > +		return -EINVAL;
> > +
> > +	list_add_tail(&ev->list, &local_event_list);
> > +	return 0;
> > +}
> > +
> > +static inline void local_event_remove(struct dyn_event *ev)
> > +{
> > +	lockdep_assert_held(&event_mutex);
> > +	list_del_init(&ev->list);
> > +}
> > +
> > +void *local_event_seq_start(struct seq_file *m, loff_t *pos);
> > +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
> > +void local_event_seq_stop(struct seq_file *m, void *v);
> > +
> > +#endif /* CONFIG_KPROBE_EVENTS */
> > +
> >  /*
> >   * for_each_dyn_event	-	iterate over the dyn_event list
> >   * @pos:	the struct dyn_event * to use as a loop cursor
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index a245ea673715..76f500b17b46 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -1213,6 +1213,52 @@ static const struct file_operations kprobe_profile_ops = {
> >  	.release        = seq_release,
> >  };
> >  
> > +#ifdef CONFIG_KPROBE_EVENTS
> > +/* kprobe Local profile  */
> > +static int local_probes_profile_seq_show(struct seq_file *m, void *v)
> > +{
> > +	struct dyn_event *ev = v;
> > +	struct trace_kprobe *tk;
> > +
> > +	if (!is_trace_kprobe(ev))
> > +		return 0;
> > +
> > +	tk = to_trace_kprobe(ev);
> > +	seq_printf(m, "  %-44s %15lu %15lu\n",
> > +		trace_probe_name(&tk->tp),
> > +		trace_kprobe_nhit(tk),
> > +		tk->rp.kp.nmissed);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct seq_operations local_profile_seq_op = {
> > +	.start  = local_event_seq_start,
> > +	.next   = local_event_seq_next,
> > +	.stop   = local_event_seq_stop,
> > +	.show   = local_probes_profile_seq_show
> > +};
> > +
> > +static int local_profile_open(struct inode *inode, struct file *file)
> > +{
> > +	int ret;
> > +
> > +	ret = security_locked_down(LOCKDOWN_TRACEFS);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return seq_open(file, &local_profile_seq_op);
> > +}
> > +
> > +static const struct file_operations kprobe_local_profile_ops = {
> > +	.owner          = THIS_MODULE,
> > +	.open           = local_profile_open,
> > +	.read           = seq_read,
> > +	.llseek         = seq_lseek,
> > +	.release        = seq_release,
> > +};
> > +#endif /* CONFIG_KPROBE_EVENTS */
> > +
> >  /* Kprobe specific fetch functions */
> >  
> >  /* Return the length of string -- including null terminal byte */
> > @@ -1830,6 +1876,7 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> >  	if (ret < 0)
> >  		goto error;
> >  
> > +	local_event_add(&tk->devent);
> >  	return trace_probe_event_call(&tk->tp);
> >  error:
> >  	free_trace_kprobe(tk);
> > @@ -1849,6 +1896,7 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> >  		return;
> >  	}
> >  
> > +	local_event_remove(&tk->devent);
> >  	__unregister_trace_kprobe(tk);
> >  
> >  	free_trace_kprobe(tk);
> > @@ -1929,6 +1977,12 @@ static __init int init_kprobe_trace(void)
> >  	trace_create_file("kprobe_profile", TRACE_MODE_READ,
> >  			  NULL, NULL, &kprobe_profile_ops);
> >  
> > +#ifdef CONFIG_KPROBE_EVENTS
> > +	/* kprobe Local profile */
> > +	tracefs_create_file("kprobe_local_profile", TRACE_MODE_READ,
> > +			  NULL, NULL, &kprobe_local_profile_ops);
> > +#endif /* CONFIG_KPROBE_EVENTS */
> > +
> >  	setup_boot_kprobe_events();
> >  
> >  	return 0;
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs
  2022-09-07  8:51   ` Masami Hiramatsu
@ 2022-09-09  1:26     ` Jianlin Lv
  2022-09-09 11:05       ` Masami Hiramatsu
  0 siblings, 1 reply; 5+ messages in thread
From: Jianlin Lv @ 2022-09-09  1:26 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Steven Rostedt, corbet, mingo, jianlv, linux-kernel, linux-doc,
	Tom Zanussi, bpf

On Wed, Sep 7, 2022 at 4:51 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> [Adding bpf ML]
>
> On Tue, 6 Sep 2022 11:12:14 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> >
> > [ Adding Masami and Tom ]
> >
> > On Mon, 25 Jul 2022 06:23:34 +0000
> > Jianlin Lv <iecedge@gmail.com> wrote:
> >
> > > The private kprobes are not added to the global list dyn_event_list,
> > > so there is a missing interface to show probe hit and probe miss.
> > > This patch adds a profiling interface to check the number of hits or
> > > misses for private kprobes.
> >
> > Masami, what do you think of this patch?
>
> I discussed it with BPF people when it was introduced and they didn't
> want to show up it on tracefs because it is a private one. I agreed that.
>
> So I think this kind of interface must be managed by BPF subsystem.
> Is there any API to manage the BPF probe points in BPF subsystem?
>
> Thank you,

As far as I know, there is no API to manage private kprobes in the BPF
subsystem, so I added a tracefs interface to display private info.

Could you give me some hints about how to implement the current
functionality in the BPF subsystem?

Regards,
Jianlin

>
> >
> > -- Steve
> >
> > >
> > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > ---
> > > v2: update commit message
> > > ---
> > >  Documentation/trace/kprobetrace.rst |  6 +++-
> > >  kernel/trace/trace_dynevent.c       | 20 +++++++++++
> > >  kernel/trace/trace_dynevent.h       | 37 ++++++++++++++++++++
> > >  kernel/trace/trace_kprobe.c         | 54 +++++++++++++++++++++++++++++
> > >  4 files changed, 116 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> > > index b175d88f31eb..8815d64dd8a6 100644
> > > --- a/Documentation/trace/kprobetrace.rst
> > > +++ b/Documentation/trace/kprobetrace.rst
> > > @@ -146,7 +146,11 @@ trigger:
> > >  Event Profiling
> > >  ---------------
> > >  You can check the total number of probe hits and probe miss-hits via
> > > -/sys/kernel/debug/tracing/kprobe_profile.
> > > +/sys/kernel/debug/tracing/kprobe_profile or
> > > +/sys/kernel/debug/tracing/kprobe_local_profile.
> > > +All kprobe events created by kprobe_events will be added to the global
> > > +list, you can get their profiling via kprobe_profile; kprobe_local_profile
> > > +shows profiling for private kprobe events created by perf_kprobe pmu.
> > >  The first column is event name, the second is the number of probe hits,
> > >  the third is the number of probe miss-hits.
> > >
> > > diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
> > > index 076b447a1b88..70ec99cd9c53 100644
> > > --- a/kernel/trace/trace_dynevent.c
> > > +++ b/kernel/trace/trace_dynevent.c
> > > @@ -181,6 +181,26 @@ static const struct seq_operations dyn_event_seq_op = {
> > >     .show   = dyn_event_seq_show
> > >  };
> > >
> > > +#ifdef CONFIG_KPROBE_EVENTS
> > > +LIST_HEAD(local_event_list);
> > > +
> > > +void *local_event_seq_start(struct seq_file *m, loff_t *pos)
> > > +{
> > > +   mutex_lock(&event_mutex);
> > > +   return seq_list_start(&local_event_list, *pos);
> > > +}
> > > +
> > > +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
> > > +{
> > > +   return seq_list_next(v, &local_event_list, pos);
> > > +}
> > > +
> > > +void local_event_seq_stop(struct seq_file *m, void *v)
> > > +{
> > > +   mutex_unlock(&event_mutex);
> > > +}
> > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > +
> > >  /*
> > >   * dyn_events_release_all - Release all specific events
> > >   * @type:  the dyn_event_operations * which filters releasing events
> > > diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
> > > index 936477a111d3..e30193470295 100644
> > > --- a/kernel/trace/trace_dynevent.h
> > > +++ b/kernel/trace/trace_dynevent.h
> > > @@ -101,6 +101,43 @@ void dyn_event_seq_stop(struct seq_file *m, void *v);
> > >  int dyn_events_release_all(struct dyn_event_operations *type);
> > >  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type);
> > >
> > > +#ifdef CONFIG_KPROBE_EVENTS
> > > +extern struct list_head local_event_list;
> > > +
> > > +static inline
> > > +int local_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
> > > +{
> > > +   if (!ev || !ops)
> > > +           return -EINVAL;
> > > +
> > > +   INIT_LIST_HEAD(&ev->list);
> > > +   ev->ops = ops;
> > > +   return 0;
> > > +}
> > > +
> > > +static inline int local_event_add(struct dyn_event *ev)
> > > +{
> > > +   lockdep_assert_held(&event_mutex);
> > > +
> > > +   if (!ev || !ev->ops)
> > > +           return -EINVAL;
> > > +
> > > +   list_add_tail(&ev->list, &local_event_list);
> > > +   return 0;
> > > +}
> > > +
> > > +static inline void local_event_remove(struct dyn_event *ev)
> > > +{
> > > +   lockdep_assert_held(&event_mutex);
> > > +   list_del_init(&ev->list);
> > > +}
> > > +
> > > +void *local_event_seq_start(struct seq_file *m, loff_t *pos);
> > > +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
> > > +void local_event_seq_stop(struct seq_file *m, void *v);
> > > +
> > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > +
> > >  /*
> > >   * for_each_dyn_event      -       iterate over the dyn_event list
> > >   * @pos:   the struct dyn_event * to use as a loop cursor
> > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > index a245ea673715..76f500b17b46 100644
> > > --- a/kernel/trace/trace_kprobe.c
> > > +++ b/kernel/trace/trace_kprobe.c
> > > @@ -1213,6 +1213,52 @@ static const struct file_operations kprobe_profile_ops = {
> > >     .release        = seq_release,
> > >  };
> > >
> > > +#ifdef CONFIG_KPROBE_EVENTS
> > > +/* kprobe Local profile  */
> > > +static int local_probes_profile_seq_show(struct seq_file *m, void *v)
> > > +{
> > > +   struct dyn_event *ev = v;
> > > +   struct trace_kprobe *tk;
> > > +
> > > +   if (!is_trace_kprobe(ev))
> > > +           return 0;
> > > +
> > > +   tk = to_trace_kprobe(ev);
> > > +   seq_printf(m, "  %-44s %15lu %15lu\n",
> > > +           trace_probe_name(&tk->tp),
> > > +           trace_kprobe_nhit(tk),
> > > +           tk->rp.kp.nmissed);
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static const struct seq_operations local_profile_seq_op = {
> > > +   .start  = local_event_seq_start,
> > > +   .next   = local_event_seq_next,
> > > +   .stop   = local_event_seq_stop,
> > > +   .show   = local_probes_profile_seq_show
> > > +};
> > > +
> > > +static int local_profile_open(struct inode *inode, struct file *file)
> > > +{
> > > +   int ret;
> > > +
> > > +   ret = security_locked_down(LOCKDOWN_TRACEFS);
> > > +   if (ret)
> > > +           return ret;
> > > +
> > > +   return seq_open(file, &local_profile_seq_op);
> > > +}
> > > +
> > > +static const struct file_operations kprobe_local_profile_ops = {
> > > +   .owner          = THIS_MODULE,
> > > +   .open           = local_profile_open,
> > > +   .read           = seq_read,
> > > +   .llseek         = seq_lseek,
> > > +   .release        = seq_release,
> > > +};
> > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > +
> > >  /* Kprobe specific fetch functions */
> > >
> > >  /* Return the length of string -- including null terminal byte */
> > > @@ -1830,6 +1876,7 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > >     if (ret < 0)
> > >             goto error;
> > >
> > > +   local_event_add(&tk->devent);
> > >     return trace_probe_event_call(&tk->tp);
> > >  error:
> > >     free_trace_kprobe(tk);
> > > @@ -1849,6 +1896,7 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> > >             return;
> > >     }
> > >
> > > +   local_event_remove(&tk->devent);
> > >     __unregister_trace_kprobe(tk);
> > >
> > >     free_trace_kprobe(tk);
> > > @@ -1929,6 +1977,12 @@ static __init int init_kprobe_trace(void)
> > >     trace_create_file("kprobe_profile", TRACE_MODE_READ,
> > >                       NULL, NULL, &kprobe_profile_ops);
> > >
> > > +#ifdef CONFIG_KPROBE_EVENTS
> > > +   /* kprobe Local profile */
> > > +   tracefs_create_file("kprobe_local_profile", TRACE_MODE_READ,
> > > +                     NULL, NULL, &kprobe_local_profile_ops);
> > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > +
> > >     setup_boot_kprobe_events();
> > >
> > >     return 0;
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs
  2022-09-09  1:26     ` Jianlin Lv
@ 2022-09-09 11:05       ` Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2022-09-09 11:05 UTC (permalink / raw)
  To: Jianlin Lv
  Cc: Steven Rostedt, corbet, mingo, jianlv, linux-kernel, linux-doc,
	Tom Zanussi, bpf

On Fri, 9 Sep 2022 09:26:23 +0800
Jianlin Lv <iecedge@gmail.com> wrote:

> On Wed, Sep 7, 2022 at 4:51 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > [Adding bpf ML]
> >
> > On Tue, 6 Sep 2022 11:12:14 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > >
> > > [ Adding Masami and Tom ]
> > >
> > > On Mon, 25 Jul 2022 06:23:34 +0000
> > > Jianlin Lv <iecedge@gmail.com> wrote:
> > >
> > > > The private kprobes are not added to the global list dyn_event_list,
> > > > so there is a missing interface to show probe hit and probe miss.
> > > > This patch adds a profiling interface to check the number of hits or
> > > > misses for private kprobes.
> > >
> > > Masami, what do you think of this patch?
> >
> > I discussed it with BPF people when it was introduced and they didn't
> > want to show up it on tracefs because it is a private one. I agreed that.
> >
> > So I think this kind of interface must be managed by BPF subsystem.
> > Is there any API to manage the BPF probe points in BPF subsystem?
> >
> > Thank you,
> 
> As far as I know, there is no API to manage private kprobes in the BPF
> subsystem, so I added a tracefs interface to display private info.
> 
> Could you give me some hints about how to implement the current
> functionality in the BPF subsystem?

There is tools/bpf/bpftool, I think that is the best tool to implement
such feature. If hitcount is shown with the BPF program, it should be
more useful.

Thank you,

> 
> Regards,
> Jianlin
> 
> >
> > >
> > > -- Steve
> > >
> > > >
> > > > Signed-off-by: Jianlin Lv <iecedge@gmail.com>
> > > > ---
> > > > v2: update commit message
> > > > ---
> > > >  Documentation/trace/kprobetrace.rst |  6 +++-
> > > >  kernel/trace/trace_dynevent.c       | 20 +++++++++++
> > > >  kernel/trace/trace_dynevent.h       | 37 ++++++++++++++++++++
> > > >  kernel/trace/trace_kprobe.c         | 54 +++++++++++++++++++++++++++++
> > > >  4 files changed, 116 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
> > > > index b175d88f31eb..8815d64dd8a6 100644
> > > > --- a/Documentation/trace/kprobetrace.rst
> > > > +++ b/Documentation/trace/kprobetrace.rst
> > > > @@ -146,7 +146,11 @@ trigger:
> > > >  Event Profiling
> > > >  ---------------
> > > >  You can check the total number of probe hits and probe miss-hits via
> > > > -/sys/kernel/debug/tracing/kprobe_profile.
> > > > +/sys/kernel/debug/tracing/kprobe_profile or
> > > > +/sys/kernel/debug/tracing/kprobe_local_profile.
> > > > +All kprobe events created by kprobe_events will be added to the global
> > > > +list, you can get their profiling via kprobe_profile; kprobe_local_profile
> > > > +shows profiling for private kprobe events created by perf_kprobe pmu.
> > > >  The first column is event name, the second is the number of probe hits,
> > > >  the third is the number of probe miss-hits.
> > > >
> > > > diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
> > > > index 076b447a1b88..70ec99cd9c53 100644
> > > > --- a/kernel/trace/trace_dynevent.c
> > > > +++ b/kernel/trace/trace_dynevent.c
> > > > @@ -181,6 +181,26 @@ static const struct seq_operations dyn_event_seq_op = {
> > > >     .show   = dyn_event_seq_show
> > > >  };
> > > >
> > > > +#ifdef CONFIG_KPROBE_EVENTS
> > > > +LIST_HEAD(local_event_list);
> > > > +
> > > > +void *local_event_seq_start(struct seq_file *m, loff_t *pos)
> > > > +{
> > > > +   mutex_lock(&event_mutex);
> > > > +   return seq_list_start(&local_event_list, *pos);
> > > > +}
> > > > +
> > > > +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
> > > > +{
> > > > +   return seq_list_next(v, &local_event_list, pos);
> > > > +}
> > > > +
> > > > +void local_event_seq_stop(struct seq_file *m, void *v)
> > > > +{
> > > > +   mutex_unlock(&event_mutex);
> > > > +}
> > > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > > +
> > > >  /*
> > > >   * dyn_events_release_all - Release all specific events
> > > >   * @type:  the dyn_event_operations * which filters releasing events
> > > > diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
> > > > index 936477a111d3..e30193470295 100644
> > > > --- a/kernel/trace/trace_dynevent.h
> > > > +++ b/kernel/trace/trace_dynevent.h
> > > > @@ -101,6 +101,43 @@ void dyn_event_seq_stop(struct seq_file *m, void *v);
> > > >  int dyn_events_release_all(struct dyn_event_operations *type);
> > > >  int dyn_event_release(const char *raw_command, struct dyn_event_operations *type);
> > > >
> > > > +#ifdef CONFIG_KPROBE_EVENTS
> > > > +extern struct list_head local_event_list;
> > > > +
> > > > +static inline
> > > > +int local_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
> > > > +{
> > > > +   if (!ev || !ops)
> > > > +           return -EINVAL;
> > > > +
> > > > +   INIT_LIST_HEAD(&ev->list);
> > > > +   ev->ops = ops;
> > > > +   return 0;
> > > > +}
> > > > +
> > > > +static inline int local_event_add(struct dyn_event *ev)
> > > > +{
> > > > +   lockdep_assert_held(&event_mutex);
> > > > +
> > > > +   if (!ev || !ev->ops)
> > > > +           return -EINVAL;
> > > > +
> > > > +   list_add_tail(&ev->list, &local_event_list);
> > > > +   return 0;
> > > > +}
> > > > +
> > > > +static inline void local_event_remove(struct dyn_event *ev)
> > > > +{
> > > > +   lockdep_assert_held(&event_mutex);
> > > > +   list_del_init(&ev->list);
> > > > +}
> > > > +
> > > > +void *local_event_seq_start(struct seq_file *m, loff_t *pos);
> > > > +void *local_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
> > > > +void local_event_seq_stop(struct seq_file *m, void *v);
> > > > +
> > > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > > +
> > > >  /*
> > > >   * for_each_dyn_event      -       iterate over the dyn_event list
> > > >   * @pos:   the struct dyn_event * to use as a loop cursor
> > > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > > index a245ea673715..76f500b17b46 100644
> > > > --- a/kernel/trace/trace_kprobe.c
> > > > +++ b/kernel/trace/trace_kprobe.c
> > > > @@ -1213,6 +1213,52 @@ static const struct file_operations kprobe_profile_ops = {
> > > >     .release        = seq_release,
> > > >  };
> > > >
> > > > +#ifdef CONFIG_KPROBE_EVENTS
> > > > +/* kprobe Local profile  */
> > > > +static int local_probes_profile_seq_show(struct seq_file *m, void *v)
> > > > +{
> > > > +   struct dyn_event *ev = v;
> > > > +   struct trace_kprobe *tk;
> > > > +
> > > > +   if (!is_trace_kprobe(ev))
> > > > +           return 0;
> > > > +
> > > > +   tk = to_trace_kprobe(ev);
> > > > +   seq_printf(m, "  %-44s %15lu %15lu\n",
> > > > +           trace_probe_name(&tk->tp),
> > > > +           trace_kprobe_nhit(tk),
> > > > +           tk->rp.kp.nmissed);
> > > > +
> > > > +   return 0;
> > > > +}
> > > > +
> > > > +static const struct seq_operations local_profile_seq_op = {
> > > > +   .start  = local_event_seq_start,
> > > > +   .next   = local_event_seq_next,
> > > > +   .stop   = local_event_seq_stop,
> > > > +   .show   = local_probes_profile_seq_show
> > > > +};
> > > > +
> > > > +static int local_profile_open(struct inode *inode, struct file *file)
> > > > +{
> > > > +   int ret;
> > > > +
> > > > +   ret = security_locked_down(LOCKDOWN_TRACEFS);
> > > > +   if (ret)
> > > > +           return ret;
> > > > +
> > > > +   return seq_open(file, &local_profile_seq_op);
> > > > +}
> > > > +
> > > > +static const struct file_operations kprobe_local_profile_ops = {
> > > > +   .owner          = THIS_MODULE,
> > > > +   .open           = local_profile_open,
> > > > +   .read           = seq_read,
> > > > +   .llseek         = seq_lseek,
> > > > +   .release        = seq_release,
> > > > +};
> > > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > > +
> > > >  /* Kprobe specific fetch functions */
> > > >
> > > >  /* Return the length of string -- including null terminal byte */
> > > > @@ -1830,6 +1876,7 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
> > > >     if (ret < 0)
> > > >             goto error;
> > > >
> > > > +   local_event_add(&tk->devent);
> > > >     return trace_probe_event_call(&tk->tp);
> > > >  error:
> > > >     free_trace_kprobe(tk);
> > > > @@ -1849,6 +1896,7 @@ void destroy_local_trace_kprobe(struct trace_event_call *event_call)
> > > >             return;
> > > >     }
> > > >
> > > > +   local_event_remove(&tk->devent);
> > > >     __unregister_trace_kprobe(tk);
> > > >
> > > >     free_trace_kprobe(tk);
> > > > @@ -1929,6 +1977,12 @@ static __init int init_kprobe_trace(void)
> > > >     trace_create_file("kprobe_profile", TRACE_MODE_READ,
> > > >                       NULL, NULL, &kprobe_profile_ops);
> > > >
> > > > +#ifdef CONFIG_KPROBE_EVENTS
> > > > +   /* kprobe Local profile */
> > > > +   tracefs_create_file("kprobe_local_profile", TRACE_MODE_READ,
> > > > +                     NULL, NULL, &kprobe_local_profile_ops);
> > > > +#endif /* CONFIG_KPROBE_EVENTS */
> > > > +
> > > >     setup_boot_kprobe_events();
> > > >
> > > >     return 0;
> > >
> >
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-09 11:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-25  6:23 [PATCH v2] tracing/kprobes: Add method to display private kprobes in tracefs Jianlin Lv
2022-09-06 15:12 ` Steven Rostedt
2022-09-07  8:51   ` Masami Hiramatsu
2022-09-09  1:26     ` Jianlin Lv
2022-09-09 11:05       ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).