On Tue, 24 Oct 2023, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> wrote:
> Hi Jani,
>
> On Mon, Oct 23, 2023 at 06:02:55PM +0300, Jani Nikula wrote:
>> It's tedious to duplicate the container_of() everywhere. Add a helper.
>>
>> Also do the logical steps of first getting from struct perf_event to
>> struct i915_pmu, and then from struct i915_pmu to struct
>> drm_i915_private if needed, instead of perf_event->i915->pmu. Not all
>> places even need the i915 pointer.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_pmu.c | 45 +++++++++++++++------------------
>> 1 file changed, 20 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
>> index dcae2fcd8d36..d45b40ec6d47 100644
>> --- a/drivers/gpu/drm/i915/i915_pmu.c
>> +++ b/drivers/gpu/drm/i915/i915_pmu.c
>> @@ -31,6 +31,11 @@
>> static cpumask_t i915_pmu_cpumask;
>> static unsigned int i915_pmu_target_cpu = -1;
>>
>> +static struct i915_pmu *event_to_pmu(struct perf_event *event)
>
> I would call it perfevent (or perf_event), event is too generic.
> We have other kind of events, too.
Fair enough.
>
>> +{
>> + return container_of(event->pmu, struct i915_pmu, base);
>> +}
>> +
>> static struct drm_i915_private *pmu_to_i915(struct i915_pmu *pmu)
>> {
>> return container_of(pmu, struct drm_i915_private, pmu);
>> @@ -510,8 +515,8 @@ static enum hrtimer_restart i915_sample(struct hrtimer *hrtimer)
>>
>> static void i915_pmu_event_destroy(struct perf_event *event)
>> {
>> - struct drm_i915_private *i915 =
>> - container_of(event->pmu, typeof(*i915), pmu.base);
>> + struct i915_pmu *pmu = event_to_pmu(event);
>> + struct drm_i915_private *i915 = pmu_to_i915(pmu);
>
> perf_event_to_i915() ?
Nah. Most places that need i915 also need pmu. Feels a bit much to add a
helper to combine two helpers.
Thanks for the review.
BR,
Jani.
>
> Andi
--
Jani Nikula, Intel
