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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <14dd06c2-39f8-ccb5-ce59-f3a1e45c94d0@arm.com>
Date:   Mon, 9 Jan 2023 15:11:47 +0000
From:   James Clark <james.clark@....com>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ravi Bangoria <ravi.bangoria@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Hyeonggon Yoo <42.hyeyoo@...il.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] perf kmem: Support legacy tracepoints



On 08/01/2023 06:23, Leo Yan wrote:
> Commit 11e9734bcb6a ("mm/slab_common: unify NUMA and UMA version of
> tracepoints") removed tracepoints 'kmalloc_node' and
> 'kmem_cache_alloc_node', these two tracepoints have disappeared in the
> latest kernel, but we also need to consider the tool should be backward
> compatible with old kernels.
> 
> If detects the tracepoint "kmem:kmalloc_node" is existed on a system,
> this patch enables the legacy tracepoints, otherwise, it will ignore
> them for the new kernels.
> 
> Reported-by: Ravi Bangoria <ravi.bangoria@....com>
> Fixes: 11e9734bcb6a ("mm/slab_common: unify NUMA and UMA version of tracepoints")
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
> ---
>  tools/perf/builtin-kmem.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index e20656c431a4..50a3df5dc18a 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -1824,6 +1824,19 @@ static int parse_line_opt(const struct option *opt __maybe_unused,
>  	return 0;
>  }
>  
> +static bool slab_legacy_tp_is_exposed(void)
> +{
> +	/*
> +	 * The tracepoints "kmem:kmalloc_node" and
> +	 * "kmem:kmem_cache_alloc_node" have been removed on the latest
> +	 * kernel, if the tracepoint "kmem:kmalloc_node" is existed it
> +	 * means the tool is running on an old kernel, we need to
> +	 * rollback to support these legacy tracepoints.
> +	 */
> +	return IS_ERR(trace_event__tp_format("kmem", "kmalloc_node")) ?
> +		false : true;
> +}
> +
>  static int __cmd_record(int argc, const char **argv)
>  {
>  	const char * const record_args[] = {
> @@ -1831,22 +1844,28 @@ static int __cmd_record(int argc, const char **argv)
>  	};
>  	const char * const slab_events[] = {
>  	"-e", "kmem:kmalloc",
> -	"-e", "kmem:kmalloc_node",
>  	"-e", "kmem:kfree",
>  	"-e", "kmem:kmem_cache_alloc",
> -	"-e", "kmem:kmem_cache_alloc_node",
>  	"-e", "kmem:kmem_cache_free",
>  	};
> +	const char * const slab_legacy_events[] = {
> +	"-e", "kmem:kmalloc_node",
> +	"-e", "kmem:kmem_cache_alloc_node",
> +	};

Reviewed-by: James Clark <james.clark@....com>

This fixes the error with mem:kmalloc_node for me.

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.

James