This came out of the discussion here [1]. It seems like we can get some
extra big.LITTLE stuff working pretty easily. The test issues mentioned
in the linked thread are actually fairly unrelated and I've fixed them
in a different set on the list.
After adding it in the first commit, the remaining ones tidy up a
related capability that doesn't do anything any more.
I've added a fixes tag for the commit where
PERF_PMU_CAP_EXTENDED_HW_TYPE was originally added because it probably
should have been added to the Arm PMU at the same time. It doesn't apply
cleanly that far back because another capability was added between then,
but the resolution is trivial.
Thanks
James
[1]: https://lore.kernel.org/linux-perf-users/CAP-5=fVkRc9=ySJ=fG-SQ8oAKmE_1mhHHzSASmGHUsda5Qy92A@mail.gmail.com/T/#t
James Clark (4):
arm_pmu: Add PERF_PMU_CAP_EXTENDED_HW_TYPE capability
perf/x86: Remove unused PERF_PMU_CAP_HETEROGENEOUS_CPUS capability
arm_pmu: Remove unused PERF_PMU_CAP_HETEROGENEOUS_CPUS capability
perf: Remove unused PERF_PMU_CAP_HETEROGENEOUS_CPUS capability
arch/x86/events/core.c | 1 -
drivers/perf/arm_pmu.c | 10 ++++++----
include/linux/perf_event.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
--
2.34.1
Since commit bd2756811766 ("perf: Rewrite core context handling") the
relationship between perf_event_context and PMUs has changed so that
the error scenario that PERF_PMU_CAP_HETEROGENEOUS_CPUS originally
silenced no longer exists.
Remove the capability to avoid confusion that it actually influences
any perf core behavior. This change should be a no-op.
Signed-off-by: James Clark <[email protected]>
---
include/linux/perf_event.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index d5628a7b5eaa..3f4d941fd6c5 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -288,7 +288,7 @@ struct perf_event_pmu_context;
#define PERF_PMU_CAP_EXTENDED_REGS 0x0008
#define PERF_PMU_CAP_EXCLUSIVE 0x0010
#define PERF_PMU_CAP_ITRACE 0x0020
-#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
+/* Unused 0x0040 */
#define PERF_PMU_CAP_NO_EXCLUDE 0x0080
#define PERF_PMU_CAP_AUX_OUTPUT 0x0100
#define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200
--
2.34.1
On Mon, Jul 10, 2023 at 5:22 AM James Clark <[email protected]> wrote:
>
> Since commit bd2756811766 ("perf: Rewrite core context handling") the
> relationship between perf_event_context and PMUs has changed so that
> the error scenario that PERF_PMU_CAP_HETEROGENEOUS_CPUS originally
> silenced no longer exists.
>
> Remove the capability to avoid confusion that it actually influences
> any perf core behavior. This change should be a no-op.
>
> Signed-off-by: James Clark <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Thanks,
Ian
> ---
> include/linux/perf_event.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index d5628a7b5eaa..3f4d941fd6c5 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -288,7 +288,7 @@ struct perf_event_pmu_context;
> #define PERF_PMU_CAP_EXTENDED_REGS 0x0008
> #define PERF_PMU_CAP_EXCLUSIVE 0x0010
> #define PERF_PMU_CAP_ITRACE 0x0020
> -#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
> +/* Unused 0x0040 */
> #define PERF_PMU_CAP_NO_EXCLUDE 0x0080
> #define PERF_PMU_CAP_AUX_OUTPUT 0x0100
> #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200
> --
> 2.34.1
>
On 7/10/23 17:51, James Clark wrote:
> Since commit bd2756811766 ("perf: Rewrite core context handling") the
> relationship between perf_event_context and PMUs has changed so that
> the error scenario that PERF_PMU_CAP_HETEROGENEOUS_CPUS originally
> silenced no longer exists.
>
> Remove the capability to avoid confusion that it actually influences
> any perf core behavior. This change should be a no-op.
>
> Signed-off-by: James Clark <[email protected]>
> ---
> include/linux/perf_event.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index d5628a7b5eaa..3f4d941fd6c5 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -288,7 +288,7 @@ struct perf_event_pmu_context;
> #define PERF_PMU_CAP_EXTENDED_REGS 0x0008
> #define PERF_PMU_CAP_EXCLUSIVE 0x0010
> #define PERF_PMU_CAP_ITRACE 0x0020
> -#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
> +/* Unused 0x0040 */
Small nit, "Unused" marking might not be required here.
> #define PERF_PMU_CAP_NO_EXCLUDE 0x0080
> #define PERF_PMU_CAP_AUX_OUTPUT 0x0100
> #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200
On 11/07/2023 13:10, Anshuman Khandual wrote:
>
>
> On 7/10/23 17:51, James Clark wrote:
>> Since commit bd2756811766 ("perf: Rewrite core context handling") the
>> relationship between perf_event_context and PMUs has changed so that
>> the error scenario that PERF_PMU_CAP_HETEROGENEOUS_CPUS originally
>> silenced no longer exists.
>>
>> Remove the capability to avoid confusion that it actually influences
>> any perf core behavior. This change should be a no-op.
>>
>> Signed-off-by: James Clark <[email protected]>
>> ---
>> include/linux/perf_event.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>> index d5628a7b5eaa..3f4d941fd6c5 100644
>> --- a/include/linux/perf_event.h
>> +++ b/include/linux/perf_event.h
>> @@ -288,7 +288,7 @@ struct perf_event_pmu_context;
>> #define PERF_PMU_CAP_EXTENDED_REGS 0x0008
>> #define PERF_PMU_CAP_EXCLUSIVE 0x0010
>> #define PERF_PMU_CAP_ITRACE 0x0020
>> -#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
>> +/* Unused 0x0040 */
>
> Small nit, "Unused" marking might not be required here.
>
But then it would be very easy to miss that there is a free bit if I
don't leave the comment. Is it really better without it?
I could shift all the following ones down by one bit, but it would be a
lot of work to make sure that nobody has hard coded some check for one
of the bits instead of using the define somewhere.
>> #define PERF_PMU_CAP_NO_EXCLUDE 0x0080
>> #define PERF_PMU_CAP_AUX_OUTPUT 0x0100
>> #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200
On 7/11/23 19:45, James Clark wrote:
>
>
> On 11/07/2023 13:10, Anshuman Khandual wrote:
>>
>>
>> On 7/10/23 17:51, James Clark wrote:
>>> Since commit bd2756811766 ("perf: Rewrite core context handling") the
>>> relationship between perf_event_context and PMUs has changed so that
>>> the error scenario that PERF_PMU_CAP_HETEROGENEOUS_CPUS originally
>>> silenced no longer exists.
>>>
>>> Remove the capability to avoid confusion that it actually influences
>>> any perf core behavior. This change should be a no-op.
>>>
>>> Signed-off-by: James Clark <[email protected]>
>>> ---
>>> include/linux/perf_event.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>>> index d5628a7b5eaa..3f4d941fd6c5 100644
>>> --- a/include/linux/perf_event.h
>>> +++ b/include/linux/perf_event.h
>>> @@ -288,7 +288,7 @@ struct perf_event_pmu_context;
>>> #define PERF_PMU_CAP_EXTENDED_REGS 0x0008
>>> #define PERF_PMU_CAP_EXCLUSIVE 0x0010
>>> #define PERF_PMU_CAP_ITRACE 0x0020
>>> -#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
>>> +/* Unused 0x0040 */
>>
>> Small nit, "Unused" marking might not be required here.
>>
>
> But then it would be very easy to miss that there is a free bit if I
> don't leave the comment. Is it really better without it?
>
> I could shift all the following ones down by one bit, but it would be a
Sounds as a better option IMHO.
> lot of work to make sure that nobody has hard coded some check for one
> of the bits instead of using the define somewhere.
These are not user visible ABI and hence defined in include/linux/perf_event.h
to be used by drivers registering a PMU for capability enumeration. I am just
wondering why they might have been hard coded any where ?
>
>>> #define PERF_PMU_CAP_NO_EXCLUDE 0x0080
>>> #define PERF_PMU_CAP_AUX_OUTPUT 0x0100
>>> #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200
On 13/07/2023 08:30, Anshuman Khandual wrote:
>
>
> On 7/11/23 19:45, James Clark wrote:
>>
>>
>> On 11/07/2023 13:10, Anshuman Khandual wrote:
>>>
>>>
>>> On 7/10/23 17:51, James Clark wrote:
>>>> Since commit bd2756811766 ("perf: Rewrite core context handling") the
>>>> relationship between perf_event_context and PMUs has changed so that
>>>> the error scenario that PERF_PMU_CAP_HETEROGENEOUS_CPUS originally
>>>> silenced no longer exists.
>>>>
>>>> Remove the capability to avoid confusion that it actually influences
>>>> any perf core behavior. This change should be a no-op.
>>>>
>>>> Signed-off-by: James Clark <[email protected]>
>>>> ---
>>>> include/linux/perf_event.h | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>>>> index d5628a7b5eaa..3f4d941fd6c5 100644
>>>> --- a/include/linux/perf_event.h
>>>> +++ b/include/linux/perf_event.h
>>>> @@ -288,7 +288,7 @@ struct perf_event_pmu_context;
>>>> #define PERF_PMU_CAP_EXTENDED_REGS 0x0008
>>>> #define PERF_PMU_CAP_EXCLUSIVE 0x0010
>>>> #define PERF_PMU_CAP_ITRACE 0x0020
>>>> -#define PERF_PMU_CAP_HETEROGENEOUS_CPUS 0x0040
>>>> +/* Unused 0x0040 */
>>>
>>> Small nit, "Unused" marking might not be required here.
>>>
>>
>> But then it would be very easy to miss that there is a free bit if I
>> don't leave the comment. Is it really better without it?
>>
>> I could shift all the following ones down by one bit, but it would be a
>
> Sounds as a better option IMHO.
>
>> lot of work to make sure that nobody has hard coded some check for one
>> of the bits instead of using the define somewhere.
>
> These are not user visible ABI and hence defined in include/linux/perf_event.h
> to be used by drivers registering a PMU for capability enumeration. I am just
> wondering why they might have been hard coded any where ?
>
You never know, I've seen worse! I suppose we'll find out if I change it
and anything breaks. It will probably be fine though, I'll make the
change on the next version.
>>
>>>> #define PERF_PMU_CAP_NO_EXCLUDE 0x0080
>>>> #define PERF_PMU_CAP_AUX_OUTPUT 0x0100
>>>> #define PERF_PMU_CAP_EXTENDED_HW_TYPE 0x0200