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

[Peter Zijlstra]

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);
> +}