Re: [PATCH] libelf: Fix unaligned d_off offsets for input sections with large alignments
Hi Andrei,
On Wed, 2021-07-07 at 13:24 -0700, Andrei Homescu wrote:
> I wrote this patch on behalf of someone else, so the copyright line
> should be:
> Copyright (C) 2021 Runsafe Security, Inc.
OK, changed.
> > The testcase already fails before the patch and succeeds after, but
> > it would be nice to double check the output with elflint just in
> > case. Shall we add:
> >
> > testrun ${abs_top_builddir}/src/elflint --gnu $outfile
>
> Sounds good, no objection from me.
> Should I submit an updated version of the patch, or can you add this
> on your end?
I made those two tweaks and pushed as:
https://sourceware.org/git/?p=elfutils.git;a=commit;h=b3601167d7a4c9f34eb65c3892c9ef25e3c1c66f
Thanks,
Mark
Buildbot failure in Wildebeest Builder on whole buildset
The Buildbot has detected a new failure on builder elfutils-centos-x86_64 while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/1/builds/784 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: centos-x86_64 Build Reason: Blamelist: Andrei Homescu BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a new failure on builder elfutils-debian-amd64 while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/2/builds/778 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-amd64 Build Reason: Blamelist: Andrei Homescu BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a new failure on builder elfutils-fedora-x86_64 while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/3/builds/783 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-x86_64 Build Reason: Blamelist: Andrei Homescu BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a new failure on builder elfutils-debian-i386 while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/4/builds/778 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: debian-i386 Build Reason: Blamelist: Andrei Homescu BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a new failure on builder elfutils-fedora-s390x while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/10/builds/749 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-s390x Build Reason: Blamelist: Andrei Homescu BUILD FAILED: failed test (failure) Sincerely, -The Buildbot
Re: Buildbot failure in Wildebeest Builder on whole buildset
On Thu, 2021-07-08 at 09:53 +, build...@builder.wildebeest.org wrote: > The Buildbot has detected a new failure on builder elfutils-centos- > x86_64 while building elfutils. > Full details are available at: > https://builder.wildebeest.org/buildbot/#builders/1/builds/784 > > Buildbot URL: https://builder.wildebeest.org/buildbot/ > > Worker for this Build: centos-x86_64 > > Build Reason: > Blamelist: Andrei Homescu > > BUILD FAILED: failed test (failure) Oops. Thanks buildbot. Typo in EXTRA_DIST that only shows up when running make distcheck. Fixed with the attached commit. Cheers, Mark From cdcd7dc3d20a002abe1ce318f9b9d0895eee1810 Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 8 Jul 2021 11:44:26 +0200 Subject: [PATCH] tests: Fix EXTRA_DIST typo in testfile-largealign.o.bz2 Signed-off-by: Mark Wielaard --- tests/ChangeLog | 5 + tests/Makefile.am | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/ChangeLog b/tests/ChangeLog index 28aaf85e..7b493c99 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2021-07-08 Mark Wielaard + + * Makefile.am (EXTRA_DIST): Fix typo testfile-largealign.bz2 was + was missing .o. + 2021-06-09 Andrei Homescu * testfile-largealign.o.bz2: New test file. diff --git a/tests/Makefile.am b/tests/Makefile.am index 8ac0d2e6..429649f4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -512,7 +512,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \ run-getphdrnum.sh testfile-phdrs.elf.bz2 \ run-test-includes.sh run-low_high_pc-dw-form-indirect.sh \ run-readelf-dw-form-indirect.sh testfile-dw-form-indirect.bz2 \ - testfile-largealign.bz2 run-strip-largealign.sh + testfile-largealign.o.bz2 run-strip-largealign.sh if USE_VALGRIND -- 2.18.4
Re: PR: 25978
Hi Noah,
Like the other patch this really needs a commit message.
If I understand the code correctly you are introducing another cache
just for prefetches separate from the fdcache. But this new cache is a
part of larger libarchive_fdcache so access to it is shared with the
existing one, it is just the eviction policy that changes for
prefetched files.
I don't have an opinion on the renamed metrics, maybe Frank can say if
it matters how and if they are renamed.
On Fri, 2021-07-02 at 14:24 -0400, Noah Sanci via Elfutils-devel wrote:
> diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
> index 286c910a..06d03e72 100644
> --- a/debuginfod/ChangeLog
> +++ b/debuginfod/ChangeLog
> @@ -1,3 +1,9 @@
> +2021-06-28 Noah Sanci
> +
> + PR25978
> + * debuginfod.cxx: Added command line options
> + --fdcache-prefetch-fds/mbs and associated metrics/functionality.
And I believe the code is good, but I look ChangeLog entries a bit more
verbose so they can be used to check that the changes made were
intentional.
> @@ -538,10 +546,22 @@ parse_opt (int key, char *arg,
>break;
> case ARGP_KEY_FDCACHE_MINTMP:
>fdcache_mintmp = atol (arg);
> + if( fdcache_mintmp > 100 || fdcache_mintmp < 0 )
> +argp_failure(state, 1, EINVAL, "fdcache mintmp percent");
>break;
For example here. This is a good change, but I had to double check
because the rest of the patch is about other keys.
> case ARGP_KEY_ARG:
>source_paths.insert(string(arg));
>break;
> +case ARGP_KEY_FDCACHE_PREFETCH_FDS:
> + fdcache_prefetch_fds = atol(arg);
> + if ( fdcache_prefetch_fds <= 0)
> +argp_failure(state, 1, EINVAL, "fdcache prefetch fds");
> + break;
> +case ARGP_KEY_FDCACHE_PREFETCH_MBS:
> + fdcache_prefetch_mbs = atol(arg);
> + if ( fdcache_prefetch_mbs <= 0)
> +argp_failure(state, 1, EINVAL, "fdcache prefetch mbs");
> + break;
Is it intentional that '0' is not allowed?
> void intern(const string& a, const string& b, string fd, off_t sz,
> bool front_p)
> @@ -1221,7 +1249,17 @@ public:
> {
>unlink (i->fd.c_str());
>lru.erase(i);
> - inc_metric("fdcache_op_count","op","dequeue");
> + inc_metric("fdcache_op_count","op","lru_dequeue");
> + break; // must not continue iterating
> +}
> +}
> + for (auto i = prefetch.begin(); i < prefetch.end(); i++) //
> nuke preexisting copy in prefetch
This comment could go on its own line before the for statement to keep
the lines a little shorter.
> +{
> + if (i->archive == a && i->entry == b)
> +{
> + unlink (i->fd.c_str());
> + prefetch.erase(i);
> + inc_metric("fdcache_op_count","op","prefetch_dequeue")
> ;
>break; // must not continue iterating
> }
This is already in the existing code, but the names 'a' and 'b' are not
really that helpful. They are the archive and filename. If you know
that, then the code does make sense.
> @@ -1266,21 +1304,32 @@ public:
>fdcache_entry n = *i;
>lru.erase(i); // invalidates i, so no more iteration!
>lru.push_front(n);
> - inc_metric("fdcache_op_count","op","requeue_front");
> + inc_metric("fdcache_op_count","op","lru_requeue_front"
> );
> + fd = open(n.fd.c_str(), O_RDONLY); // NB: no problem
> if
> dup() fails; looks like cache miss
This comment could also be on its own line.
I don't understand it though. Where does the dup happen?
Did you mean if open () fails, it is no problem that fd becomes -1?
> + break;
> +}
> +}
> + for ( auto i = prefetch.begin(); fd == -1 && i <
> prefetch.end(); ++i)
Should this be fd != -1 ?
> +{
> + if (i->archive == a && i->entry == b)
> +{ // found it; take the entry from the prefetch deque to
> the lru deque, since it has now been accessed.
> + fdcache_entry n = *i;
> + prefetch.erase(i);
> + lru.push_front(n);
> + inc_metric("fdcache_op_count","op","prefetch_access");
>fd = open(n.fd.c_str(), O_RDONLY); // NB: no problem
> if
> dup() fails; looks like cache miss
>break;
> }
> }
> }
> -
Please try to avoid spurious whitespace changes, they make the diffs
bigger than necessary.
> index 1ba42cf6..8945eb9b 100644
> --- a/doc/debuginfod.8
> +++ b/doc/debuginfod.8
> @@ -212,6 +212,16 @@ $TMPDIR or \fB/tmp\fP filesystem. This is
> because that is where the
> most recently used extracted files are kept. Grooming cleans this
> cache.
>
> +.TP
> +.B "\-\-fdcache\-\-prefetch\-fds=NUM" "\-\-fdcache\-\-prefetch\-
> mbs=MB"
> +Configure how many file descriptors (fds) and megabytes (mbs) are
> +allocated to the prefetch portion of the fdcache. If unspecified,
> +
Re: PR: 25978
Hi - > If they are portions of the full fdcache shouldn't there be a check in > the code that the specified fdcache_prefetch_fds and > fdcache_prefetch_mbs aren't larger than fdcache_fds and fdcache_mbs? Or > maybe they should be given as percentages? We've iterated on a couple of ways of representing & controlling this. The code makes the lru and prefetch caches separate, in that they have separate limits and separate metrics. So there is no requirement for the prefetch one to be smaller than the other. (We will probably want to tweak the defaults to be a little larger at some point.) - FChE
Re: [PATCH] PR27531: retry within default retry_limit will be supported. In debuginfod-client.c (debuginfod_query_server), insert a goto statement for jumping back to the beginning of curl handles set
Hi Alice,
On Tue, 2021-07-06 at 16:15 -0400, Alice Zhang via Elfutils-devel
wrote:
> Also introduced DEBUGINFOD_RETRY_LIMIT_ENV_VAR and default
> DEBUGINFOD_RETRY_LIMIT(which is 2).
>
> Correponding test has been added to tests/run-debuginfod-find.sh
>
> Signed-off-by: Alice Zhang
Nice. But try to split up the first paragraph. e.g.
---
PR27531: retry within default retry_limit will be supported.
In debuginfod-client.c (debuginfod_query_server), insert a
goto statement for jumping back to the beginning of curl
handles set up if query fails and a non ENOENT error is
returned.
Also introduced DEBUGINFOD_RETRY_LIMIT_ENV_VAR and default
DEBUGINFOD_RETRY_LIMIT (which is 2).
Correponding test has been added to tests/run-debuginfod-find.sh
Signed-off-by: Alice Zhang
---
That way the first sentence (note the extra blank line) becomes the
short commit message/subject.
+ /* If the verified_handle is NULL and rc != -ENOENT, the query
> fails with
> + * an error code other than 404, then do several retry within the
> retry_limit.
> + * Clean up all old handles and jump back to the beginning of
> query_in_parallel,
> + * reinitialize handles and query again.*/
>if (verified_handle == NULL)
> -goto out1;
> +{
> + if (rc != -ENOENT && retry_limit-- > 0)
> +{
> + if (vfd >= 0)
> +dprintf (vfd, "Retry failed query, %d attempt(s) remaining\n",
> retry_limit);
> + /* remove all handles from multi */
> + for (int i = 0; i < num_urls; i++)
> +{
> + curl_multi_remove_handle(curlm, data[i].handle); /* ok to
> repeat */
> + curl_easy_cleanup (data[i].handle);
> +}
> + goto query_in_parallel;
> + }
> + else
> + goto out1;
> +}
You also need to call free (data) before the goto query_in_parallel
since that will also be reallocated. Or you can rearrange the code a
little around query_in_parallel to keep the data around, but only
reinitialize it, but I think it is fine to free and then malloc again.
Try to run under valgrind --full-leak-check and you'll see:
==24684== 43,840 bytes in 10 blocks are definitely lost in loss record 61 of 61
==24684==at 0x4C29F73: malloc (vg_replace_malloc.c:309)
==24684==by 0x52EB2A7: debuginfod_query_server (debuginfod-client.c:789)
==24684==by 0x401131: main (debuginfod-find.c:192)
(P.S. Don't worry about the still reachable issues.)
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
> index 559ea947..282e523d 100644
> --- a/debuginfod/debuginfod.h.in
> +++ b/debuginfod/debuginfod.h.in
> @@ -35,6 +35,7 @@
> #define DEBUGINFOD_TIMEOUT_ENV_VAR "DEBUGINFOD_TIMEOUT"
> #define DEBUGINFOD_PROGRESS_ENV_VAR "DEBUGINFOD_PROGRESS"
> #define DEBUGINFOD_VERBOSE_ENV_VAR "DEBUGINFOD_VERBOSE"
> +#define DEBUGINFOD_RETRY_LIMIT_ENV_VAR "DEBUGINFOD_RETRY_LIMIT"
>
> /* The libdebuginfod soname. */
> #define DEBUGINFOD_SONAME "@LIBDEBUGINFOD_SONAME@"
Could you also add an entry for DEBUGINFOD_RETRY_LIMIT to
doc/debuginfod_find_debuginfo.3 under ENVIRONMENT VARIABLES.
> +
>
> +# set up tests for retrying failed queries.
> +retry_attempts=`(testrun env DEBUGINFOD_URLS=
> http://255.255.255.255/JUNKJUNK DEBUGINFOD_RETRY_LIMIT=10
> DEBUGINFOD_VERBOSE=1 \
> + ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo
> /bin/ls || true) 2>&1 >/dev/null \
> + | grep -c 'Retry failed query'`
> +if [ $retry_attempts -ne 10 ]; then
> +echo "retry mechanism failed."
> +exit 1;
> + fi
> +
> exit 0
Cute testcase. This works because 255.255.255.255 will always fail.
Thanks,
Mark
PR27711
Hello,
Find the patch information for pr27711 attached .
Noah Sanci
From f126b48bf1131070d80e063bfd296ddb69af8c9a Mon Sep 17 00:00:00 2001
From: Noah
Date: Wed, 7 Jul 2021 14:40:10 -0400
Subject: [PATCH] debuginfod: PR27711 - Use -I/-X regexes during groom phase
The debuginfod -I/-X regexes operate during traversal to identify
those files in need of scanning. The regexes are not used during
grooming. This means that if from run to run, the regex changes so
that formerly indexed files are excluded from traversal, the data is
still retained in the index.
This is both good and bad. On one hand, if the underlying data is
still available, grooming will preserve the data, and let clients ask
for it. On the other hand, if the growing index size is a problem,
and one wishes to age no-longer-regex-matching index data out, there
is no way.
Let's add a debuginfod flag to use regexes during grooming.
Specifically, in groom(), where the stat() test exists, also check
for regex matching as in scan_source_paths(). Treat failure of the
regex the same way as though the file didn't exist.
Signed-off-by: Noah Sanci
---
debuginfod/ChangeLog | 8
debuginfod/debuginfod.cxx| 11 --
doc/debuginfod.8 | 3 +++
tests/ChangeLog | 5 +
tests/run-debuginfod-find.sh | 39 ++--
5 files changed, 58 insertions(+), 8 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 286c910a..29d3e815 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,11 @@
+2021-07-01 Noah Sanci
+
+ PR27711
+ * debuginfod.cxx (options): Add --regex-groon, -r option.
+ (regex_groom): New static bool defaults to false.
+ (parse_opt): Handle 'r' option by setting regex_groom to true.
+ (groom): Introduce and use reg_include and reg_exclude.
+
2021-06-03 Frank Ch. Eigler
PR27863
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 543044c6..4f7fd2d5 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -360,6 +360,7 @@ static const struct argp_option options[] =
{ "database", 'd', "FILE", 0, "Path to sqlite database.", 0 },
{ "ddl", 'D', "SQL", 0, "Apply extra sqlite ddl/pragma to connection.", 0 },
{ "verbose", 'v', NULL, 0, "Increase verbosity.", 0 },
+ { "regex-groom", 'r', NULL, 0,"Uses regexes from -I and -X arguments to groom the database.",0},
#define ARGP_KEY_FDCACHE_FDS 0x1001
{ "fdcache-fds", ARGP_KEY_FDCACHE_FDS, "NUM", 0, "Maximum number of archive files to keep in fdcache.", 0 },
#define ARGP_KEY_FDCACHE_MBS 0x1002
@@ -407,6 +408,7 @@ static map scan_archives;
static vector extra_ddl;
static regex_t file_include_regex;
static regex_t file_exclude_regex;
+static bool regex_groom = false;
static bool traverse_logical;
static long fdcache_fds;
static long fdcache_mbs;
@@ -527,6 +529,9 @@ parse_opt (int key, char *arg,
if (rc != 0)
argp_failure(state, 1, EINVAL, "regular expression");
break;
+case 'r':
+ regex_groom = true;
+ break;
case ARGP_KEY_FDCACHE_FDS:
fdcache_fds = atol (arg);
break;
@@ -3249,8 +3254,11 @@ void groom()
int64_t fileid = sqlite3_column_int64 (files, 1);
const char* filename = ((const char*) sqlite3_column_text (files, 2) ?: "");
struct stat s;
+ bool reg_include = !regexec (&file_include_regex, filename, 0, 0, 0);
+ bool reg_exclude = !regexec (&file_exclude_regex, filename, 0, 0, 0);
+
rc = stat(filename, &s);
- if (rc < 0 || (mtime != (int64_t) s.st_mtime))
+ if ( (regex_groom && reg_exclude && !reg_include) || rc < 0 || (mtime != (int64_t) s.st_mtime) )
{
if (verbose > 2)
obatched(clog) << "groom: forgetting file=" << filename << " mtime=" << mtime << endl;
@@ -3261,7 +3269,6 @@ void groom()
}
else
inc_metric("groomed_total", "decision", "fresh");
-
if (sigusr1 != forced_rescan_count) // stop early if scan triggered
break;
}
diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
index 1ba42cf6..1adf703a 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -159,6 +159,9 @@ scan, independent of the rescan time (including if it was zero),
interrupting a groom pass (if any).
.TP
+.B "\-r"
+Apply the -I and -X during groom cycles, so that files excluded by the regexes are removed from the index. These parameters are in addition to what normally qualifies a file for grooming, not a replacement.
+
.B "\-g SECONDS" "\-\-groom\-time=SECONDS"
Set the groom time for the index database. This is the amount of time
the grooming thread will wait after finishing a grooming pass before
diff --git a/tests/ChangeLog b/tests/ChangeLog
index d8fa97fa..346b9e6e 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2021-07-01 Noah Sanci
+ PR2711
+ * run-debuginfod-find.sh: Added test case for grooming the database
+ using regexes.
+
202
Re: Working with ELF already loaded in memory
Hi Sinal, On Thu, 2021-07-08 at 05:02 +, Sonal Santan via Elfutils-devel wrote: > Going through the libdw it appears that all APIs require either a > file handle or a file name of the ELF object to create a session. > Since we do not have access to the ELF file -- but rather the ELF > file contents are already loaded in memory -- is there any other > mechanism to create a session for extracting DWARF information using > libdw? Yes, if you just need the information already loaded into memory and you know where the library is mapped you can use: /* Create descriptor for memory region. */ extern Elf *elf_memory (char *__image, size_t __size); You can then use that Elf handle to extract the information that has been mapped in. Which often is not the actual debug information though. If you have a Elf handle you can use: /* Returns the build ID as found in a NT_GNU_BUILD_ID note from either a SHT_NOTE section or from a PT_NOTE segment if the ELF file doesn't contain any section headers. On success a pointer to the build ID is written to *BUILDID_P, and the positive length of the build ID is returned. Returns 0 if the ELF lacks a NT_GNU_BUILD_ID note. Returns -1 in case of malformed data or other errors. */ extern ssize_t dwelf_elf_gnu_build_id (Elf *elf, const void **build_idp); You can then use the build_id to lookup the debug information (file). You can also use libdwfl (part of libdw) to do some of the above automagically. See for example: /* Call dwfl_report_module for each file mapped into the address space of PID. Returns zero on success, -1 if dwfl_report_module failed, or an errno code if opening the proc files failed. */ extern int dwfl_linux_proc_report (Dwfl *dwfl, pid_t pid); The Dwfl will then be a representation of the modules (executable and shared libraries) of that particular process. You can then iterate through those modules using dwfl_getmodules and get a Dwarf handle using dwfl_module_getdwarf (or for all with dwfl_getdwarf). libdw will then try to extract that build-id from each module and try various lookups to get the (separate on disk) debuginfo. Hope that helps, Mark
Re: PR: 25978
Hi, On Thu, 2021-07-08 at 09:12 -0400, Frank Ch. Eigler wrote: > > If they are portions of the full fdcache shouldn't there be a check > > in the code that the specified fdcache_prefetch_fds and > > fdcache_prefetch_mbs aren't larger than fdcache_fds and fdcache_mbs? Or > > maybe they should be given as percentages? > > We've iterated on a couple of ways of representing & controlling this. > The code makes the lru and prefetch caches separate, in that they have > separate limits and separate metrics. So there is no requirement for > the prefetch one to be smaller than the other. Aha, yes, that does look like how the code actually works. Lets make sure the documentation matches that description (or at least remove the phrase "portion of"). Cheers, Mar
Re: PR27711
Hi Noah, On Thu, 2021-07-08 at 10:48 -0400, Noah Sanci via Elfutils-devel wrote: > Find the patch information for pr27711 attached . Looks good. Pushed. Thanks, Mark
Re: [PATCH] libelf: Fix unaligned d_off offsets for input sections with large alignments
On Thu, Jul 8, 2021 at 2:37 AM Mark Wielaard wrote:
> Hi Andrei,
>
> On Wed, 2021-07-07 at 13:24 -0700, Andrei Homescu wrote:
> > I wrote this patch on behalf of someone else, so the copyright line
> > should be:
> > Copyright (C) 2021 Runsafe Security, Inc.
>
> OK, changed.
>
> > > The testcase already fails before the patch and succeeds after, but
> > > it would be nice to double check the output with elflint just in
> > > case. Shall we add:
> > >
> > > testrun ${abs_top_builddir}/src/elflint --gnu $outfile
> >
> > Sounds good, no objection from me.
> > Should I submit an updated version of the patch, or can you add this
> > on your end?
>
> I made those two tweaks and pushed as:
>
>
> https://sourceware.org/git/?p=elfutils.git;a=commit;h=b3601167d7a4c9f34eb65c3892c9ef25e3c1c66f
>
> Thanks,
>
> Mark
>
That's great, thank you!
Andrei
[debuginfod/25978]
Hello,
Please find the patch information for pr25978 attached.
Noah Sanci
From af80431af96b3d1f921df264672829db71b46d20 Mon Sep 17 00:00:00 2001
From: Noah
Date: Thu, 10 Jun 2021 10:29:45 -0400
Subject: [PATCH] debuginfod: PR25978 - Created the prefetch fdcache
The debuginfod fdcache-prefetch logic has been observed to show some
degeneracies in operation. Since fdcache evictions are done
frequently, and freshly prefetched archive elements are put at the
back of lru[], each eviction round can summarily nuke things that
were just prefetched and are just going to be prefetched again.
It would be better to have two lru lists, or being able to insert
newly prefetched entries somewhere in the middle of the list rather
than at the very very end.
https://sourceware.org/bugzilla/show_bug.cgi?id=25978
Signed-off-by: Noah Sanci
---
debuginfod/ChangeLog | 15
debuginfod/debuginfod.cxx| 146 +--
doc/debuginfod.8 | 10 +++
tests/ChangeLog | 7 ++
tests/run-debuginfod-find.sh | 25 +-
5 files changed, 178 insertions(+), 25 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 286c910a..02ad34b4 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,18 @@
+2021-06-28 Noah Sanci
+
+ PR25978
+ * debuginfod.cxx (options): Added --fdcache-prefetch-fds/mbs options.
+ (set_metric): Added a condition for fdcache_mintmp to ensure no
+ negative percentages or percentages larger than 100% are given.
+ (globals): Added fdcache_prefetch_mbs/fdcache_prefetch_fds.
+ (set_metrics): Differentiate between lru and prefetch metrics.
+ (intern): Added prefetch functionality for nuking preexisting copies
+ and incrementing prefetch metrics.
+ (lookup): Search prefetch cache and increment associated metrics. Upon
+ finding in the prefetch cache move the element to the lru cache.
+ (limit): Arguments updated. Update size of prefetch cache.
+ (main): Log prefetch and cache fds/mbs
+
2021-06-03 Frank Ch. Eigler
PR27863
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 543044c6..c348aa61 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -368,7 +368,13 @@ static const struct argp_option options[] =
{ "fdcache-prefetch", ARGP_KEY_FDCACHE_PREFETCH, "NUM", 0, "Number of archive files to prefetch into fdcache.", 0 },
#define ARGP_KEY_FDCACHE_MINTMP 0x1004
{ "fdcache-mintmp", ARGP_KEY_FDCACHE_MINTMP, "NUM", 0, "Minimum free space% on tmpdir.", 0 },
- { NULL, 0, NULL, 0, NULL, 0 }
+#define ARGP_KEY_FDCACHE_PREFETCH_MBS 0x1005
+ { "fdcache-prefetch-mbs", ARGP_KEY_FDCACHE_PREFETCH_MBS, "MB", 0,"Megabytes allocated to the \
+ prefetch cache.", 0},
+#define ARGP_KEY_FDCACHE_PREFETCH_FDS 0x1006
+ { "fdcache-prefetch-fds", ARGP_KEY_FDCACHE_PREFETCH_FDS, "NUM", 0,"Number of files allocated to the \
+ prefetch cache.", 0},
+ { NULL, 0, NULL, 0, NULL, 0 },
};
/* Short description of program. */
@@ -412,6 +418,8 @@ static long fdcache_fds;
static long fdcache_mbs;
static long fdcache_prefetch;
static long fdcache_mintmp;
+static long fdcache_prefetch_mbs;
+static long fdcache_prefetch_fds;
static string tmpdir;
static void set_metric(const string& key, double value);
@@ -538,10 +546,22 @@ parse_opt (int key, char *arg,
break;
case ARGP_KEY_FDCACHE_MINTMP:
fdcache_mintmp = atol (arg);
+ if( fdcache_mintmp > 100 || fdcache_mintmp < 0 )
+argp_failure(state, 1, EINVAL, "fdcache mintmp percent");
break;
case ARGP_KEY_ARG:
source_paths.insert(string(arg));
break;
+case ARGP_KEY_FDCACHE_PREFETCH_FDS:
+ fdcache_prefetch_fds = atol(arg);
+ if ( fdcache_prefetch_fds < 0)
+argp_failure(state, 1, EINVAL, "fdcache prefetch fds");
+ break;
+case ARGP_KEY_FDCACHE_PREFETCH_MBS:
+ fdcache_prefetch_mbs = atol(arg);
+ if ( fdcache_prefetch_mbs < 0)
+argp_failure(state, 1, EINVAL, "fdcache prefetch mbs");
+ break;
// case 'h': argp_state_help (state, stderr, ARGP_HELP_LONG|ARGP_HELP_EXIT_OK);
default: return ARGP_ERR_UNKNOWN;
}
@@ -1199,23 +1219,32 @@ private:
};
deque lru; // @head: most recently used
long max_fds;
+ deque prefetch; // prefetched
long max_mbs;
+ long max_prefetch_mbs;
+ long max_prefetch_fds;
public:
void set_metrics()
{
-double total_mb = 0.0;
+double fdcache_mb = 0.0;
+double prefetch_mb = 0.0;
for (auto i = lru.begin(); i < lru.end(); i++)
- total_mb += i->fd_size_mb;
-set_metric("fdcache_bytes", (int64_t)(total_mb*1024.0*1024.0));
+ fdcache_mb += i->fd_size_mb;
+for (auto j = prefetch.begin(); j < prefetch.end(); j++)
+ prefetch_mb += j->fd_size_mb;
+set_metric("fdcache_bytes", fdcache_mb*1024.0*1024.0);
set_metric("fdcache_count", lru.size());
+set_metric("fdcache_prefetch_bytes", prefetch_mb*1024.0*1024.0);
+set_metric("fdcac
RE: Working with ELF already loaded in memory
Hello Mark, > -Original Message- > From: Mark Wielaard > Sent: Thursday, July 8, 2021 8:02 AM > To: Sonal Santan ; elfutils-devel@sourceware.org > Subject: Re: Working with ELF already loaded in memory > > Hi Sinal, > > On Thu, 2021-07-08 at 05:02 +, Sonal Santan via Elfutils-devel > wrote: > > Going through the libdw it appears that all APIs require either a file > > handle or a file name of the ELF object to create a session. > > Since we do not have access to the ELF file -- but rather the ELF file > > contents are already loaded in memory -- is there any other mechanism > > to create a session for extracting DWARF information using libdw? > > Yes, if you just need the information already loaded into memory and you know > where the library is mapped you can use: > > /* Create descriptor for memory region. */ extern Elf *elf_memory (char > *__image, size_t __size); > > You can then use that Elf handle to extract the information that has been > mapped in. Which often is not the actual debug information though. > Thanks for the pointer. I was looking for APIs inside /usr/include/elfutils directory and did not realize elfutils also places header files under /usr/include directory. I am working with this API now. > If you have a Elf handle you can use: > > /* Returns the build ID as found in a NT_GNU_BUILD_ID note from either >a SHT_NOTE section or from a PT_NOTE segment if the ELF file >doesn't contain any section headers. On success a pointer to the >build ID is written to *BUILDID_P, and the positive length of the >build ID is returned. Returns 0 if the ELF lacks a NT_GNU_BUILD_ID >note. Returns -1 in case of malformed data or other errors. */ extern > ssize_t > dwelf_elf_gnu_build_id (Elf *elf, const void **build_idp); > > You can then use the build_id to lookup the debug information (file). > > You can also use libdwfl (part of libdw) to do some of the above > automagically. > See for example: > > /* Call dwfl_report_module for each file mapped into the address space of PID. >Returns zero on success, -1 if dwfl_report_module failed, >or an errno code if opening the proc files failed. */ extern int > dwfl_linux_proc_report (Dwfl *dwfl, pid_t pid); Does this also work for ELF files which are loaded into heap as blob -- not really _mapped_ into the address space? I guess I will have to provide some hints to dwfl so it can locate the loaded blob when walking through the process map? Thanks, -Sonal > > The Dwfl will then be a representation of the modules (executable and shared > libraries) of that particular process. You can then iterate through those > modules using dwfl_getmodules and get a Dwarf handle using > dwfl_module_getdwarf (or for all with dwfl_getdwarf). libdw will then try to > extract that build-id from each module and try various lookups to get the > (separate on disk) debuginfo. > > Hope that helps, > > Mark
