[PATCH 1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

[PATCH 1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

[Tvrtko Ursulin]

From: Chris Wilson <chris@chris-wilson.co.uk>

Even for static CPU configurations, the hotplug CPU framework is still
used to determine the CPU topology, and is still being used by the perf
event register to check for valid CPUs.

Fixes: b46a33e271ed ("drm/i915/pmu: Expose a PMU interface for perf queries")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index 1071935bfa67..d7acdaf7d9f8 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -40,7 +40,7 @@
 
 #define ENGINE_SAMPLE_BITS (1 << I915_PMU_SAMPLE_BITS)
 
-static cpumask_t i915_pmu_cpumask = CPU_MASK_NONE;
+static cpumask_t i915_pmu_cpumask;
 
 static u8 engine_config_sample(u64 config)
 {
@@ -742,7 +742,6 @@ static const struct attribute_group *i915_pmu_attr_groups[] = {
 	NULL
 };
 
-#ifdef CONFIG_HOTPLUG_CPU
 static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
 {
 	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), node);
@@ -778,11 +777,9 @@ static int i915_pmu_cpu_offline(unsigned int cpu, struct hlist_node *node)
 }
 
 static enum cpuhp_state cpuhp_slot = CPUHP_INVALID;
-#endif
 
 static int i915_pmu_register_cpuhp_state(struct drm_i915_private *i915)
 {
-#ifdef CONFIG_HOTPLUG_CPU
 	enum cpuhp_state slot;
 	int ret;
 
@@ -801,17 +798,14 @@ static int i915_pmu_register_cpuhp_state(struct drm_i915_private *i915)
 	}
 
 	cpuhp_slot = slot;
-#endif
 	return 0;
 }
 
 static void i915_pmu_unregister_cpuhp_state(struct drm_i915_private *i915)
 {
-#ifdef CONFIG_HOTPLUG_CPU
 	WARN_ON(cpuhp_slot == CPUHP_INVALID);
 	WARN_ON(cpuhp_state_remove_instance(cpuhp_slot, &i915->pmu.node));
 	cpuhp_remove_multi_state(cpuhp_slot);
-#endif
 }
 
 void i915_pmu_register(struct drm_i915_private *i915)
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/pmu: Only allow running on a single CPU

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We do two things, both of which are purely to simplify and clarify the
implementation:

1.

Simplify the CPU online callback so it is more obvious that the purpose
there is to set a single CPU mask bit for the first CPU which comes
online. Using cpumask_weight for this reads more obvious than the trick
with cpumask_and_any.

2.

Modify the event init so that events can be created only on a single CPU.

This removes looking at the requested CPU thread siblings, and only allows
creating on the current active CPU.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
---
 drivers/gpu/drm/i915/i915_pmu.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
index d7acdaf7d9f8..d83653c83d6e 100644
--- a/drivers/gpu/drm/i915/i915_pmu.c
+++ b/drivers/gpu/drm/i915/i915_pmu.c
@@ -326,7 +326,7 @@ static int i915_pmu_event_init(struct perf_event *event)
 {
 	struct drm_i915_private *i915 =
 		container_of(event->pmu, typeof(*i915), pmu.base);
-	int cpu, ret;
+	int ret;
 
 	if (event->attr.type != event->pmu->type)
 		return -ENOENT;
@@ -341,9 +341,8 @@ static int i915_pmu_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
-	cpu = cpumask_any_and(&i915_pmu_cpumask,
-			      topology_sibling_cpumask(event->cpu));
-	if (cpu >= nr_cpu_ids)
+	/* only allow running on one cpu at a time */
+	if (!cpumask_test_cpu(event->cpu, &i915_pmu_cpumask))
 		return -ENODEV;
 
 	if (is_engine_event(event)) {
@@ -378,7 +377,6 @@ static int i915_pmu_event_init(struct perf_event *event)
 	if (ret)
 		return ret;
 
-	event->cpu = cpu;
 	if (!event->parent)
 		event->destroy = i915_pmu_event_destroy;
 
@@ -745,13 +743,11 @@ static const struct attribute_group *i915_pmu_attr_groups[] = {
 static int i915_pmu_cpu_online(unsigned int cpu, struct hlist_node *node)
 {
 	struct i915_pmu *pmu = hlist_entry_safe(node, typeof(*pmu), node);
-	unsigned int target;
 
 	GEM_BUG_ON(!pmu->base.event_init);
 
-	target = cpumask_any_and(&i915_pmu_cpumask, &i915_pmu_cpumask);
 	/* Select the first online CPU as a designated reader. */
-	if (target >= nr_cpu_ids)
+	if (!cpumask_weight(&i915_pmu_cpumask))
 		cpumask_set_cpu(cpu, &i915_pmu_cpumask);
 
 	return 0;
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/pmu: Only allow running on a single CPU

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-23 12:34:32)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> We do two things, both of which are purely to simplify and clarify the
> implementation:
> 
> 1.
> 
> Simplify the CPU online callback so it is more obvious that the purpose
> there is to set a single CPU mask bit for the first CPU which comes
> online. Using cpumask_weight for this reads more obvious than the trick
> with cpumask_and_any.
> 
> 2.
> 
> Modify the event init so that events can be created only on a single CPU.
> 
> This removes looking at the requested CPU thread siblings, and only allows
> creating on the current active CPU.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>

Works for me.
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>

As much as I understand this (which is not a lot!),
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/pmu: Only allow running on a single CPU

[Tvrtko Ursulin]

On 23/11/2017 12:50, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2017-11-23 12:34:32)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> We do two things, both of which are purely to simplify and clarify the
>> implementation:
>>
>> 1.
>>
>> Simplify the CPU online callback so it is more obvious that the purpose
>> there is to set a single CPU mask bit for the first CPU which comes
>> online. Using cpumask_weight for this reads more obvious than the trick
>> with cpumask_and_any.
>>
>> 2.
>>
>> Modify the event init so that events can be created only on a single CPU.
>>
>> This removes looking at the requested CPU thread siblings, and only allows
>> creating on the current active CPU.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin@intel.com>
> 
> Works for me.
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> As much as I understand this (which is not a lot!),
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks, now waiting for results!

Your patch (1/2) is the main fix here, without it the cpumask is not 
initialized on !hotplug kernels hence -ENODEV on everything.

This patch is mostly making the code more obvious. I am not sure that 
the addition of rejecting thread siblings actually changes anything. 
Perf stat at least seems to only try to init one event per real core and 
seems to be skipping thread siblings. So it just makes it more obvious 
we really want to only run on a single CPU.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration
URL   : https://patchwork.freedesktop.org/series/34300/
State : success

== Summary ==

Series 34300v1 series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration
https://patchwork.freedesktop.org/api/1.0/series/34300/revisions/1/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:449s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:386s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:537s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:505s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:512s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:501s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:490s
fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:608s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:428s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:266s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:425s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:569s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:467s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:548s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:561s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:515s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:466s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:559s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:420s
Blacklisted hosts:
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:570s
fi-glk-dsi       total:289  pass:132  dwarn:0   dfail:10  fail:2   skip:145 time:309s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:483s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:529s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:480s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:539s

9d399f81694538228a539de9a908cdf9ef00f814 drm-tip: 2017y-11m-23d-12h-28m-12s UTC integration manifest
52921f335e1f drm/i915/pmu: Only allow running on a single CPU
f8a679d0893c drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7258/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration
URL   : https://patchwork.freedesktop.org/series/34300/
State : success

== Summary ==

Test drv_module_reload:
        Subgroup basic-no-display:
                dmesg-warn -> PASS       (shard-snb) fdo#102707 +2
Test kms_flip:
        Subgroup plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368
        Subgroup blt-flip-vs-panning-interruptible:
                dmesg-warn -> PASS       (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623
Test drv_selftest:
        Subgroup mock_sanitycheck:
                pass       -> DMESG-WARN (shard-hsw) fdo#103719

fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#103719 https://bugs.freedesktop.org/show_bug.cgi?id=103719

shard-hsw        total:2667 pass:1533 dwarn:3   dfail:0   fail:10  skip:1121 time:9599s
shard-snb        total:2667 pass:1313 dwarn:1   dfail:0   fail:12  skip:1341 time:8128s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7258/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

[Tvrtko Ursulin]

On 23/11/2017 13:48, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration
> URL   : https://patchwork.freedesktop.org/series/34300/
> State : success
> 
> == Summary ==
> 
> Series 34300v1 series starting with [1/2] drm/i915/pmu: Remove conditional HOTPLUG_CPU registration
> https://patchwork.freedesktop.org/api/1.0/series/34300/revisions/1/mbox/
> 
> fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:449s
> fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
> fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:386s
> fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:537s
> fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
> fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:505s
> fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:512s
> fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:501s
> fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:490s
> fi-cfl-s2        total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:608s
> fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:428s
> fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:266s
> fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:540s
> fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
> fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
> fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:425s
> fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:486s
> fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
> fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:569s
> fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:467s
> fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:548s
> fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:561s
> fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:515s
> fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
> fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:466s
> fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:559s
> fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:420s
> Blacklisted hosts:
> fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:570s
> fi-glk-dsi       total:289  pass:132  dwarn:0   dfail:10  fail:2   skip:145 time:309s
> fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:483s
> fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:529s
> fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:480s
> fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:539s
> 
> 9d399f81694538228a539de9a908cdf9ef00f814 drm-tip: 2017y-11m-23d-12h-28m-12s UTC integration manifest
> 52921f335e1f drm/i915/pmu: Only allow running on a single CPU
> f8a679d0893c drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

Pushed and hopefully this is the end of the cpu hotplug saga!

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx