Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1186698 > unrolled thread

[RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter kaixu xia <xiakaixu@huawei.com> - 2015-07-17 12:50 +0200
    Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 13:10 +0200
      Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 13:40 +0200
      Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 13:40 +0200
        Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter "Wangnan (F)" <wangnan0@huawei.com> - 2015-07-17 13:50 +0200
          Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 14:00 +0200
            Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 14:00 +0200
              Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter "Wangnan (F)" <wangnan0@huawei.com> - 2015-07-17 14:10 +0200
                Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter "Wangnan (F)" <wangnan0@huawei.com> - 2015-07-17 14:10 +0200
                Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 14:20 +0200
                  Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter "Wangnan (F)" <wangnan0@huawei.com> - 2015-07-17 14:40 +0200
                    Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 14:50 +0200
                      Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 14:50 +0200
                      Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter pi3orama <pi3orama@163.com> - 2015-07-17 15:10 +0200
                        Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter Peter Zijlstra <peterz@infradead.org> - 2015-07-17 15:30 +0200
                          Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter pi3orama <pi3orama@163.com> - 2015-07-17 15:50 +0200
      Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get  the selected hardware PMU conuter "Wangnan (F)" <wangnan0@huawei.com> - 2015-07-17 13:40 +0200

#1186698 — [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

The function bpf_read_pmu() can get the specific map key, convert
the corresponding map value to the pointer to struct perf_event and
return the Hardware PMU counter value.

Signed-off-by: kaixu xia <xiakaixu@huawei.com>
---
 include/linux/bpf.h      |    2 ++
 include/uapi/linux/bpf.h |    2 ++
 kernel/bpf/helpers.c     |   27 +++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |    2 ++
 4 files changed, 33 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 31a93fc..6efff20 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -190,6 +190,8 @@ extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
 extern const struct bpf_func_proto bpf_map_update_elem_proto;
 extern const struct bpf_func_proto bpf_map_delete_elem_proto;
 
+extern const struct bpf_func_proto bpf_read_pmu_proto;
+
 extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
 extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
 extern const struct bpf_func_proto bpf_tail_call_proto;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 47d8516..1431ec6 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -263,6 +263,8 @@ enum bpf_func_id {
 	 * Return: 0 on success
 	 */
 	BPF_FUNC_get_current_comm,
+
+	BPF_FUNC_read_pmu,	/* u64 bpf_read_pmu(&value) */
 	__BPF_FUNC_MAX_ID,
 };
 
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1447ec0..6a0ed1b 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -16,6 +16,7 @@
 #include <linux/ktime.h>
 #include <linux/sched.h>
 #include <linux/uidgid.h>
+#include <linux/perf_event.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -182,3 +183,29 @@ const struct bpf_func_proto bpf_get_current_comm_proto = {
 	.arg1_type	= ARG_PTR_TO_STACK,
 	.arg2_type	= ARG_CONST_STACK_SIZE,
 };
+
+static u64 bpf_read_pmu(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
+{
+	void *value = (void *) (unsigned long) r1;
+	struct perf_event *event;
+	u64 count;
+
+	if (!value || !(*(unsigned long *)value))
+		return 0;
+
+	event = (struct perf_event *)(*(unsigned long *)value);
+
+	if (event->state == PERF_EVENT_STATE_ACTIVE)
+		event->pmu->read(event);
+
+	count = local64_read(&event->count);
+
+	return count;
+}
+
+const struct bpf_func_proto bpf_read_pmu_proto = {
+	.func = bpf_read_pmu,
+	.gpl_only = false,
+	.ret_type = RET_INTEGER,
+	.arg1_type = ARG_PTR_TO_MAP_PERF_EVENT_VALUE,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 88a041a..2343159 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -183,6 +183,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
 		return bpf_get_trace_printk_proto();
 	case BPF_FUNC_get_smp_processor_id:
 		return &bpf_get_smp_processor_id_proto;
+	case BPF_FUNC_read_pmu:
+		return &bpf_read_pmu_proto;
 	default:
 		return NULL;
 	}
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1186713 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 06:43:35PM +0800, kaixu xia wrote:
> The function bpf_read_pmu() can get the specific map key, convert
> the corresponding map value to the pointer to struct perf_event and
> return the Hardware PMU counter value.

Thanks for having me on Cc :/

> Signed-off-by: kaixu xia <xiakaixu@huawei.com>
> ---
> +static u64 bpf_read_pmu(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
> +{
> +	void *value = (void *) (unsigned long) r1;
> +	struct perf_event *event;
> +	u64 count;
> +
> +	if (!value || !(*(unsigned long *)value))
> +		return 0;
> +
> +	event = (struct perf_event *)(*(unsigned long *)value);
> +
> +	if (event->state == PERF_EVENT_STATE_ACTIVE)
> +		event->pmu->read(event);
> +
> +	count = local64_read(&event->count);
> +
> +	return count;
> +}

Hell no, that's way broken.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186728 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 01:05:41PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 06:43:35PM +0800, kaixu xia wrote:
> > The function bpf_read_pmu() can get the specific map key, convert
> > the corresponding map value to the pointer to struct perf_event and
> > return the Hardware PMU counter value.
> 
> Thanks for having me on Cc :/
> 
> > Signed-off-by: kaixu xia <xiakaixu@huawei.com>
> > ---
> > +static u64 bpf_read_pmu(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
> > +{
> > +	void *value = (void *) (unsigned long) r1;
> > +	struct perf_event *event;
> > +	u64 count;
> > +
> > +	if (!value || !(*(unsigned long *)value))
> > +		return 0;
> > +
> > +	event = (struct perf_event *)(*(unsigned long *)value);
> > +
> > +	if (event->state == PERF_EVENT_STATE_ACTIVE)
> > +		event->pmu->read(event);
> > +
> > +	count = local64_read(&event->count);
> > +
> > +	return count;
> > +}
> 
> Hell no, that's way broken.

You want something long these lines..

---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 22 +++++++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 2027809433b3..6e7be7345511 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -661,6 +661,7 @@ extern void perf_pmu_migrate_context(struct pmu *pmu,
 				int src_cpu, int dst_cpu);
 extern u64 perf_event_read_value(struct perf_event *event,
 				 u64 *enabled, u64 *running);
+extern u64 perf_event_read(struct perf_event *event);
 
 
 struct perf_sample_data {
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d3dae3419b99..53521360c13d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3212,15 +3212,31 @@ static inline u64 perf_event_count(struct perf_event *event)
 	return __perf_event_count(event);
 }
 
-static u64 perf_event_read(struct perf_event *event)
+u64 perf_event_read(struct perf_event *event)
 {
 	/*
 	 * If event is enabled and currently active on a CPU, update the
 	 * value in the event structure:
 	 */
 	if (event->state == PERF_EVENT_STATE_ACTIVE) {
-		smp_call_function_single(event->oncpu,
-					 __perf_event_read, event, 1);
+		/*
+		 * If the event is for the current task, its guaranteed that we
+		 * never need the cross cpu call, and therefore can allow this
+		 * to be called with IRQs disabled.
+		 *
+		 * Avoids the warning otherwise generated by
+		 * smp_call_function_single().
+		 */
+		if (event->ctx->task == current) {
+			unsigned long flags;
+
+			local_irq_save(flags);
+			__perf_event_read(event);
+			local_irq_restore(flags);
+		} else {
+			smp_call_function_single(event->oncpu,
+					__perf_event_read, event, 1);
+		}
 	} else if (event->state == PERF_EVENT_STATE_INACTIVE) {
 		struct perf_event_context *ctx = event->ctx;
 		unsigned long flags;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186729 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 07:29:07PM +0800, Wangnan (F) wrote:
> 
> 
> On 2015/7/17 19:05, Peter Zijlstra wrote:
> >On Fri, Jul 17, 2015 at 06:43:35PM +0800, kaixu xia wrote:
> >>The function bpf_read_pmu() can get the specific map key, convert
> >>the corresponding map value to the pointer to struct perf_event and
> >>return the Hardware PMU counter value.
> >Thanks for having me on Cc :/
> >
> >>Signed-off-by: kaixu xia <xiakaixu@huawei.com>
> >>---
> >>+static u64 bpf_read_pmu(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
> >>+{
> >>+	void *value = (void *) (unsigned long) r1;
> >>+	struct perf_event *event;
> >>+	u64 count;
> >>+
> >>+	if (!value || !(*(unsigned long *)value))
> >>+		return 0;
> >>+
> >>+	event = (struct perf_event *)(*(unsigned long *)value);
> >>+
> >>+	if (event->state == PERF_EVENT_STATE_ACTIVE)
> >>+		event->pmu->read(event);
> >>+
> >>+	count = local64_read(&event->count);
> >>+
> >>+	return count;
> >>+}
> >Hell no, that's way broken.
> What about calling perf_event_read_value() then?

Depends on what all you need, if you need full perf events to work then
yes perf_event_read_value() is your only option.

But note that that requires scheduling, so you cannot actually use it
for tracing purposes etc..
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186734 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On 2015/7/17 19:39, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 07:29:07PM +0800, Wangnan (F) wrote:
>>
>> On 2015/7/17 19:05, Peter Zijlstra wrote:
>>> On Fri, Jul 17, 2015 at 06:43:35PM +0800, kaixu xia wrote:
>>>> The function bpf_read_pmu() can get the specific map key, convert
>>>> the corresponding map value to the pointer to struct perf_event and
>>>> return the Hardware PMU counter value.
>>> Thanks for having me on Cc :/
>>>
>>>> Signed-off-by: kaixu xia <xiakaixu@huawei.com>
>>>> ---
>>>> +static u64 bpf_read_pmu(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
>>>> +{
>>>> +	void *value = (void *) (unsigned long) r1;
>>>> +	struct perf_event *event;
>>>> +	u64 count;
>>>> +
>>>> +	if (!value || !(*(unsigned long *)value))
>>>> +		return 0;
>>>> +
>>>> +	event = (struct perf_event *)(*(unsigned long *)value);
>>>> +
>>>> +	if (event->state == PERF_EVENT_STATE_ACTIVE)
>>>> +		event->pmu->read(event);
>>>> +
>>>> +	count = local64_read(&event->count);
>>>> +
>>>> +	return count;
>>>> +}
>>> Hell no, that's way broken.
>> What about calling perf_event_read_value() then?
> Depends on what all you need, if you need full perf events to work then
> yes perf_event_read_value() is your only option.
>
> But note that that requires scheduling, so you cannot actually use it
> for tracing purposes etc..
What you mean "full perf events"? Even with your code some event still 
not work?

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186742 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 07:45:02PM +0800, Wangnan (F) wrote:

> >Depends on what all you need, if you need full perf events to work then
> >yes perf_event_read_value() is your only option.
> >
> >But note that that requires scheduling, so you cannot actually use it
> >for tracing purposes etc..

> What you mean "full perf events"? Even with your code some event still not
> work?

The code I posted only works for events that do not have inherit set.
And only works from IRQ/NMI context for events that monitor the current
task or the current CPU (although that needs a little extra code still).

Anything else and it does not work (correctly).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186745 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 01:55:05PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 07:45:02PM +0800, Wangnan (F) wrote:
> 
> > >Depends on what all you need, if you need full perf events to work then
> > >yes perf_event_read_value() is your only option.
> > >
> > >But note that that requires scheduling, so you cannot actually use it
> > >for tracing purposes etc..
> 
> > What you mean "full perf events"? Even with your code some event still not
> > work?
> 
> The code I posted only works for events that do not have inherit set.
> And only works from IRQ/NMI context for events that monitor the current
> task or the current CPU (although that needs a little extra code still).
> 
> Anything else and it does not work (correctly).

Scratch that from NMI, for that to work we need more magic still.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186751 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On 2015/7/17 19:56, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 01:55:05PM +0200, Peter Zijlstra wrote:
>> On Fri, Jul 17, 2015 at 07:45:02PM +0800, Wangnan (F) wrote:
>>
>>>> Depends on what all you need, if you need full perf events to work then
>>>> yes perf_event_read_value() is your only option.
>>>>
>>>> But note that that requires scheduling, so you cannot actually use it
>>>> for tracing purposes etc..
>>> What you mean "full perf events"? Even with your code some event still not
>>> work?
>> The code I posted only works for events that do not have inherit set.
>> And only works from IRQ/NMI context for events that monitor the current
>> task or the current CPU (although that needs a little extra code still).
>>
>> Anything else and it does not work (correctly).
> Scratch that from NMI, for that to work we need more magic still.
The scheduling you said is caused by

mutex_lock(&event->child_mutex)

right?

What about replacing it to mutex_trylock() and simply return an error
if it read from a BPF program?

Thank you.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186759 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On 2015/7/17 20:01, Wangnan (F) wrote:
>
>
> On 2015/7/17 19:56, Peter Zijlstra wrote:
>> On Fri, Jul 17, 2015 at 01:55:05PM +0200, Peter Zijlstra wrote:
>>> On Fri, Jul 17, 2015 at 07:45:02PM +0800, Wangnan (F) wrote:
>>>
>>>>> Depends on what all you need, if you need full perf events to work 
>>>>> then
>>>>> yes perf_event_read_value() is your only option.
>>>>>
>>>>> But note that that requires scheduling, so you cannot actually use it
>>>>> for tracing purposes etc..
>>>> What you mean "full perf events"? Even with your code some event 
>>>> still not
>>>> work?
>>> The code I posted only works for events that do not have inherit set.
>>> And only works from IRQ/NMI context for events that monitor the current
>>> task or the current CPU (although that needs a little extra code 
>>> still).
>>>
>>> Anything else and it does not work (correctly).
>> Scratch that from NMI, for that to work we need more magic still.
> The scheduling you said is caused by
>
> mutex_lock(&event->child_mutex)
>
> right?
>
> What about replacing it to mutex_trylock() and simply return an error
> if it read from a BPF program?
>

Sorry. Should be: "return an error if it doesn't get the lock and the 
caller is a BPF program."

> Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186774 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 08:01:07PM +0800, Wangnan (F) wrote:
> 
> 
> On 2015/7/17 19:56, Peter Zijlstra wrote:
> >On Fri, Jul 17, 2015 at 01:55:05PM +0200, Peter Zijlstra wrote:
> >>On Fri, Jul 17, 2015 at 07:45:02PM +0800, Wangnan (F) wrote:
> >>
> >>>>Depends on what all you need, if you need full perf events to work then
> >>>>yes perf_event_read_value() is your only option.
> >>>>
> >>>>But note that that requires scheduling, so you cannot actually use it
> >>>>for tracing purposes etc..
> >>>What you mean "full perf events"? Even with your code some event still not
> >>>work?
> >>The code I posted only works for events that do not have inherit set.
> >>And only works from IRQ/NMI context for events that monitor the current
> >>task or the current CPU (although that needs a little extra code still).
> >>
> >>Anything else and it does not work (correctly).
> >Scratch that from NMI, for that to work we need more magic still.
> The scheduling you said is caused by
> 
> mutex_lock(&event->child_mutex)
> 
> right?
> 
> What about replacing it to mutex_trylock() and simply return an error
> if it read from a BPF program?

That is vile and unreliable.

I think you really want to put very strict limits on what kind of events
you accept, or create the events yourself.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186785 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On 2015/7/17 20:18, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 08:01:07PM +0800, Wangnan (F) wrote:
>>
>> On 2015/7/17 19:56, Peter Zijlstra wrote:
>>> On Fri, Jul 17, 2015 at 01:55:05PM +0200, Peter Zijlstra wrote:
>>>> On Fri, Jul 17, 2015 at 07:45:02PM +0800, Wangnan (F) wrote:
>>>>
>>>>>> Depends on what all you need, if you need full perf events to work then
>>>>>> yes perf_event_read_value() is your only option.
>>>>>>
>>>>>> But note that that requires scheduling, so you cannot actually use it
>>>>>> for tracing purposes etc..
>>>>> What you mean "full perf events"? Even with your code some event still not
>>>>> work?
>>>> The code I posted only works for events that do not have inherit set.
>>>> And only works from IRQ/NMI context for events that monitor the current
>>>> task or the current CPU (although that needs a little extra code still).
>>>>
>>>> Anything else and it does not work (correctly).
>>> Scratch that from NMI, for that to work we need more magic still.
>> The scheduling you said is caused by
>>
>> mutex_lock(&event->child_mutex)
>>
>> right?
>>
>> What about replacing it to mutex_trylock() and simply return an error
>> if it read from a BPF program?
> That is vile and unreliable.
>
> I think you really want to put very strict limits on what kind of events
> you accept, or create the events yourself.
>
I think we can check the limitation in BPF program. What about this:

  event must on current CPU or must be on current process. If not,
  bpf_read_pmu() should simply return an error.

With current design it is easy to implement, and users can still control
it through bpf map.

But what if we really want cross-cpu PMU accessing? Impossible?

Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186791 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 08:27:43PM +0800, Wangnan (F) wrote:
> I think we can check the limitation in BPF program.

You typically do not want to rely on your program for correctness.

> What about this:
> 
>  event must on current CPU or must be on current process. If not,
>  bpf_read_pmu() should simply return an error.

OK, that's workable. That enforces the constraints outside of the
program itself.

> With current design it is easy to implement, and users can still control
> it through bpf map.
> 
> But what if we really want cross-cpu PMU accessing? Impossible?

Under the assumption that the eBPF program is called from tracing, and
therefore from any context (task, softirq, irq and nmi), yes impossible.

You cannot do (synchronous) IPIs from either IRQ context or with IRQs
disabled. And both are valid trace contexts.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186793 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 02:45:38PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 08:27:43PM +0800, Wangnan (F) wrote:

> >  event must on current CPU or must be on current process. If not,
> >  bpf_read_pmu() should simply return an error.

I would further restrict by saying !inherit (and maybe even !sample).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186806

发自我的 iPhone

> 在 2015年7月17日，下午8:45，Peter Zijlstra <peterz@infradead.org> 写道：
> 
>> On Fri, Jul 17, 2015 at 08:27:43PM +0800, Wangnan (F) wrote:
>> I think we can check the limitation in BPF program.
> 
> You typically do not want to rely on your program for correctness.

Sorry. NOT BPF program. What I want to express is bpf_read_pmu() function which is called from BPF program. User can put anything into the map during preparation but he or she gets only error code if the perf event he or she read from doesn't meet our restriction at runtime.

>> What about this:
>> 
>> event must on current CPU or must be on current process. If not,
>> bpf_read_pmu() should simply return an error.
> 
> OK, that's workable. That enforces the constraints outside of the
> program itself.
> 
>> With current design it is easy to implement, and users can still control
>> it through bpf map.
>> 
>> But what if we really want cross-cpu PMU accessing? Impossible?
> 
> Under the assumption that the eBPF program is called from tracing, and
> therefore from any context (task, softirq, irq and nmi), yes impossible.
> 
> You cannot do (synchronous) IPIs from either IRQ context or with IRQs
> disabled. And both are valid trace contexts.

What about software perf event? For example, tracepoints?

Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186818 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On Fri, Jul 17, 2015 at 08:57:00PM +0800, pi3orama wrote:
> >> But what if we really want cross-cpu PMU accessing? Impossible?
> > 
> > Under the assumption that the eBPF program is called from tracing, and
> > therefore from any context (task, softirq, irq and nmi), yes impossible.
> > 
> > You cannot do (synchronous) IPIs from either IRQ context or with IRQs
> > disabled. And both are valid trace contexts.
> 
> What about software perf event? For example, tracepoints?

Some of them, tracepoints would work. So you could exempt
TYPE_TRACEPOINT, but I would suggest starting as constrained as possible
and relaxing when we really need/want.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186837

发自我的 iPhone

> 在 2015年7月17日，下午9:26，Peter Zijlstra <peterz@infradead.org> 写道：
> 
> On Fri, Jul 17, 2015 at 08:57:00PM +0800, pi3orama wrote:
>>>> But what if we really want cross-cpu PMU accessing? Impossible?
>>> 
>>> Under the assumption that the eBPF program is called from tracing, and
>>> therefore from any context (task, softirq, irq and nmi), yes impossible.
>>> 
>>> You cannot do (synchronous) IPIs from either IRQ context or with IRQs
>>> disabled. And both are valid trace contexts.
>> 
>> What about software perf event? For example, tracepoints?
> 
> Some of them, tracepoints would work. So you could exempt
> TYPE_TRACEPOINT, but I would suggest starting as constrained as possible
> and relaxing when we really need/want.
> 
> 

Thanks to your advise. We will follow them in v2. Do you have further comment on other part of this patch set?

Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1186730 — Re: [RFC PATCH 5/6] bpf: Implement function bpf_read_pmu() that get the selected hardware PMU conuter

On 2015/7/17 19:05, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 06:43:35PM +0800, kaixu xia wrote:
>> The function bpf_read_pmu() can get the specific map key, convert
>> the corresponding map value to the pointer to struct perf_event and
>> return the Hardware PMU counter value.
> Thanks for having me on Cc :/
>
>> Signed-off-by: kaixu xia <xiakaixu@huawei.com>
>> ---
>> +static u64 bpf_read_pmu(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
>> +{
>> +	void *value = (void *) (unsigned long) r1;
>> +	struct perf_event *event;
>> +	u64 count;
>> +
>> +	if (!value || !(*(unsigned long *)value))
>> +		return 0;
>> +
>> +	event = (struct perf_event *)(*(unsigned long *)value);
>> +
>> +	if (event->state == PERF_EVENT_STATE_ACTIVE)
>> +		event->pmu->read(event);
>> +
>> +	count = local64_read(&event->count);
>> +
>> +	return count;
>> +}
> Hell no, that's way broken.
What about calling perf_event_read_value() then?

   ...
   struct perf_event_context *ctx;
   u64 enabled, u64 running

   ctx = perf_event_ctx_lock(event);
   if (!event->state == PERF_EVENT_STATE_ERROR) {
     count = perf_event_read_value(event, &enable, &running);
   }
   perf_event_ctx_unlock(event, ctx);
   ...

Code is from perf_read().

Thank you.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web