Re: patch rfc - valgrind vs. debuginfod
Hi Frank, On irc we discussed some issues with trying to disable debuginfod client for valgrind while actually using it in the to be tested executable. The main issue is that valgrind and the test executable are actually one and the same. So disabling or enabling DEBUGINFOD_URLS for one also disables or enables it for the other. So maybe it is just simpler to disable testing under valgrind for those tests that are testing the debuginfod client cache dir directly. That is what the attached patch does. It is pity to not have valgrind run over all the tests, but it is the simplest I could come up with. And this has given us more headaches than we deserve in the first place. What do you think? Thanks, Mark From ad3c57cfb68d1ef5f6a1d426578d8005048c251e Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Fri, 2 Jul 2021 18:16:00 +0200 Subject: [PATCH] run-debuginfod-find.sh: Disable valgrind for debuginfod client cache tests valgrind itself might use the debuginfod client. So disable valgrind when testing the cache. Signed-off-by: Mark Wielaard --- tests/ChangeLog | 5 + tests/run-debuginfod-find.sh | 5 + 2 files changed, 10 insertions(+) diff --git a/tests/ChangeLog b/tests/ChangeLog index d8fa97fa..a0e4ec52 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2021-07-02 Mark Wielaard + + * run-debuginfo-find.sh: unset VALGRIND_CMD before testing debuginfod + client cache. + 2021-06-16 Frank Ch. Eigler * run-debuginfod-find.sh: Fix intermittent groom/stale failure, diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index 456dc2f8..74a5ceff 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -583,6 +583,11 @@ curl -s http://127.0.0.1:$PORT2/buildid/deadbeef/debuginfo > /dev/null || true curl -s http://127.0.0.1:$PORT2/buildid/deadbeef/badtype > /dev/null || true (curl -s http://127.0.0.1:$PORT2/metrics | grep 'badtype') && false +# DISABLE VALGRIND checking because valgrind might use debuginfod client +# requests itself, causing confusion about who put what in the cache. +# It stays disabled till the end of this test. +unset VALGRIND_CMD + # Confirm that reused curl connections survive 404 errors. # The rm's force an uncached fetch rm -f $DEBUGINFOD_CACHE_PATH/$BUILDID/debuginfo .client_cache*/$BUILDID/debuginfo -- 2.18.4
Re: patch rfc - valgrind vs. debuginfod
Hi - > On irc we discussed some issues with trying to disable debuginfod > client for valgrind while actually using it in the to be tested > executable. The main issue is that valgrind and the test executable are > actually one and the same. So disabling or enabling DEBUGINFOD_URLS for > one also disables or enables it for the other. Yeah, it makes sense to disable this testing. You're doing it late enough that -some- valgrinding will still happen, so lgtm. - FChE
PR: 25978
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.
+
2021-06-03 Frank Ch. Eigler
PR27863
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 543044c6..431605aa 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,16 +1219,24 @@ 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));
-set_metric("fdcache_count", lru.size());
+ 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", fdcache_mb);
+set_metric("fdcache_prefetch_bytes", prefetch_mb*1024.0*1024.0);
+set_metric("fdcache_prefetch_count", prefetch_mb);
}
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
+{
+ 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
}
}
@@ -1229,13 +1267,13 @@ public:
fdcache_entry n = { a, b, fd, mb };
if (front_p)
{
- inc_metric("fdcache_op_count","op","enqueue_front");
+ inc_metric("fdcache_op_count","op","lru_enqueue");
lru.push_front(n);
}
else
{
- inc_metric("fdcache_op_count","op","enqueue_back");
- lru.push_back(n);
+ inc_metric("fdcache_op_count","op","prefetch_enqueue");
+ prefetch.push_front(n);
}
if (verbose > 3)
obatched(clog) << "fdcache interned a=" << a << " b=" << b
@@ -1248,10 +1286,10 @@ public:
{
inc_metric("fdcache_op_count","op","emerg-flush");
obatched(clog) << "fdcache emergency flush for filling tmpdir" << endl;
-this->limit(0, 0); // emergency
Re: [PATCH] libdwfl: add dwfl_local_find_debuginfo callback
Hi Nick,
Sorry for the late reply.
On Sun, Jun 13, 2021 at 11:27:02AM +0800, Nick Gasson via Elfutils-devel wrote:
> This patch adds a variant of dwfl_standard_find_debuginfo that only
> searches the local filesystem and does not fall back to debuginfod.
>
> In my application I am using libdw to decode stack traces where some of
> the loaded DSOs are generated on-the-fly by LLVM. If these don't have
> debug information I want the find_debuginfo callback to fail immediately
> and not fall back to debuginfod which will never succeed. Similarly if
> the main executable is stripped. I supposed I could unset the
> DEBUGINFOD_URLS environment variable before calling into libdw, but that
> seems a bit hacky and liable to break in the future if the debuginfod
> client loads the URLs from a different place.
I don't think the (un)setting of DEBUGINFOD_URLS will ever change. It
is API now. But I can see a need for this.
I wonder if we can use the Dwfl_Callbacks debuginfo_path for this
instead of introducing a dwfl_standard_find_debuginfo variant. That
might make things a bit more configurable.
Something like the following maybe:
- If the last path element is "@" then the debuginfo client won't be used.
- If the last path element is "@{URLs}" (space separated) then the debuginfo
client
will be used with to given URLs.
- If there is no path element starting with @ then the default debuginfo client
will be used after all local searches failed (or not when DEBUGINFOD_URLS
isn't set).
Does that sound helpful? Does it make sense to make things configurable like
this?
Thanks,
Mark
Re: [PATCH] debuginfod-client: Fix client dereference when calloc fails.
On Fri, Jun 18, 2021 at 03:02:43PM +0200, Mark Wielaard wrote: > When the calloc call in debuginfod_begin fails we should skip all > initialization of the client handle. Pushed.
Re: [PATCH] strip: Always check gelf_update results.
On Fri, Jun 18, 2021 at 03:06:32PM +0200, Mark Wielaard wrote: > +2021-06-18 Mark Wielaard > + > + * strip.c (remove_debug_relocations): Check gelf_update results. > + (update_section_size): Likewise. Pushed.
Re: [PATCH] unstrip: Always check gelf_getrel[a] results
On Fri, Jun 18, 2021 at 03:08:48PM +0200, Mark Wielaard wrote: > +2021-06-18 Mark Wielaard > + > + * unstrip.c (adjust_relocs): Check gelf_getrel and geld_getrela. > + Pushed.
