All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Cc: will@kernel.org, mark.rutland@arm.com,
	linux-arm-kernel@lists.infradead.org,
	linux-perf-users@vger.kernel.org, ilkka@os.amperecomputing.com,
	leo.yan@arm.com
Subject: Re: [PATCH v2 1/5] perf/arm-cmn: Rename filter variables for clarity
Date: Wed, 29 Jul 2026 19:39:33 +0100	[thread overview]
Message-ID: <25ddc6ec-3a8a-4ae5-8f33-e6c71ff354b5@arm.com> (raw)
In-Reply-To: <20260729102014.000019ea@oss.qualcomm.com>

On 29/07/2026 6:20 pm, Jonathan Cameron wrote:
> On Tue, 28 Jul 2026 16:59:21 +0100
> Robin Murphy <robin.murphy@arm.com> wrote:
> 
>> CMN has already grown many more event-specific filters than the original
>> Occupancy ID, but since they are all independent of each other we've
>> just overloaded them onto the same name. Before we add yet more, and
>> they begin to overlap, rename all our "occupid" variables to "filter" so
>> that things can be a bit clearer and more consistent (but leaving the
>> format attribute itself, to avoid UAPI concerns).
> Hi Robin,
> 
> Drive by review... I'm a bit confused.
> The print out change isn't a ABI change?  I'd at least mention what changes
> around that in the commit message.
> 
> Jonathan
> 
>>
>> Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>
>> v2: No change
>>
>>   drivers/perf/arm-cmn.c | 53 ++++++++++++++++++++++--------------------
>>   1 file changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
>> index 50402bc4a21d..2a8a67da72c3 100644
>> --- a/drivers/perf/arm-cmn.c
>> +++ b/drivers/perf/arm-cmn.c
> ...
> 
>> @@ -709,8 +709,8 @@ static ssize_t arm_cmn_event_show(struct device *dev,
>>   				  eattr->type, eattr->eventid);
>>   
>>   	if (eattr->fsel > SEL_NONE)
>> -		return sysfs_emit(buf, "type=0x%x,eventid=0x%x,occupid=0x%x\n",
>> -				  eattr->type, eattr->eventid, eattr->occupid);
>> +		return sysfs_emit(buf, "type=0x%x,eventid=0x%x,filter=0x%x\n",
>> +				  eattr->type, eattr->eventid, eattr->filter);
> 
> Changing the format of this smells like ABI change to me.

This is not a change per the event alias ABI itself[1], since that 
contract is only that these aliases may contain *some* symbolic terms 
which are to be dynamically resolved from the corresponding "format" 
attributes. Both the "occupid" and "filter" format names yield the same 
result, so tools simply parsing the sysfs aliases from scratch as 
intended are entirely unaffected.

The potential UAPI concern alluded to is between *those* tools and any 
higher-level scripts driving them - if those scripts contain 
already-copied or manually-generated event specifiers using an 
"occupid=" term then they would start failing if perf tool (or 
equivalent) could no longer resolve the "occupid" format. Thus although 
we're free to stop referring to it from our own event aliases, we can't 
just remove the old format name altogether. Because unfortunately, such 
scripts definitely do exist[2].

Anyone who was reading the sysfs event attributes expecting to see 
something they already know would have missed the point of the whole 
mechanism so hard I wouldn't even know what to say...

Thanks,
Robin.

[1] i.e. Documentation/ABI/testing/sysfs-bus-event_source-devices-events
[2] 
https://gitlab.arm.com/telemetry-solution/telemetry-solution/-/blob/main/tools/topdown_tool/topdown_tool/cmn_probe/common.py?ref_type=heads#L311

> 
>>   
>>   	return sysfs_emit(buf, "type=0x%x,eventid=0x%x\n", eattr->type,
>>   			  eattr->eventid);



  reply	other threads:[~2026-07-29 18:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 15:59 [PATCH v2 0/5] perf/arm-cmn: Support CMN S3 r2 Robin Murphy
2026-07-28 15:59 ` [PATCH v2 1/5] perf/arm-cmn: Rename filter variables for clarity Robin Murphy
2026-07-29 17:20   ` Jonathan Cameron
2026-07-29 18:39     ` Robin Murphy [this message]
2026-07-28 15:59 ` [PATCH v2 2/5] perf/arm-cmn: Refactor event filter programming Robin Murphy
2026-07-28 15:59 ` [PATCH v2 3/5] perf/arm-cmn: Refactor event filter data Robin Murphy
2026-07-28 15:59 ` [PATCH v2 4/5] perf/arm-cmn: Plumb in new filter types Robin Murphy
2026-07-28 16:43   ` sashiko-bot
2026-07-28 15:59 ` [PATCH v2 5/5] perf/arm-cmn: Support CMN S3 r2 Robin Murphy
2026-07-28 16:57   ` sashiko-bot
2026-07-28 17:16     ` Robin Murphy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=25ddc6ec-3a8a-4ae5-8f33-e6c71ff354b5@arm.com \
    --to=robin.murphy@arm.com \
    --cc=ilkka@os.amperecomputing.com \
    --cc=jonathan.cameron@oss.qualcomm.com \
    --cc=leo.yan@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.