Re: [PATCH v8] perf: Sharing PMU counters across compatible events

Peter Zijlstra
2019-12-12T13:49:57Z

On Fri, Dec 06, 2019 at 04:24:47PM -0800, Song Liu wrote:

> @@ -750,6 +752,16 @@ struct perf_event {
> void *security;
> #endif
> struct list_head sb_list;
> +
> + /* for PMU sharing */
> + struct perf_event *dup_master;
> + /* check event_sync_dup_count() for the use of dup_base_* */
> + u64 dup_base_count;
> + u64 dup_base_child_count;
> + /* when this event is master, read from master*count */
> + local64_t master_count;
> + atomic64_t master_child_count;
> + int dup_active_count;
> #endif /* CONFIG_PERF_EVENTS */
> };

> +/* PMU sharing aware version of event->pmu->add() */
> +static int event_pmu_add(struct perf_event *event,
> + struct perf_event_context *ctx)
> +{
> + struct perf_event *master;
> + int ret;
> +
> + /* no sharing, just do event->pmu->add() */
> + if (!event->dup_master)
> + return event->pmu->add(event, PERF_EF_START);

Possibly we should look at the location of perf_event::dup_master to be
in a hot cacheline. Because I'm thinking you just added a guaranteed
miss here.

> +
> + master = event->dup_master;
> +
> + if (!master->dup_active_count) {
> + ret = event->pmu->add(master, PERF_EF_START);
> + if (ret)
> + return ret;
> +
> + if (master != event)
> + perf_event_set_state(master, PERF_EVENT_STATE_ENABLED);
> + }
> +
> + master->dup_active_count++;
> + master->pmu->read(master);
> + event->dup_base_count = local64_read(&master->count);
> + event->dup_base_child_count = atomic64_read(&master->child_count);
> + return 0;
> +}

> +/* PMU sharing aware version of event->pmu->del() */
> +static void event_pmu_del(struct perf_event *event,
> + struct perf_event_context *ctx)
> +{
> + struct perf_event *master;
> +
> + if (event->dup_master == NULL) {
> + event->pmu->del(event, 0);
> + return;
> + }

How about you write it exactly like the add version:

if (!event->dup_master)
return event->pmu->del(event, 0);

?

> +
> + master = event->dup_master;
> + event_sync_dup_count(event, master);
> + if (--master->dup_active_count == 0) {
> + event->pmu->del(master, 0);
> + perf_event_set_state(master, PERF_EVENT_STATE_INACTIVE);
> + } else if (master == event) {
> + perf_event_set_state(master, PERF_EVENT_STATE_ENABLED);
> + }
> +}
> +
> +/* PMU sharing aware version of event->pmu->read() */
> +static void event_pmu_read(struct perf_event *event)
> +{
> + if (event->dup_master == NULL) {
> + event->pmu->read(event);
> + return;
> + }

And here too.

> + event_sync_dup_count(event, event->dup_master);
> +}