Re: [PATCH 1/2] perf kmem: Support legacy tracepoints

[James Clark]

On 10/01/2023 01:45, Leo Yan wrote:
> On Mon, Jan 09, 2023 at 12:38:04PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> [...]
> 
>>>> +	const char * const slab_legacy_events[] = {
>>>> +	"-e", "kmem:kmalloc_node",
>>>> +	"-e", "kmem:kmem_cache_alloc_node",
>>>> +	};
>>>
>>> Reviewed-by: James Clark <james.clark@xxxxxxx>
>>>
>>> This fixes the error with mem:kmalloc_node for me.
> 
> Thanks for reviewing and testing!
> 
>>> I was thinking that it might be best to add all events to the list
>>> conditionally instead of just the legacy ones. That way, the same error
>>> won't happen in the future. But maybe it's best to have an explicit
>>> error again in case the breaking change was unintentional so it's fine
>>> as it is I think.
> 
> Yeah, this is a good idea for refactoring.
> 
> James, do you mind to send patches for this?

Do you not think there is any value in keeping it as showing an error
for the next time one is removed? I was assuming that was your intention
with this change, and I'm ok with keeping it that way for now. It's
probably quite rare anyway so the refactor could be more effort than the
gain.

James


> 
>> Just applied this, the changes you brains stormed may come as later
>> patches, thanks,
> 
> Thanks, Arnaldo.
> 
> Leo