linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf test: Enable Symbols test to work with a current module dso
@ 2024-01-31 19:24 Adrian Hunter
  2024-01-31 20:58 ` Ian Rogers
  2024-02-21  1:57 ` Namhyung Kim
  0 siblings, 2 replies; 4+ messages in thread
From: Adrian Hunter @ 2024-01-31 19:24 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel, linux-perf-users

The test needs a struct machine and creates one for the current host,
but a side-effect is that struct machine has set up kernel maps
including module maps.

If the 'Symbols' test --dso option specifies a current kernel module,
it will already be present as a kernel dso, and a map with kmaps needs
to be used otherwise there will be a segfault - see below.

For that case, find the existing map and use that. In that case also,
the dso is split by section into multiple dsos, so test those dsos
also. That in turn, shows up that those dsos have not had overlapping
symbols removed, so the test fails.

Example:

  Before:

    $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko
     70: Symbols                                                         :
    --- start ---
    Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko
    Segmentation fault (core dumped)

  After:

    $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko
     70: Symbols                                                         :
    --- start ---
    Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko
    Overlapping symbols:
     41d30-41fbb l vmx_init
     41d30-41fbb g init_module
    ---- end ----
    Symbols: FAILED!

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/symbols.c | 68 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c
index 16e1c5502b09..2fed6d67f50f 100644
--- a/tools/perf/tests/symbols.c
+++ b/tools/perf/tests/symbols.c
@@ -41,6 +41,30 @@ static void exit_test_info(struct test_info *ti)
 	machine__delete(ti->machine);
 }
 
+struct dso_map {
+	struct dso *dso;
+	struct map *map;
+};
+
+static int find_map_cb(struct map *map, void *d)
+{
+	struct dso_map *data = d;
+
+	if (map__dso(map) != data->dso)
+		return 0;
+	data->map = map;
+	return 1;
+}
+
+static struct map *find_module_map(struct machine *machine, struct dso *dso)
+{
+	struct dso_map data = { .dso = dso };
+
+	machine__for_each_kernel_map(machine, find_map_cb, &data);
+
+	return data.map;
+}
+
 static void get_test_dso_filename(char *filename, size_t max_sz)
 {
 	if (dso_to_test)
@@ -51,6 +75,26 @@ static void get_test_dso_filename(char *filename, size_t max_sz)
 
 static int create_map(struct test_info *ti, char *filename, struct map **map_p)
 {
+	struct dso *dso = machine__findnew_dso(ti->machine, filename);
+
+	/*
+	 * If 'filename' matches a current kernel module, must use a kernel
+	 * map. Find the one that already exists.
+	 */
+	if (dso && dso->kernel) {
+		*map_p = find_module_map(ti->machine, dso);
+		dso__put(dso);
+		if (!*map_p) {
+			pr_debug("Failed to find map for curent kernel module %s",
+				 filename);
+			return TEST_FAIL;
+		}
+		map__get(*map_p);
+		return TEST_OK;
+	}
+
+	dso__put(dso);
+
 	/* Create a dummy map at 0x100000 */
 	*map_p = map__new(ti->machine, 0x100000, 0xffffffff, 0, NULL,
 			  PROT_EXEC, 0, NULL, filename, ti->thread);
@@ -97,6 +141,26 @@ static int test_dso(struct dso *dso)
 	return ret;
 }
 
+static int subdivided_dso_cb(struct dso *dso, struct machine *machine __maybe_unused, void *d)
+{
+	struct dso *text_dso = d;
+
+	if (dso != text_dso && strstarts(dso->short_name, text_dso->short_name))
+		if (test_dso(dso) != TEST_OK)
+			return -1;
+
+	return 0;
+}
+
+static int process_subdivided_dso(struct machine *machine, struct dso *dso)
+{
+	int ret;
+
+	ret = machine__for_each_dso(machine, subdivided_dso_cb, dso);
+
+	return ret < 0 ? TEST_FAIL : TEST_OK;
+}
+
 static int test_file(struct test_info *ti, char *filename)
 {
 	struct map *map = NULL;
@@ -124,6 +188,10 @@ static int test_file(struct test_info *ti, char *filename)
 	}
 
 	ret = test_dso(dso);
+
+	/* Module dso is split into many dsos by section */
+	if (ret == TEST_OK && dso->kernel)
+		ret = process_subdivided_dso(ti->machine, dso);
 out_put:
 	map__put(map);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf test: Enable Symbols test to work with a current module dso
  2024-01-31 19:24 [PATCH] perf test: Enable Symbols test to work with a current module dso Adrian Hunter
@ 2024-01-31 20:58 ` Ian Rogers
  2024-02-16  9:47   ` Adrian Hunter
  2024-02-21  1:57 ` Namhyung Kim
  1 sibling, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2024-01-31 20:58 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim, linux-kernel,
	linux-perf-users

On Wed, Jan 31, 2024 at 11:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> The test needs a struct machine and creates one for the current host,
> but a side-effect is that struct machine has set up kernel maps
> including module maps.
>
> If the 'Symbols' test --dso option specifies a current kernel module,
> it will already be present as a kernel dso, and a map with kmaps needs
> to be used otherwise there will be a segfault - see below.
>
> For that case, find the existing map and use that. In that case also,
> the dso is split by section into multiple dsos, so test those dsos
> also. That in turn, shows up that those dsos have not had overlapping
> symbols removed, so the test fails.
>
> Example:
>
>   Before:
>
>     $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko
>      70: Symbols                                                         :
>     --- start ---
>     Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko
>     Segmentation fault (core dumped)
>
>   After:
>
>     $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko
>      70: Symbols                                                         :
>     --- start ---
>     Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko
>     Overlapping symbols:
>      41d30-41fbb l vmx_init
>      41d30-41fbb g init_module
>     ---- end ----
>     Symbols: FAILED!
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/tests/symbols.c | 68 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/tools/perf/tests/symbols.c b/tools/perf/tests/symbols.c
> index 16e1c5502b09..2fed6d67f50f 100644
> --- a/tools/perf/tests/symbols.c
> +++ b/tools/perf/tests/symbols.c
> @@ -41,6 +41,30 @@ static void exit_test_info(struct test_info *ti)
>         machine__delete(ti->machine);
>  }
>
> +struct dso_map {
> +       struct dso *dso;
> +       struct map *map;
> +};
> +
> +static int find_map_cb(struct map *map, void *d)
> +{
> +       struct dso_map *data = d;
> +
> +       if (map__dso(map) != data->dso)
> +               return 0;
> +       data->map = map;
> +       return 1;
> +}
> +
> +static struct map *find_module_map(struct machine *machine, struct dso *dso)
> +{
> +       struct dso_map data = { .dso = dso };
> +
> +       machine__for_each_kernel_map(machine, find_map_cb, &data);
> +
> +       return data.map;
> +}
> +
>  static void get_test_dso_filename(char *filename, size_t max_sz)
>  {
>         if (dso_to_test)
> @@ -51,6 +75,26 @@ static void get_test_dso_filename(char *filename, size_t max_sz)
>
>  static int create_map(struct test_info *ti, char *filename, struct map **map_p)
>  {
> +       struct dso *dso = machine__findnew_dso(ti->machine, filename);
> +
> +       /*
> +        * If 'filename' matches a current kernel module, must use a kernel
> +        * map. Find the one that already exists.
> +        */
> +       if (dso && dso->kernel) {
> +               *map_p = find_module_map(ti->machine, dso);
> +               dso__put(dso);
> +               if (!*map_p) {
> +                       pr_debug("Failed to find map for curent kernel module %s",
> +                                filename);
> +                       return TEST_FAIL;
> +               }
> +               map__get(*map_p);
> +               return TEST_OK;
> +       }
> +
> +       dso__put(dso);
> +
>         /* Create a dummy map at 0x100000 */
>         *map_p = map__new(ti->machine, 0x100000, 0xffffffff, 0, NULL,
>                           PROT_EXEC, 0, NULL, filename, ti->thread);
> @@ -97,6 +141,26 @@ static int test_dso(struct dso *dso)
>         return ret;
>  }
>
> +static int subdivided_dso_cb(struct dso *dso, struct machine *machine __maybe_unused, void *d)
> +{
> +       struct dso *text_dso = d;
> +
> +       if (dso != text_dso && strstarts(dso->short_name, text_dso->short_name))
> +               if (test_dso(dso) != TEST_OK)
> +                       return -1;
> +
> +       return 0;
> +}
> +
> +static int process_subdivided_dso(struct machine *machine, struct dso *dso)
> +{
> +       int ret;
> +
> +       ret = machine__for_each_dso(machine, subdivided_dso_cb, dso);
> +
> +       return ret < 0 ? TEST_FAIL : TEST_OK;
> +}
> +
>  static int test_file(struct test_info *ti, char *filename)
>  {
>         struct map *map = NULL;
> @@ -124,6 +188,10 @@ static int test_file(struct test_info *ti, char *filename)
>         }
>
>         ret = test_dso(dso);
> +
> +       /* Module dso is split into many dsos by section */
> +       if (ret == TEST_OK && dso->kernel)
> +               ret = process_subdivided_dso(ti->machine, dso);
>  out_put:
>         map__put(map);
>
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf test: Enable Symbols test to work with a current module dso
  2024-01-31 20:58 ` Ian Rogers
@ 2024-02-16  9:47   ` Adrian Hunter
  0 siblings, 0 replies; 4+ messages in thread
From: Adrian Hunter @ 2024-02-16  9:47 UTC (permalink / raw)
  To: Namhyung Kim, Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, linux-kernel, linux-perf-users, Ian Rogers

On 31/01/24 22:58, Ian Rogers wrote:
> On Wed, Jan 31, 2024 at 11:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> The test needs a struct machine and creates one for the current host,
>> but a side-effect is that struct machine has set up kernel maps
>> including module maps.
>>
>> If the 'Symbols' test --dso option specifies a current kernel module,
>> it will already be present as a kernel dso, and a map with kmaps needs
>> to be used otherwise there will be a segfault - see below.
>>
>> For that case, find the existing map and use that. In that case also,
>> the dso is split by section into multiple dsos, so test those dsos
>> also. That in turn, shows up that those dsos have not had overlapping
>> symbols removed, so the test fails.
>>
>> Example:
>>
>>   Before:
>>
>>     $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko
>>      70: Symbols                                                         :
>>     --- start ---
>>     Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko
>>     Segmentation fault (core dumped)
>>
>>   After:
>>
>>     $ perf test -F -v Symbols --dso /lib/modules/$(uname -r)/kernel/arch/x86/kvm/kvm-intel.ko
>>      70: Symbols                                                         :
>>     --- start ---
>>     Testing /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko
>>     Overlapping symbols:
>>      41d30-41fbb l vmx_init
>>      41d30-41fbb g init_module
>>     ---- end ----
>>     Symbols: FAILED!
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 

Thanks Ian!

This patch is still OK.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf test: Enable Symbols test to work with a current module dso
  2024-01-31 19:24 [PATCH] perf test: Enable Symbols test to work with a current module dso Adrian Hunter
  2024-01-31 20:58 ` Ian Rogers
@ 2024-02-21  1:57 ` Namhyung Kim
  1 sibling, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2024-02-21  1:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Adrian Hunter
  Cc: linux-kernel, linux-perf-users, Jiri Olsa, Ian Rogers

On Wed, 31 Jan 2024 21:24:16 +0200, Adrian Hunter wrote:
> The test needs a struct machine and creates one for the current host,
> but a side-effect is that struct machine has set up kernel maps
> including module maps.
> 
> If the 'Symbols' test --dso option specifies a current kernel module,
> it will already be present as a kernel dso, and a map with kmaps needs
> to be used otherwise there will be a segfault - see below.
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
-- 
Namhyung Kim <namhyung@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-02-21  1:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-31 19:24 [PATCH] perf test: Enable Symbols test to work with a current module dso Adrian Hunter
2024-01-31 20:58 ` Ian Rogers
2024-02-16  9:47   ` Adrian Hunter
2024-02-21  1:57 ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).