lkml.org 
[lkml]   [2010]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH 8/8] perf: Rework the PMU methods
On Wed, Jun 16, 2010 at 06:00:35PM +0200, Peter Zijlstra wrote:
> -static void x86_pmu_stop(struct perf_event *event)
> +static void x86_pmu_stop(struct perf_event *event, int flags)
> {
> - struct cpu_hw_events *cpuc = &__get_cpu_var(cpu_hw_events);
> - struct hw_perf_event *hwc = &event->hw;
> - int idx = hwc->idx;
> -
> if (!__test_and_clear_bit(idx, cpuc->active_mask))
> - return;



Do you still need active_mask now that you have HES_STOPPED?



> @@ -4069,6 +4051,9 @@ static int perf_swevent_match(struct per
> struct perf_sample_data *data,
> struct pt_regs *regs)
> {
> + if (event->hw.state)
> + return 0;
> +
> if (event->attr.type != type)
> return 0;
>
> @@ -4211,7 +4196,7 @@ static void perf_swevent_read(struct per
> {
> }
>
> -static int perf_swevent_enable(struct perf_event *event)
> +static int perf_swevent_add(struct perf_event *event, int flags)
> {
> struct hw_perf_event *hwc = &event->hw;
> struct perf_cpu_context *cpuctx;
> @@ -4224,6 +4209,8 @@ static int perf_swevent_enable(struct pe
> perf_swevent_set_period(event);
> }
>
> + hwc->state = !(flags & PERF_EF_START);
> +
> head = find_swevent_head(cpuctx, event);
> if (WARN_ON_ONCE(!head))
> return -EINVAL;
> @@ -4233,13 +4220,19 @@ static int perf_swevent_enable(struct pe
> return 0;
> }
>
> -static void perf_swevent_disable(struct perf_event *event)
> +static void perf_swevent_del(struct perf_event *event, int flags)
> {
> hlist_del_rcu(&event->hlist_entry);
> }
>
> -static void perf_swevent_void(struct perf_event *event)
> +static void perf_swevent_start(struct perf_event *event, int flags)
> +{
> + event->hw.state = 0;
> +}
> +
> +static void perf_swevent_stop(struct perf_event *event, int flags)
> {
> + event->hw.state = 1;
> }


So, instead of doing this and add yet another check in the fast path,
what about just playing with the hlist insertion and deletion?

And if we have PERF_EF_RELOAD in start or PERF_EF_UPDATE in stop,
we simply do nothing, as we know it's just about updating the counter
or reset the interrupt, things that software events just don't care.

And in ->add, you can also do nothing if PERF_EF_START.

It would be nice to have a PERF_EF_STOP as well in ->del, so that
each pmu don't need to maintain an internal state.
If we assume the generic code will never imbalance add/start/stop/del,
or call start after add(PERF_EF_START), or things like this, pmus
like this don't need to ever touch event->hw.state. It's only
necessary for hardware events that call their start/stop internally.


> static inline void perf_tp_register(void)
> @@ -4546,7 +4537,7 @@ void perf_bp_event(struct perf_event *bp
>
> perf_sample_data_init(&sample, bp->attr.bp_addr);
>
> - if (!perf_exclude_event(bp, regs))
> + if (!bp->hw.state && !perf_exclude_event(bp, regs))
> perf_swevent_add(bp, 1, 1, &sample, regs);



Same thing here, and same for trace events.



> }
> #endif
> @@ -4591,12 +4582,12 @@ static void perf_swevent_start_hrtimer(s
> if (hwc->sample_period) {
> u64 period;
>
> - if (hwc->remaining) {
> - if (hwc->remaining < 0)
> + if (hwc->period_left) {
> + if (hwc->period_left < 0)
> period = 10000;
> else
> - period = hwc->remaining;
> - hwc->remaining = 0;
> + period = hwc->period_left;
> + hwc->period_left = 0;



If remaining can be replaced by period_left, it should probably be done
in another patch.



\
 
 \ /
  Last update: 2010-06-18 06:23    [W:0.126 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site