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

[Song Liu <songliubraving@fb.com>]

+ Alexander

On Nov 15, 2019, at 3:55 PM, Song Liu <songliubraving@fb.com> wrote:
> 
> This patch tries to enable PMU sharing. When multiple perf_events are
> counting the same metric, they can share the hardware PMU counter. We
> call these events as "compatible events".
> 
> The PMU sharing are limited to events within the same perf_event_context
> (ctx). When a event is installed or enabled, search the ctx for compatible
> events. This is implemented in perf_event_setup_dup(). One of these
> compatible events are picked as the master (stored in event->dup_master).
> Similarly, when the event is removed or disabled, perf_event_remove_dup()
> is used to clean up sharing.
> 
> A new state PERF_EVENT_STATE_ENABLED is introduced for the master event.
> This state is used when the slave event is ACTIVE, but the master event
> is not.
> 
> On the critical paths (add, del read), sharing PMU counters doesn't
> increase the complexity. Helper functions event_pmu_[add|del|read]() are
> introduced to cover these cases. All these functions have O(1) time
> complexity.
> 

[...]

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index aec8dba2bea4..00b1e19e70fd 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -1657,6 +1657,139 @@ perf_event_groups_next(struct perf_event *event)
> 		event = rb_entry_safe(rb_next(&event->group_node),	\
> 				typeof(*event), group_node))
> 
> +static inline bool perf_event_can_share(struct perf_event *event)
> +{
> +	/* only share hardware counting events */
> +	return !is_software_event(event) && !is_sampling_event(event);
> +}
> +
> +/*
> + * Returns whether the two events can share a PMU counter.
> + *
> + * Note: This function does NOT check perf_event_can_share() for
> + * the two events, they should be checked before this function
> + */
> +static inline bool perf_event_compatible(struct perf_event *event_a,
> +					 struct perf_event *event_b)
> +{
> +	return event_a->attr.type == event_b->attr.type &&
> +		event_a->attr.config == event_b->attr.config &&
> +		event_a->attr.config1 == event_b->attr.config1 &&
> +		event_a->attr.config2 == event_b->attr.config2;

In a discussion on the IRC, Alexander highlighted that we need to 
compare event->hw.config as well (or something equivalent). I will 
fix this in the next version (will need cover a few different 
cases). 

Hi Peter, could you please share your feedback on this? Does the 
overall direction look right?

Thanks,
Song