Skip to contentSkip to conversation

perf kcore_copy: Do not check /proc/modules is unchanged

PATCH Applied Adrian Hunter 4 participants 4 messages Sep 14, 2022 – Sep 20, 2022 perf events

lore
Share & tools
lore

Adrian Hunter

author reviewer
#0 lore raw

PATCH 1/1 perf kcore_copy: Do not check /proc/modules is unchanged

/proc/kallsyms and /proc/modules are compared before and after the copy in order to ensure no changes during the copy. However /proc/modules also might change due to reference counts changing even though that does not make any difference. Any modules loaded or unloaded should be visible in changes to kallsyms, so it is not necessary to check /proc/modules also anyway.

Remove the comparison checking that /proc/modules is unchanged.

Fixes fc1b691d7651 ("perf buildid-cache: Add ability to add kcore to the cache")
---
 tools/perf/util/symbol-elf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
tools/perf/util/symbol-elf.c +2 -5
index 75bec32d4f57..647b7dff8ef3 100644
@@ -2102,8 +2102,8 @@ static int kcore_copy__compare_file(const char *from_dir, const char *to_dir,
  * unusual.  One significant peculiarity is that the mapping (start -> pgoff)
  * is not the same for the kernel map and the modules map.  That happens because
  * the data is copied adjacently whereas the original kcore has gaps.  Finally,
- * kallsyms and modules files are compared with their copies to check that
- * modules have not been loaded or unloaded while the copies were taking place.
+ * kallsyms file is compared with its copy to check that modules have not been
+ * loaded or unloaded while the copies were taking place.
  *
  * Return: %0 on success, %-1 on failure.
  */
@@ -2166,9 +2166,6 @@ int kcore_copy(const char *from_dir, const char *to_dir)
 			goto out_extract_close;
 	}
 
-	if (kcore_copy__compare_file(from_dir, to_dir, "modules"))
-		goto out_extract_close;
-
 	if (kcore_copy__compare_file(from_dir, to_dir, "kallsyms"))
 		goto out_extract_close;
 
Signature · 1 line
2.25.1
Tested-by on v1 1/1
#1 lore raw
Quoted 43 lines · Adrian Hunter

/proc/kallsyms and /proc/modules are compared before and after the copy in order to ensure no changes during the copy. However /proc/modules also might change due to reference counts changing even though that does not make any difference. Any modules loaded or unloaded should be visible in changes to kallsyms, so it is not necessary to check /proc/modules also anyway.

Remove the comparison checking that /proc/modules is unchanged.

Fixes fc1b691d7651 ("perf buildid-cache: Add ability to add kcore to the cache")
---
 tools/perf/util/symbol-elf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
tools/perf/util/symbol-elf.c +2 -6
index 75bec32d4f57..647b7dff8ef3 100644
@@ -2102,8 +2102,8 @@ static int kcore_copy__compare_file(const char *from_dir, const char *to_dir,
  * unusual.  One significant peculiarity is that the mapping (start -> pgoff)
  * is not the same for the kernel map and the modules map.  That happens because
  * the data is copied adjacently whereas the original kcore has gaps.  Finally,
- * kallsyms and modules files are compared with their copies to check that
- * modules have not been loaded or unloaded while the copies were taking place.
+ * kallsyms file is compared with its copy to check that modules have not been
+ * loaded or unloaded while the copies were taking place.
  *
  * Return: %0 on success, %-1 on failure.
  */
@@ -2166,9 +2166,6 @@ int kcore_copy(const char *from_dir, const char *to_dir)
                        goto out_extract_close;
        }
 
-       if (kcore_copy__compare_file(from_dir, to_dir, "modules"))
-               goto out_extract_close;
-
        if (kcore_copy__compare_file(from_dir, to_dir, "kallsyms"))
                goto out_extract_close;
 
--
Quoted 2 lines

2.25.1

Thanks Adrian,

maintainer Acked-by on v1 1/1
#2 lore raw
In reply to Adrian Hunter
Quoted 8 lines · Adrian Hunter

/proc/kallsyms and /proc/modules are compared before and after the copy in order to ensure no changes during the copy. However /proc/modules also might change due to reference counts changing even though that does not make any difference. Any modules loaded or unloaded should be visible in changes to kallsyms, so it is not necessary to check /proc/modules also anyway.

Quoted 5 lines

Remove the comparison checking that /proc/modules is unchanged.

Fixes fc1b691d7651 ("perf buildid-cache: Add ability to add kcore to the cache")

Thanks,
Namhyung

Quoted 32 lines
---
 tools/perf/util/symbol-elf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
tools/perf/util/symbol-elf.c +2 -6
index 75bec32d4f57..647b7dff8ef3 100644
@@ -2102,8 +2102,8 @@ static int kcore_copy__compare_file(const char *from_dir, const char *to_dir,
  * unusual.  One significant peculiarity is that the mapping (start -> pgoff)
  * is not the same for the kernel map and the modules map.  That happens because
  * the data is copied adjacently whereas the original kcore has gaps.  Finally,
- * kallsyms and modules files are compared with their copies to check that
- * modules have not been loaded or unloaded while the copies were taking place.
+ * kallsyms file is compared with its copy to check that modules have not been
+ * loaded or unloaded while the copies were taking place.
  *
  * Return: %0 on success, %-1 on failure.
  */
@@ -2166,9 +2166,6 @@ int kcore_copy(const char *from_dir, const char *to_dir)
                        goto out_extract_close;
        }
 
-       if (kcore_copy__compare_file(from_dir, to_dir, "modules"))
-               goto out_extract_close;
-
        if (kcore_copy__compare_file(from_dir, to_dir, "kallsyms"))
                goto out_extract_close;
 
--

2.25.1

maintainer Applied
#3 lore raw

Em Fri, Sep 16, 2022 at 11:02:25AM -0700, Namhyung Kim escreveu:

Quoted 9 lines · Adrian Hunter
Quoted 8 lines

/proc/kallsyms and /proc/modules are compared before and after the copy in order to ensure no changes during the copy. However /proc/modules also might change due to reference counts changing even though that does not make any difference. Any modules loaded or unloaded should be visible in changes to kallsyms, so it is not necessary to check /proc/modules also anyway.

Quoted 7 lines
Quoted 5 lines

Remove the comparison checking that /proc/modules is unchanged.

Fixes fc1b691d7651 ("perf buildid-cache: Add ability to add kcore to the cache")

Thanks, applied.

- Arnaldo

Quoted 36 lines

Thanks,
Namhyung

Quoted 32 lines
---
 tools/perf/util/symbol-elf.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
tools/perf/util/symbol-elf.c +2 -6
index 75bec32d4f57..647b7dff8ef3 100644
@@ -2102,8 +2102,8 @@ static int kcore_copy__compare_file(const char *from_dir, const char *to_dir,
  * unusual.  One significant peculiarity is that the mapping (start -> pgoff)
  * is not the same for the kernel map and the modules map.  That happens because
  * the data is copied adjacently whereas the original kcore has gaps.  Finally,
- * kallsyms and modules files are compared with their copies to check that
- * modules have not been loaded or unloaded while the copies were taking place.
+ * kallsyms file is compared with its copy to check that modules have not been
+ * loaded or unloaded while the copies were taking place.
  *
  * Return: %0 on success, %-1 on failure.
  */
@@ -2166,9 +2166,6 @@ int kcore_copy(const char *from_dir, const char *to_dir)
                        goto out_extract_close;
        }
 
-       if (kcore_copy__compare_file(from_dir, to_dir, "modules"))
-               goto out_extract_close;
-
        if (kcore_copy__compare_file(from_dir, to_dir, "kallsyms"))
                goto out_extract_close;
 
--

2.25.1

Signature · 1 line
- Arnaldo