Skip to contentSkip to conversation

drm/i915/pmu: Remove conditional HOTPLUG_CPU registration

PATCH In review Tvrtko Ursulin 4 participants 7 messages Nov 23, 2017 – Nov 24, 2017

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Tvrtko Ursulin

author maintainer
#0 lore raw

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

From: Chris Wilson <[email protected]>

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 <[email protected]>
Cc Tvrtko Ursulin <[email protected]>
Cc Dmitry Rogozhkin <[email protected]>
Reviewed-by Tvrtko Ursulin <[email protected]>
---
 drivers/gpu/drm/i915/i915_pmu.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/gpu/drm/i915/i915_pmu.c +1 -7
index 1071935bfa67..d7acdaf7d9f8 100644
@@ -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)
Signature · 6 lines
2.14.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author maintainer
#1 lore raw

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

From: Tvrtko Ursulin <[email protected]>

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 <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Dmitry Rogozhkin <[email protected]>
---
 drivers/gpu/drm/i915/i915_pmu.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
drivers/gpu/drm/i915/i915_pmu.c +4 -8
index d7acdaf7d9f8..d83653c83d6e 100644
@@ -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;
Signature · 6 lines
2.14.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tested-by Reviewed-by on v1 2/2
#2 lore raw

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

Quoting Tvrtko Ursulin (2017-11-23 12:34:32)

Quoted 14 lines

From: Tvrtko Ursulin <[email protected]>

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.

Quoted 8 lines

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 <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Dmitry Rogozhkin <[email protected]>

Works for me.

Tested-by Chris Wilson <[email protected]>

As much as I understand this (which is not a lot!),

Reviewed-by Chris Wilson <[email protected]>

-Chris
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#3 lore raw

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

Quoted 24 lines · Chris Wilson

Quoting Tvrtko Ursulin (2017-11-23 12:34:32)

Quoted 14 lines

From: Tvrtko Ursulin <[email protected]>

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.

Quoted 8 lines

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 <[email protected]>
Cc Chris Wilson <[email protected]>
Cc Dmitry Rogozhkin <[email protected]>
Quoted 5 lines

Works for me.

Tested-by Chris Wilson <[email protected]>

As much as I understand this (which is not a lot!),

Reviewed-by Chris Wilson <[email protected]>

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#4 lore raw

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

In reply to Tvrtko Ursulin
== Series Details ==

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#5 lore raw

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

Quoted 46 lines · 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
Quoted 3 lines

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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#6 lore raw

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

In reply to Tvrtko Ursulin
== Series Details ==

== 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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx