Skip to contentSkip to conversation

perf unwind: Set userdata for all __report_module paths

PATCH In review [email protected] 3 participants 6 messages Feb 18, 2021 perf events

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf unwind: Set userdata for all __report_module paths

From: Dave Rigby <[email protected]>

When locating the dwarf module for a given address, __find_debuginfo() requires a 'struct dso' passed via the userdata argument.

However, this field is only set in __report_module() if the module is found in via dwfl_addrmodule(), not if it is found later via dwfl_report_elf().

Set userdata irrespective of how the dwarf module was found, as long as we found a module.

Fixes bf53fc6b5f41 ("perf unwind: Fix separate debug info files when using elfutils' libdw's unwinder")
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jan Kratochvil <[email protected]>
Cc Jiri Olsa <[email protected]>
---
 tools/perf/util/unwind-libdw.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
tools/perf/util/unwind-libdw.c +8 -3
index 0ada907..a74b517 100644
@@ -60,10 +60,8 @@ static int __report_module(struct addr_location *al, u64 ip,
 	mod = dwfl_addrmodule(ui->dwfl, ip);
 	if (mod) {
 		Dwarf_Addr s;
-		void **userdatap;
 
-		dwfl_module_info(mod, &userdatap, &s, NULL, NULL, NULL, NULL, NULL);
-		*userdatap = dso;
+		dwfl_module_info(mod, NULL, &s, NULL, NULL, NULL, NULL, NULL);
 		if (s != al->map->start - al->map->pgoff)
 			mod = 0;
 	}
@@ -79,6 +77,13 @@ static int __report_module(struct addr_location *al, u64 ip,
 					      al->map->start - al->map->pgoff, false);
 	}
 
+	if (mod) {
+		void **userdatap;
+
+		dwfl_module_info(mod, &userdatap, NULL, NULL, NULL, NULL, NULL, NULL);
+		*userdatap = dso;
+	}
+
 	return mod && dwfl_addrmodule(ui->dwfl, ip) == mod ? 0 : -1;
 }
 
Signature · 1 line
1.8.3.1
author
#1 lore raw

I missed adding a link to the bugzilla issue: https://bugzilla.kernel.org/show_bug.cgi?id=211801

Note sure what the recommended way of tagging that in the patch is…

DaveR

Quoted 54 lines · On 18 Feb 2021, at 16:56, [email protected] wrote:

From: Dave Rigby <[email protected]>

When locating the dwarf module for a given address, __find_debuginfo() requires a 'struct dso' passed via the userdata argument.

However, this field is only set in __report_module() if the module is found in via dwfl_addrmodule(), not if it is found later via dwfl_report_elf().

Set userdata irrespective of how the dwarf module was found, as long as we found a module.

Fixes bf53fc6b5f41 ("perf unwind: Fix separate debug info files when using elfutils' libdw's unwinder")
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jan Kratochvil <[email protected]>
Cc Jiri Olsa <[email protected]>

---
tools/perf/util/unwind-libdw.c | 11 ++++++++---

1 file changed, 8 insertions(+), 3 deletions(-)
tools/perf/util/unwind-libdw.c +8 -3
index 0ada907..a74b517 100644
@@ -60,10 +60,8 @@ static int __report_module(struct addr_location *al, u64 ip,
 	mod = dwfl_addrmodule(ui->dwfl, ip);
 	if (mod) {
 		Dwarf_Addr s;
-		void **userdatap;
 
-		dwfl_module_info(mod, &userdatap, &s, NULL, NULL, NULL, NULL, NULL);
-		*userdatap = dso;
+		dwfl_module_info(mod, NULL, &s, NULL, NULL, NULL, NULL, NULL);
 		if (s != al->map->start - al->map->pgoff)
 			mod = 0;
 	}
@@ -79,6 +77,13 @@ static int __report_module(struct addr_location *al, u64 ip,
 					      al->map->start - al->map->pgoff, false);
 	}
 
+	if (mod) {
+		void **userdatap;
+
+		dwfl_module_info(mod, &userdatap, NULL, NULL, NULL, NULL, NULL, NULL);
+		*userdatap = dso;
+	}
+
 	return mod && dwfl_addrmodule(ui->dwfl, ip) == mod ? 0 : -1;
 }
 
Signature · 1 line
1.8.3.1
#2 lore raw

Em Thu, Feb 18, 2021 at 04:58:44PM +0000, Dave Rigby escreveu:

Quoted 3 lines

I missed adding a link to the bugzilla issue: https://bugzilla.kernel.org/show_bug.cgi?id=211801

Note sure what the recommended way of tagging that in the patch is…

I'll add a:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=211801

tag,

Thanks,

- Arnaldo

Quoted 58 lines

DaveR

Quoted 54 lines · On 18 Feb 2021, at 16:56, [email protected] wrote:

From: Dave Rigby <[email protected]>

When locating the dwarf module for a given address, __find_debuginfo() requires a 'struct dso' passed via the userdata argument.

However, this field is only set in __report_module() if the module is found in via dwfl_addrmodule(), not if it is found later via dwfl_report_elf().

Set userdata irrespective of how the dwarf module was found, as long as we found a module.

Fixes bf53fc6b5f41 ("perf unwind: Fix separate debug info files when using elfutils' libdw's unwinder")
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jan Kratochvil <[email protected]>
Cc Jiri Olsa <[email protected]>

---
tools/perf/util/unwind-libdw.c | 11 ++++++++---

1 file changed, 8 insertions(+), 3 deletions(-)
tools/perf/util/unwind-libdw.c +8 -3
index 0ada907..a74b517 100644
@@ -60,10 +60,8 @@ static int __report_module(struct addr_location *al, u64 ip,
 	mod = dwfl_addrmodule(ui->dwfl, ip);
 	if (mod) {
 		Dwarf_Addr s;
-		void **userdatap;
 
-		dwfl_module_info(mod, &userdatap, &s, NULL, NULL, NULL, NULL, NULL);
-		*userdatap = dso;
+		dwfl_module_info(mod, NULL, &s, NULL, NULL, NULL, NULL, NULL);
 		if (s != al->map->start - al->map->pgoff)
 			mod = 0;
 	}
@@ -79,6 +77,13 @@ static int __report_module(struct addr_location *al, u64 ip,
 					      al->map->start - al->map->pgoff, false);
 	}
 
+	if (mod) {
+		void **userdatap;
+
+		dwfl_module_info(mod, &userdatap, NULL, NULL, NULL, NULL, NULL, NULL);
+		*userdatap = dso;
+	}
+
 	return mod && dwfl_addrmodule(ui->dwfl, ip) == mod ? 0 : -1;
 }
 
Signature · 1 line
1.8.3.1
Signature · 1 line
- Arnaldo
#3 lore raw
In reply to Dave Rigby

Em Thu, Feb 18, 2021 at 04:58:44PM +0000, Dave Rigby escreveu:

Quoted 3 lines

I missed adding a link to the bugzilla issue: https://bugzilla.kernel.org/show_bug.cgi?id=211801

Note sure what the recommended way of tagging that in the patch is…

You also forgot to add your:

I'm adding it, ok?

Please take a look at Documentation/process/submitting-patches.rst, this part:

----------------------------------------------------------------------------
Developer's Certificate of Origin 1.1
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

then you just add a line saying::

using your real name (sorry, no pseudonyms or anonymous contributions.) This will be done for you automatically if you use ``git commit -s``. Reverts should also include "Signed-off-by". ``git revert -s`` does that for you.

Some people also put extra tags at the end. They'll just be ignored for now, but you can do this to mark internal company procedures or just point out some special detail about the sign-off.

Any further SoBs (Signed-off-by:'s) following the author's SoB are from people handling and transporting the patch, but were not involved in its development. SoB chains should reflect the **real** route a patch took as it was propagated to the maintainers and ultimately to Linus, with the first SoB entry signalling primary authorship of a single author.
----------------------------------------------------------------------------

Quoted 58 lines

DaveR

Quoted 54 lines · On 18 Feb 2021, at 16:56, [email protected] wrote:

From: Dave Rigby <[email protected]>

When locating the dwarf module for a given address, __find_debuginfo() requires a 'struct dso' passed via the userdata argument.

However, this field is only set in __report_module() if the module is found in via dwfl_addrmodule(), not if it is found later via dwfl_report_elf().

Set userdata irrespective of how the dwarf module was found, as long as we found a module.

Fixes bf53fc6b5f41 ("perf unwind: Fix separate debug info files when using elfutils' libdw's unwinder")
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jan Kratochvil <[email protected]>
Cc Jiri Olsa <[email protected]>

---
tools/perf/util/unwind-libdw.c | 11 ++++++++---

1 file changed, 8 insertions(+), 3 deletions(-)
tools/perf/util/unwind-libdw.c +8 -3
index 0ada907..a74b517 100644
@@ -60,10 +60,8 @@ static int __report_module(struct addr_location *al, u64 ip,
 	mod = dwfl_addrmodule(ui->dwfl, ip);
 	if (mod) {
 		Dwarf_Addr s;
-		void **userdatap;
 
-		dwfl_module_info(mod, &userdatap, &s, NULL, NULL, NULL, NULL, NULL);
-		*userdatap = dso;
+		dwfl_module_info(mod, NULL, &s, NULL, NULL, NULL, NULL, NULL);
 		if (s != al->map->start - al->map->pgoff)
 			mod = 0;
 	}
@@ -79,6 +77,13 @@ static int __report_module(struct addr_location *al, u64 ip,
 					      al->map->start - al->map->pgoff, false);
 	}
 
+	if (mod) {
+		void **userdatap;
+
+		dwfl_module_info(mod, &userdatap, NULL, NULL, NULL, NULL, NULL, NULL);
+		*userdatap = dso;
+	}
+
 	return mod && dwfl_addrmodule(ui->dwfl, ip) == mod ? 0 : -1;
 }
 
Signature · 1 line
1.8.3.1
Signature · 1 line
- Arnaldo
author
#4 lore raw
Quoted 7 lines · Arnaldo Carvalho de Melo

Em Thu, Feb 18, 2021 at 04:58:44PM +0000, Dave Rigby escreveu:

Quoted 2 lines

I missed adding a link to the bugzilla issue: https://bugzilla.kernel.org/show_bug.cgi?id=211801

Quoted 7 lines
Quoted 1 line

Note sure what the recommended way of tagging that in the patch is…

You also forgot to add your:

I'm adding it, ok?

Thanks - I missed that in the documentation.

Quoted 114 lines

Please take a look at Documentation/process/submitting-patches.rst, this part:

----------------------------------------------------------------------------
Developer's Certificate of Origin 1.1
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

then you just add a line saying::

using your real name (sorry, no pseudonyms or anonymous contributions.) This will be done for you automatically if you use ``git commit -s``. Reverts should also include "Signed-off-by". ``git revert -s`` does that for you.

Some people also put extra tags at the end. They'll just be ignored for now, but you can do this to mark internal company procedures or just point out some special detail about the sign-off.

Any further SoBs (Signed-off-by:'s) following the author's SoB are from people handling and transporting the patch, but were not involved in its development. SoB chains should reflect the **real** route a patch took as it was propagated to the maintainers and ultimately to Linus, with the first SoB entry signalling primary authorship of a single author.
----------------------------------------------------------------------------

Quoted 58 lines

DaveR

Quoted 54 lines · On 18 Feb 2021, at 16:56, [email protected] wrote:

From: Dave Rigby <[email protected]>

When locating the dwarf module for a given address, __find_debuginfo() requires a 'struct dso' passed via the userdata argument.

However, this field is only set in __report_module() if the module is found in via dwfl_addrmodule(), not if it is found later via dwfl_report_elf().

Set userdata irrespective of how the dwarf module was found, as long as we found a module.

Fixes bf53fc6b5f41 ("perf unwind: Fix separate debug info files when using elfutils' libdw's unwinder")
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jan Kratochvil <[email protected]>
Cc Jiri Olsa <[email protected]>

---
tools/perf/util/unwind-libdw.c | 11 ++++++++---

1 file changed, 8 insertions(+), 3 deletions(-)
tools/perf/util/unwind-libdw.c +8 -3
index 0ada907..a74b517 100644
@@ -60,10 +60,8 @@ static int __report_module(struct addr_location *al, u64 ip,
 	mod = dwfl_addrmodule(ui->dwfl, ip);
 	if (mod) {
 		Dwarf_Addr s;
-		void **userdatap;
 
-		dwfl_module_info(mod, &userdatap, &s, NULL, NULL, NULL, NULL, NULL);
-		*userdatap = dso;
+		dwfl_module_info(mod, NULL, &s, NULL, NULL, NULL, NULL, NULL);
 		if (s != al->map->start - al->map->pgoff)
 			mod = 0;
 	}
@@ -79,6 +77,13 @@ static int __report_module(struct addr_location *al, u64 ip,
 					      al->map->start - al->map->pgoff, false);
 	}
 
+	if (mod) {
+		void **userdatap;
+
+		dwfl_module_info(mod, &userdatap, NULL, NULL, NULL, NULL, NULL, NULL);
+		*userdatap = dso;
+	}
+
 	return mod && dwfl_addrmodule(ui->dwfl, ip) == mod ? 0 : -1;
 }
 
Signature · 1 line
1.8.3.1
Signature · 1 line
- Arnaldo
Acked-by on v1 1/1
#5 lore raw
In reply to [email protected]
Quoted 8 lines · On Thu, 18 Feb 2021 17:56:54 +0100, [email protected] wrote:

From: Dave Rigby <[email protected]>

When locating the dwarf module for a given address, __find_debuginfo() requires a 'struct dso' passed via the userdata argument.

However, this field is only set in __report_module() if the module is found in via dwfl_addrmodule(), not if it is found later via dwfl_report_elf().

Quoted 8 lines

Set userdata irrespective of how the dwarf module was found, as long as we found a module.

Fixes bf53fc6b5f41 ("perf unwind: Fix separate debug info files when using elfutils' libdw's unwinder")
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jan Kratochvil <[email protected]>
Cc Jiri Olsa <[email protected]>

Sorry for the regression,
Jan