bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
	<dwarves@vger.kernel.org>
Cc: "Alexei Starovoitov" <ast@kernel.org>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Bill Wendling" <morbo@google.com>,
	bpf@vger.kernel.org, "David Blaikie" <dblaikie@gmail.com>,
	"Fāng-ruì Sòng" <maskray@google.com>,
	kernel-team@fb.com, "Nick Desaulniers" <ndesaulniers@google.com>
Subject: [PATCH dwarves 1/2] dwarf_loader: check .debug_abbrev for cross-cu references
Date: Wed, 31 Mar 2021 19:58:20 -0700	[thread overview]
Message-ID: <20210401025820.2254482-1-yhs@fb.com> (raw)
In-Reply-To: <20210401025815.2254256-1-yhs@fb.com>

Commit 39227909db3c checked compilation flags to see
whether the binary is built with lto or not (-flto).
Currently, for clang lto build, default setting
won't put compilation flags in dwarf due to size
concern.

David Blaikie suggested in [1] to scan through .debug_abbrev
for DW_FORM_ref_addr which should be most faster than
scanning through cu's. This patch implemented this
suggestion and replaced the previous compilation flag
matching approach. Indeed, it seems that the overhead
for this approach is indeed managable.

I did some change to measure the overhead of cus_merging_cu():
  @@ -2650,7 +2652,15 @@ static int cus__load_module(struct cus *cus, struct conf_load *conf,
                  }
          }

  -       if (cus__merging_cu(dw)) {
  +       bool do_merging;
  +       struct timeval start, end;
  +       gettimeofday(&start, NULL);
  +       do_merging = cus__merging_cu(dw);
  +       gettimeofday(&end, NULL);
  +       fprintf(stderr, "%ld %ld -> %ld %ld\n", start.tv_sec, start.tv_usec,
  +                       end.tv_sec, end.tv_usec);
  +
  +       if (do_merging) {
                  res = cus__merge_and_process_cu(cus, conf, mod, dw, elf, filename,
                                                  build_id, build_id_len,
                                                  type_cu ? &type_dcu : NULL);

For lto vmlinux, the cus__merging_cu() checking takes
130us over total "pahole -J vmlinux" time 65sec as the function bail out
earlier due to detecting a merging cu condition.
For non-lto vmlinux, the cus__merging_cu() checking takes
~171368us over total pahole time 36sec, roughly 0.5% overhead.

 [1] https://lore.kernel.org/bpf/20210328064121.2062927-1-yhs@fb.com/

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 dwarf_loader.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index c1ca1a3..bd23751 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2503,35 +2503,40 @@ static int cus__load_debug_types(struct cus *cus, struct conf_load *conf,
 
 static bool cus__merging_cu(Dwarf *dw)
 {
-	uint8_t pointer_size, offset_size;
 	Dwarf_Off off = 0, noff;
 	size_t cuhl;
-	int cnt = 0;
 
-	/*
-	 * Just checking the first cu is not enough.
-	 * In linux, some C files may have LTO is disabled, e.g.,
-	 *   e242db40be27  x86, vdso: disable LTO only for vDSO
-	 *   d2dcd3e37475  x86, cpu: disable LTO for cpu.c
-	 * Fortunately, disabling LTO for a particular file in a LTO build
-	 * is rather an exception. Iterating 5 cu's to check whether
-	 * LTO is used or not should be enough.
-	 */
-	while (dwarf_nextcu(dw, off, &noff, &cuhl, NULL, &pointer_size,
-			    &offset_size) == 0) {
+	while (dwarf_nextcu (dw, off, &noff, &cuhl, NULL, NULL, NULL) == 0) {
 		Dwarf_Die die_mem;
 		Dwarf_Die *cu_die = dwarf_offdie(dw, off + cuhl, &die_mem);
 
 		if (cu_die == NULL)
 			break;
 
-		if (++cnt > 5)
-			break;
+		Dwarf_Off offset = 0;
+		while (true) {
+			size_t length;
+			Dwarf_Abbrev *abbrev = dwarf_getabbrev (cu_die, offset, &length);
+			if (abbrev == NULL || abbrev == DWARF_END_ABBREV)
+				break;
 
-		const char *producer = attr_string(cu_die, DW_AT_producer);
-		if (strstr(producer, "clang version") != NULL &&
-		    strstr(producer, "-flto") != NULL)
-			return true;
+			size_t attrcnt;
+			if (dwarf_getattrcnt (abbrev, &attrcnt) != 0)
+				return false;
+
+			unsigned int attr_num, attr_form;
+			Dwarf_Off aboffset;
+			size_t j;
+			for (j = 0; j < attrcnt; ++j) {
+				if (dwarf_getabbrevattr (abbrev, j, &attr_num, &attr_form,
+							 &aboffset))
+					return false;
+				if (attr_form == DW_FORM_ref_addr)
+					return true;
+			}
+
+			offset += length;
+		}
 
 		off = noff;
 	}
-- 
2.30.2


  reply	other threads:[~2021-04-01  2:59 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01  2:58 [PATCH dwarves 0/2] dwarf_loader: improve cus__merging_cu() Yonghong Song
2021-04-01  2:58 ` Yonghong Song [this message]
2021-04-01 18:52   ` [PATCH dwarves 1/2] dwarf_loader: check .debug_abbrev for cross-cu references Nick Desaulniers
2021-04-01 19:36     ` Arnaldo
2021-04-01 20:54       ` Yonghong Song
2021-04-01  2:58 ` [PATCH dwarves 2/2] dwarf_loader: check .notes section for lto build info Yonghong Song
2021-04-01 12:59 ` [PATCH dwarves 0/2] dwarf_loader: improve cus__merging_cu() Arnaldo Carvalho de Melo
2021-04-01 18:50 ` Nick Desaulniers
2021-04-01 19:35 ` Bill Wendling
2021-04-01 20:56   ` Bill Wendling
2021-04-01 21:59     ` Yonghong Song
2021-04-02 19:44       ` Bill Wendling
2021-04-06 13:22         ` Arnaldo Carvalho de Melo
2021-04-06 13:23           ` Arnaldo Carvalho de Melo
2021-04-06 16:40           ` Arnaldo Carvalho de Melo
2021-04-06 16:55             ` Arnaldo Carvalho de Melo
2021-04-06 16:59               ` Arnaldo Carvalho de Melo
2021-04-06 17:03                 ` Arnaldo Carvalho de Melo
2021-04-06 17:07                   ` Arnaldo Carvalho de Melo
2021-04-06 17:25                     ` Arnaldo Carvalho de Melo
2021-04-06 17:23                   ` Yonghong Song
2021-04-06 17:34                     ` Arnaldo Carvalho de Melo
2021-04-06 17:36                       ` Arnaldo Carvalho de Melo
2021-04-06 17:48                     ` Yonghong Song
2021-04-06 18:03                       ` Arnaldo Carvalho de Melo
2021-04-06 18:04                         ` Arnaldo Carvalho de Melo
2021-04-06 18:31                           ` Nick Desaulniers
2021-04-06 18:17                         ` David Blaikie
2021-04-06 18:20                           ` Arnaldo Carvalho de Melo
2021-04-06 18:22                           ` Fāng-ruì Sòng
2021-04-06 17:09     ` Jiri Olsa
2021-04-02 13:27   ` Arnaldo Carvalho de Melo
2021-04-02 14:34     ` Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210401025820.2254482-1-yhs@fb.com \
    --to=yhs@fb.com \
    --cc=andrii@kernel.org \
    --cc=arnaldo.melo@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dblaikie@gmail.com \
    --cc=dwarves@vger.kernel.org \
    --cc=kernel-team@fb.com \
    --cc=maskray@google.com \
    --cc=morbo@google.com \
    --cc=ndesaulniers@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).