Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <12288ed4-b4c4-cd53-ab0e-8b235e0852c0@arm.com>
Date:   Tue, 10 Jan 2023 09:33:37 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH 1/2] perf kmem: Support legacy tracepoints
Content-Language: en-US
To:     Leo Yan <leo.yan@linaro.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Ravi Bangoria <ravi.bangoria@amd.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230108062400.250690-1-leo.yan@linaro.org>
 <14dd06c2-39f8-ccb5-ce59-f3a1e45c94d0@arm.com> <Y7w03A7f2zUWwihm@kernel.org>
 <Y7zC5imskN9tvtBn@leoy-yangtze.lan>
From:   James Clark <james.clark@arm.com>
In-Reply-To: <Y7zC5imskN9tvtBn@leoy-yangtze.lan>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


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@arm.com>
>>>
>>> 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