[PATCH] debuginfod: Check result of curl_easy_getinfo in debuginfod_write_callback
This was the only place in debuginfod-client.c where we didn't check the result of curl_easy_getinfo. Just check it to make things consistent. Signed-off-by: Mark Wielaard --- debuginfod/ChangeLog | 5 + 1 file changed, 5 insertions(+) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 619ebd8c..91890786 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,8 @@ +2022-05-09 Mark Wielaard + + * debuginfod-client.c (debuginfod_write_callback): Check result + of curl_easy_getinfo. + 2022-05-04 Frank Ch. Eigler Mark Wielaard -- 2.18.4
[PATCHv2] debuginfod: Check result of curl_easy_getinfo in debuginfod_write_callback
This was the only place in debuginfod-client.c where we didn't check
the result of curl_easy_getinfo. Just check it to make things consistent.
Signed-off-by: Mark Wielaard
---
v2 This time with the actual code change...
debuginfod/ChangeLog | 5 +
debuginfod/debuginfod-client.c | 5 +++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 619ebd8c..91890786 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2022-05-09 Mark Wielaard
+
+ * debuginfod-client.c (debuginfod_write_callback): Check result
+ of curl_easy_getinfo.
+
2022-05-04 Frank Ch. Eigler
Mark Wielaard
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 521972e4..882a809a 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -213,8 +213,9 @@ debuginfod_write_callback (char *ptr, size_t size, size_t
nmemb, void *data)
*d->target_handle = d->handle;
/* update the client object */
const char *url = NULL;
- (void) curl_easy_getinfo (d->handle, CURLINFO_EFFECTIVE_URL, &url);
- if (url)
+ CURLcode curl_res = curl_easy_getinfo (d->handle,
+ CURLINFO_EFFECTIVE_URL, &url);
+ if (curl_res == CURLE_OK && url)
{
free (d->client->url);
d->client->url = strdup(url); /* ok if fails */
--
2.18.4
[PATCH] debuginfod: Check all curl_easy_setopt calls
curl_easy_setup can fail for various reasons. Add a curl_easy_setopt_ck
macro to check all curl_easy_setopt calls and provides a human readable
error message in verbose mode.
Signed-off-by: Mark Wielaard
---
debuginfod/ChangeLog | 5 +++
debuginfod/debuginfod-client.c | 61 ++
2 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 91890786..983fd44a 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2022-05-09 Mark Wielaard
+
+ * debuginfod-client.c (debuginfod_query_server): Add
+ curl_easy_setopt_ck macro, use it for all curl_easy_setopt calls.
+
2022-05-09 Mark Wielaard
* debuginfod-client.c (debuginfod_write_callback): Check result
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 882a809a..89208216 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1032,43 +1032,60 @@ debuginfod_query_server (debuginfod_client *c,
if (vfd >= 0)
dprintf (vfd, "url %d %s\n", i, data[i].url);
+ /* Some boilerplate for checking curl_easy_setopt. */
+#define curl_easy_setopt_ck(H,O,P) do {\
+ CURLcode curl_res = curl_easy_setopt (H,O,P);\
+ if (curl_res != CURLE_OK)\
+ { \
+ if (vfd >= 0) \
+ dprintf (vfd, \
+"Bad curl_easy_setopt: %s\n", \
+curl_easy_strerror(curl_res)); \
+ rc = -EINVAL; \
+ goto out2;\
+ } \
+ } while (0)
+
/* Only allow http:// + https:// + file:// so we aren't being
redirected to some unsupported protocol. */
- curl_easy_setopt(data[i].handle, CURLOPT_PROTOCOLS,
- CURLPROTO_HTTP | CURLPROTO_HTTPS | CURLPROTO_FILE);
- curl_easy_setopt(data[i].handle, CURLOPT_URL, data[i].url);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_PROTOCOLS,
+ (CURLPROTO_HTTP | CURLPROTO_HTTPS | CURLPROTO_FILE));
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_URL, data[i].url);
if (vfd >= 0)
- curl_easy_setopt(data[i].handle, CURLOPT_ERRORBUFFER, data[i].errbuf);
- curl_easy_setopt(data[i].handle,
- CURLOPT_WRITEFUNCTION,
- debuginfod_write_callback);
- curl_easy_setopt(data[i].handle, CURLOPT_WRITEDATA, (void*)&data[i]);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_ERRORBUFFER,
+ data[i].errbuf);
+ curl_easy_setopt_ck(data[i].handle,
+ CURLOPT_WRITEFUNCTION,
+ debuginfod_write_callback);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_WRITEDATA, (void*)&data[i]);
if (timeout > 0)
{
/* Make sure there is at least some progress,
try to get at least 100K per timeout seconds. */
- curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_TIME,
- timeout);
- curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_LIMIT,
- 100 * 1024L);
+ curl_easy_setopt_ck (data[i].handle, CURLOPT_LOW_SPEED_TIME,
+ timeout);
+ curl_easy_setopt_ck (data[i].handle, CURLOPT_LOW_SPEED_LIMIT,
+ 100 * 1024L);
}
data[i].response_data = NULL;
data[i].response_data_size = 0;
- curl_easy_setopt(data[i].handle, CURLOPT_FILETIME, (long) 1);
- curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1);
- curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1);
- curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1);
- curl_easy_setopt(data[i].handle, CURLOPT_HEADERFUNCTION,
header_callback);
- curl_easy_setopt(data[i].handle, CURLOPT_HEADERDATA, (void *)
&(data[i]));
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_FILETIME, (long) 1);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_FAILONERROR, (long) 1);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_NOSIGNAL, (long) 1);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_HEADERFUNCTION,
+ header_callback);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_HEADERDATA,
+ (void *) &(data[i]));
#if LIBCURL_VERSION_NUM >= 0x072a00 /* 7.42.0 */
- curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1);
+ curl_easy_setopt_ck(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1);
#else
/* On old curl; no big deal, canonicalization here is almost the
[PATCH] libdw: Add sanity check to store_implicit_value
Don't just skip the block length, but check it is equal to the op->number that we are going to use as length. Signed-off-by: Mark Wielaard --- libdw/ChangeLog | 4 libdw/dwarf_getlocation.c | 5 - 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 631f2f2a..b254d9cd 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,7 @@ +2022-05-09 Mark Wielaard + + * dwarf_getlocation.c (store_implicit_value): Check block length. + 2022-04-16 Mark Wielaard * libdwP.h: Remove atomics.h include. diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c index 5db3cf97..d0d78163 100644 --- a/libdw/dwarf_getlocation.c +++ b/libdw/dwarf_getlocation.c @@ -147,7 +147,10 @@ store_implicit_value (Dwarf *dbg, void **cache, Dwarf_Op *op) sizeof (struct loc_block_s), 1); const unsigned char *data = (const unsigned char *) (uintptr_t) op->number2; /* Skip the block length. */ - __libdw_get_uleb128_unchecked (&data); + Dwarf_Word length; + get_uleb128_unchecked (length, data); + if (length != op->number) +return -1; block->addr = op; block->data = (unsigned char *) data; block->length = op->number; -- 2.18.4
[Bug debuginfod/29098] set default prefetch limits to >0
debuginfod/debuginfod.cxx:
- Added default value to fdcache_prefetch_mds and
fdcache_prefetch_fds when -R is specified. Defaults to one half of
fdcache's values for those respective variables. These values are only set
when -R is specified
Note: the ratio of prefetch:fdcache file descriptors and MBs
is set to 1:2 since the fdcache seems to be used less
than the standard fdcache.
From caa420f97225c5cc31c3fa0650cc7e25af3b4e91 Mon Sep 17 00:00:00 2001
From: Noah Sanci
Date: Mon, 9 May 2022 13:15:04 -0400
Subject: [PATCH] PR29098: debuginfod - set default prefetch limits to >0
debuginfod/debuginfod.cxx: - Added default value to fdcache_prefetch_mds and
fdcache_prefetch_fds when -R is specified. Defaults to one half of
fdcache's values for those respective variables. These values are only set
when -R is specified
Signed-off-by: Noah Sanci
---
debuginfod/debuginfod.cxx | 12
1 file changed, 12 insertions(+)
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 4aaf41c0..b2fb2afb 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -3826,6 +3826,18 @@ main (int argc, char *argv[])
error (EXIT_FAILURE, 0,
"unexpected argument: %s", argv[remaining]);
+ // Make the prefetch cache spaces smaller than the normal
+ // fd cache if rpm scanning is on. This is to not waste memory
+ // since the prefetch cache isn't used when -R isn't specified
+ // Set to 1/2 arbitrarily
+ if ( scan_archives[".rpm"] == "cat" )
+{
+ if ( fdcache_prefetch_fds == 0 )
+fdcache_prefetch_fds = fdcache_fds * .5;
+ if ( fdcache_prefetch_mbs == 0 )
+fdcache_prefetch_mbs = fdcache_mbs * .5;
+}
+
if (scan_archives.size()==0 && !scan_files && source_paths.size()>0)
obatched(clog) << "warning: without -F -R -U -Z, ignoring PATHs" << endl;
--
2.31.1
Re: [Bug debuginfod/29098] set default prefetch limits to >0
Hi -
> + // Make the prefetch cache spaces smaller than the normal
> + // fd cache if rpm scanning is on. This is to not waste memory
> + // since the prefetch cache isn't used when -R isn't specified
> + // Set to 1/2 arbitrarily
> + if ( scan_archives[".rpm"] == "cat" )
> +{
> + if ( fdcache_prefetch_fds == 0 )
> +fdcache_prefetch_fds = fdcache_fds * .5;
> + if ( fdcache_prefetch_mbs == 0 )
> +fdcache_prefetch_mbs = fdcache_mbs * .5;
> +}
Yeah, something like that. It turns out that space for the prefetch
cache is not used at all if RPM (and don't forget about other archive
types!) is not in effect. So it's harmless to set those defaults
unconditionally. How about a simpler:
if ( fdcache_prefetch_fds == 0 )
fdcache_prefetch_fds = fdcache_fds * .5;
if ( fdcache_prefetch_mbs == 0 )
fdcache_prefetch_mbs = fdcache_mbs * .5;
- FChE
[Bug debuginfod/28577] Make run-debuginfod-fd-prefetch-caches.sh test something
https://sourceware.org/bugzilla/show_bug.cgi?id=28577 Noah Sanci changed: What|Removed |Added Assignee|unassigned at sourceware dot org |nsanci at redhat dot com CC||nsanci at redhat dot com -- You are receiving this mail because: You are on the CC list for the bug.
[PATCH] strip: Add more NULL check
When gelf_getshdr, gelf_getrela, gelf_getrel or gelf_getsymshndx
return NULL it is an internal error which we want to report instead of
crashing.
Signed-off-by: Mark Wielaard
---
src/ChangeLog | 5 +
src/strip.c | 12 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index fd87ce2f..b978f9ef 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2022-05-09 Mark Wielaard
+
+ * strip.c (remove_debug_relocations): Check gelf_getshdr, gelf_getrela,
+ gelf_getrel and gelf_getsymshndx don't return NULL.
+
2022-04-24 Mark Wielaard
* elfclassify.c (main): Use classify_flag_no_stdin for no-std in
options.
diff --git a/src/strip.c b/src/strip.c
index 30a1f9da..452b1279 100644
--- a/src/strip.c
+++ b/src/strip.c
@@ -576,7 +576,8 @@ remove_debug_relocations (Ebl *ebl, Elf *elf, GElf_Ehdr
*ehdr,
might want to change the size. */
GElf_Shdr shdr_mem;
GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
- if (shdr->sh_type == SHT_REL || shdr->sh_type == SHT_RELA)
+ if (shdr != NULL
+ && (shdr->sh_type == SHT_REL || shdr->sh_type == SHT_RELA))
{
/* Make sure that this relocation section points to a
section to relocate with contents, that isn't
@@ -584,7 +585,8 @@ remove_debug_relocations (Ebl *ebl, Elf *elf, GElf_Ehdr
*ehdr,
Elf_Scn *tscn = elf_getscn (elf, shdr->sh_info);
GElf_Shdr tshdr_mem;
GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
- if (tshdr->sh_type == SHT_NOBITS
+ if (tshdr == NULL
+ || tshdr->sh_type == SHT_NOBITS
|| tshdr->sh_size == 0
|| (tshdr->sh_flags & SHF_ALLOC) != 0)
continue;
@@ -653,6 +655,8 @@ remove_debug_relocations (Ebl *ebl, Elf *elf, GElf_Ehdr
*ehdr,
if (is_rela)
{
GElf_Rela *r = gelf_getrela (reldata, relidx, &mem.rela);
+ if (r == NULL)
+ INTERNAL_ERROR (fname);
offset = r->r_offset;
addend = r->r_addend;
rtype = GELF_R_TYPE (r->r_info);
@@ -662,6 +666,8 @@ remove_debug_relocations (Ebl *ebl, Elf *elf, GElf_Ehdr
*ehdr,
else
{
GElf_Rel *r = gelf_getrel (reldata, relidx, &mem.rel);
+ if (r == NULL)
+ INTERNAL_ERROR (fname);
offset = r->r_offset;
addend = 0;
rtype = GELF_R_TYPE (r->r_info);
@@ -685,6 +691,8 @@ remove_debug_relocations (Ebl *ebl, Elf *elf, GElf_Ehdr
*ehdr,
GElf_Sym *sym = gelf_getsymshndx (symdata, xndxdata,
symndx, &sym_mem,
&xndx);
+ if (sym == NULL)
+ INTERNAL_ERROR (fname);
Elf32_Word sec = (sym->st_shndx == SHN_XINDEX
? xndx : sym->st_shndx);
--
2.30.2
[PATCH] debuginfod: Always request servname from getnameinfo for conninfo.
When getting the connection info getnameinfo is called getting the
hostname and servname except when the sockaddr is a pure ipv6
address. In that last case only hostname is requested. Since servname
is stack allocated and not initialized it might contain garbage which
is then put in the log. Just always request both hostname and servname
with NI_NUMERICHOST | NI_NUMERICSERV.
Signed-off-by: Mark Wielaard
---
debuginfod/ChangeLog | 4
debuginfod/debuginfod.cxx | 15 ++-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 983fd44a..257ac39f 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,7 @@
+2022-05-09 Mark Wielaard
+
+ * debuginfod.cxx (conninfo): Always provide servname to getnameinfo.
+
2022-05-09 Mark Wielaard
* debuginfod-client.c (debuginfod_query_server): Add
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 4aaf41c0..b664b74e 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1060,8 +1060,10 @@ conninfo (struct MHD_Connection * conn)
struct sockaddr *so = u ? u->client_addr : 0;
if (so && so->sa_family == AF_INET) {
-sts = getnameinfo (so, sizeof (struct sockaddr_in), hostname, sizeof
(hostname), servname,
- sizeof (servname), NI_NUMERICHOST | NI_NUMERICSERV);
+sts = getnameinfo (so, sizeof (struct sockaddr_in),
+ hostname, sizeof (hostname),
+ servname, sizeof (servname),
+ NI_NUMERICHOST | NI_NUMERICSERV);
} else if (so && so->sa_family == AF_INET6) {
struct sockaddr_in6* addr6 = (struct sockaddr_in6*) so;
if (IN6_IS_ADDR_V4MAPPED(&addr6->sin6_addr)) {
@@ -1071,11 +1073,14 @@ conninfo (struct MHD_Connection * conn)
addr4.sin_port = addr6->sin6_port;
memcpy (&addr4.sin_addr.s_addr, addr6->sin6_addr.s6_addr+12,
sizeof(addr4.sin_addr.s_addr));
sts = getnameinfo ((struct sockaddr*) &addr4, sizeof (addr4),
- hostname, sizeof (hostname), servname, sizeof
(servname),
+ hostname, sizeof (hostname),
+ servname, sizeof (servname),
NI_NUMERICHOST | NI_NUMERICSERV);
} else {
- sts = getnameinfo (so, sizeof (struct sockaddr_in6), hostname, sizeof
(hostname), NULL, 0,
- NI_NUMERICHOST);
+ sts = getnameinfo (so, sizeof (struct sockaddr_in6),
+ hostname, sizeof (hostname),
+ servname, sizeof (servname),
+ NI_NUMERICHOST | NI_NUMERICSERV);
}
}
--
2.30.2
Optimize debuginfod-client cache lookup/cleanup a little
Hi, debuginfod-client would try to create the cache config files twice, once through debuginfod_init_cache, which was always called before the debuginfod_clean_cache check. Which called debuginfod_config_cache which also tried to create the config files when they didn't exist yet. debuginfod_config_cache however had a small bug that meant it would not provide a valid struct stat if the config file didn't exist yet. The first patch fixes that: [PATCH 1/3] debuginfod: Make sure debuginfod_config_cache always returns valid stat Then the second patch removes debuginfod_init_cache which saves two stat calls (but introduces a new mkdir call). [PATCH 2/3] debuginfod: Remove debuginfod_init_cache Finally as soon as debuginfod_clean_cache commits to clean the cache dir we immediately update the mtime of the interval config file so other threads will not try to simultaniously also try to clean up the cache dir. Because that is just duplicate work. [PATCH 3/3] debuginfod: update mtime of interval_path as early as possible Cheers, Mark
[PATCH 1/3] debuginfod: Make sure debuginfod_config_cache always returns valid stat
If the condig file which value was requested from
debuginfod_config_cache didn't exist yet, stat would fail and no valid
struct stat would be returned even when the file was correctly
created. Fix this by always using O_CREAT to open the file, and reuse
that file descriptor to call fstat and for either writing the default
value or reading the config file value.
Signed-off-by: Mark Wielaard
---
debuginfod/ChangeLog | 6 ++
debuginfod/debuginfod-client.c | 21 ++---
2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 257ac39f..46a6e22b 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2022-05-09 Mark Wielaard
+
+ * debuginfod-client.c (debuginfod_config_cache): Always open with
+ O_CREATE first, then use fstat, only write the cache_config_default_s
+ value if st_size == 0, otherwise read value from file.
+
2022-05-09 Mark Wielaard
* debuginfod.cxx (conninfo): Always provide servname to getnameinfo.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 89208216..3b2728f1 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -235,14 +235,19 @@ debuginfod_config_cache(char *config_path,
long cache_config_default_s,
struct stat *st)
{
- int fd;
- /* if the config file doesn't exist, create one with DEFFILEMODE*/
- if(stat(config_path, st) == -1)
+ int fd = open(config_path, O_CREAT | O_RDWR, DEFFILEMODE);
+ if (fd < 0)
+return -errno;
+
+ if (fstat (fd, st) < 0)
{
- fd = open(config_path, O_CREAT | O_RDWR, DEFFILEMODE);
- if (fd < 0)
-return -errno;
+ int ret = -errno;
+ close (fd);
+ return ret;
+}
+ if (st->st_size == 0)
+{
if (dprintf(fd, "%ld", cache_config_default_s) < 0)
{
int ret = -errno;
@@ -251,10 +256,11 @@ debuginfod_config_cache(char *config_path,
}
close (fd);
+ return cache_config_default_s;
}
long cache_config;
- FILE *config_file = fopen(config_path, "r");
+ FILE *config_file = fdopen(fd, "r");
if (config_file)
{
if (fscanf(config_file, "%ld", &cache_config) != 1)
@@ -264,6 +270,7 @@ debuginfod_config_cache(char *config_path,
else
cache_config = cache_config_default_s;
+ close (fd);
return cache_config;
}
--
2.30.2
[PATCH 2/3] debuginfod: Remove debuginfod_init_cache
debuginfod_init_cache would create all config files if they didn't
exist yet. It always made two stat calls. Then debuginfod_clean_cache
would call debuginfod_config_cache which did the same checks and
created any missing config files. Just make sure the cache_path
directory exists and remove debuginfod_init_cache before calling
debuginfod_clean_cache.
Signed-off-by: Mark Wielaard
---
debuginfod/ChangeLog | 6
debuginfod/debuginfod-client.c | 61 +-
2 files changed, 15 insertions(+), 52 deletions(-)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 46a6e22b..c9aa4fcf 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2022-05-09 Mark Wielaard
+
+ * debuginfod-client.c (debuginfod_init_cache): Remove.
+ (debuginfod_query_server): Don't call debuginfod_init_cache, call
+ mkdir then debuginfod_clean_cache.
+
2022-05-09 Mark Wielaard
* debuginfod-client.c (debuginfod_config_cache): Always open with
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 3b2728f1..6bdf1908 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -274,55 +274,6 @@ debuginfod_config_cache(char *config_path,
return cache_config;
}
-/* Create the cache and interval file if they do not already exist.
- Return 0 if cache and config file are initialized, otherwise return
- the appropriate error code. */
-static int
-debuginfod_init_cache (char *cache_path, char *interval_path, char
*maxage_path)
-{
- struct stat st;
-
- /* If the cache and config file already exist then we are done. */
- if (stat(cache_path, &st) == 0 && stat(interval_path, &st) == 0)
-return 0;
-
- /* Create the cache and config files as necessary. */
- if (stat(cache_path, &st) != 0 && mkdir(cache_path, ACCESSPERMS) < 0)
-return -errno;
-
- int fd = -1;
-
- /* init cleaning interval config file. */
- fd = open(interval_path, O_CREAT | O_RDWR, DEFFILEMODE);
- if (fd < 0)
-return -errno;
-
- if (dprintf(fd, "%ld", cache_clean_default_interval_s) < 0)
-{
- int ret = -errno;
- close (fd);
- return ret;
-}
-
- close (fd);
-
- /* init max age config file. */
- if (stat(maxage_path, &st) != 0
- && (fd = open(maxage_path, O_CREAT | O_RDWR, DEFFILEMODE)) < 0)
-return -errno;
-
- if (dprintf(fd, "%ld", cache_default_max_unused_age_s) < 0)
-{
- int ret = -errno;
- close (fd);
- return ret;
-}
-
- close (fd);
- return 0;
-}
-
-
/* Delete any files that have been unmodied for a period
longer than $DEBUGINFOD_CACHE_CLEAN_INTERVAL_S. */
static int
@@ -795,9 +746,15 @@ debuginfod_query_server (debuginfod_client *c,
if (vfd >= 0)
dprintf (vfd, "checking cache dir %s\n", cache_path);
- rc = debuginfod_init_cache(cache_path, interval_path, maxage_path);
- if (rc != 0)
-goto out;
+ /* Make sure cache dir exists. debuginfo_clean_cache will then make
+ sure the interval, cache_miss and maxage files exist. */
+ if (mkdir (cache_path, ACCESSPERMS) != 0
+ && errno != EEXIST)
+{
+ rc = -errno;
+ goto out;
+}
+
rc = debuginfod_clean_cache(c, cache_path, interval_path, maxage_path);
if (rc != 0)
goto out;
--
2.30.2
[PATCH 3/3] debuginfod: update mtime of interval_path as early as possible
Call utime on interval_path file as soon as the thread is committed to cleanup the cache files. This will prevent other threads trying to also commit to cleaning the cache files. Having multiple threads try to clean the cache simultaniously doesn't improve cleanup speed because the threads will try to delete the files in the same order. Signed-off-by: Mark Wielaard --- debuginfod/ChangeLog | 5 + debuginfod/debuginfod-client.c | 7 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index c9aa4fcf..209a22a7 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,8 @@ +2022-05-09 Mark Wielaard + + * debuginfod-client.c (debuginfod_clean_cache): Move utime call to + before fts traversal. + 2022-05-09 Mark Wielaard * debuginfod-client.c (debuginfod_init_cache): Remove. diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index 6bdf1908..b7b65aff 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -297,6 +297,11 @@ debuginfod_clean_cache(debuginfod_client *c, /* Interval has not passed, skip cleaning. */ return 0; + /* Update timestamp representing when the cache was last cleaned. + Do it at the start to reduce the number of threads trying to do a + cleanup simultaniously. */ + utime (interval_path, NULL); + /* Read max unused age value from config file. */ rc = debuginfod_config_cache(max_unused_path, cache_default_max_unused_age_s, &st); @@ -351,8 +356,6 @@ debuginfod_clean_cache(debuginfod_client *c, fts_close (fts); regfree (&re); - /* Update timestamp representing when the cache was last cleaned. */ - utime (interval_path, NULL); return 0; } -- 2.30.2
Re: [Bug debuginfod/29098] set default prefetch limits to >0
Hi - Thanks, committing this, with corrected comments and changelog entries and a bit of man page cleanup. diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 619ebd8c9202..026908c85000 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,7 @@ +2022-05-09 Noah Sanci + + * debuginfod.cxx (main): Set nonzero defaults for fdcache. + 2022-05-04 Frank Ch. Eigler Mark Wielaard diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 4aaf41c0886e..fde4e194b526 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -3826,6 +3826,13 @@ main (int argc, char *argv[]) error (EXIT_FAILURE, 0, "unexpected argument: %s", argv[remaining]); + // Make the prefetch cache spaces a fraction of the main fdcache if + // unspecified. + if (fdcache_prefetch_fds == 0) +fdcache_prefetch_fds = fdcache_fds / 2; + if (fdcache_prefetch_mbs == 0) +fdcache_prefetch_mbs = fdcache_mbs / 2; + if (scan_archives.size()==0 && !scan_files && source_paths.size()>0) obatched(clog) << "warning: without -F -R -U -Z, ignoring PATHs" << endl; diff --git a/doc/ChangeLog b/doc/ChangeLog index 303e3dc05dc5..cb754d04ba3f 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,7 @@ +2022-05-09 Frank Ch. Eigler + + * debuginfod.8: Tweak prefetch descriptions. + 2022-01-31 Frank Ch. Eigler * debuginfod-client-config.7: Elaborate DEBUGINFOD_URLS. diff --git a/doc/debuginfod.8 b/doc/debuginfod.8 index ee8e4078e5b5..95b827e9cc35 100644 --- a/doc/debuginfod.8 +++ b/doc/debuginfod.8 @@ -232,34 +232,36 @@ loops in the symbolic directory tree might lead to \fIinfinite traversal\fP. .TP -.B "\-\-fdcache\-fds=NUM" "\-\-fdcache\-mbs=MB" "\-\-fdcache\-prefetch=NUM2" +.B "\-\-fdcache\-fds=NUM" "\-\-fdcache\-mbs=MB" Configure limits on a cache that keeps recently extracted files from archives. Up to NUM requested files and up to a total of MB megabytes will be kept extracted, in order to avoid having to decompress their -archives over and over again. In addition, up to NUM2 other files -from an archive may be prefetched into the cache before they are even -requested. The default NUM, NUM2, and MB values depend on the -concurrency of the system, and on the available disk space on the +archives over and over again. The default NUM and MB values depend on +the concurrency of the system, and on the available disk space on the $TMPDIR or \fB/tmp\fP filesystem. This is because that is where the -most recently used extracted files are kept. Grooming cleans this +most recently used extracted files are kept. Grooming cleans out 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 fdcache. If unspecified, values of -\fB\-\-prefetch\-fds\fP and \fB\-\-prefetch\-mbs\fP depend -on concurrency of the system and on the available disk space on -the $TMPDIR. Allocating more to the prefetch cache will improve -performance in environments where different parts of several large -archives are being accessed. +.B "\-\-fdcache\-prefetch=NUM2" + +In addition to the main fdcache, up to NUM2 other files from an +archive may be prefetched into another cache before they are even +requested. Configure how many file descriptors (fds) and megabytes +(mbs) are allocated to the prefetch fdcache. If unspecified, these +values depend on concurrency of the system and on the available disk +space on the $TMPDIR. Allocating more to the prefetch cache will +improve performance in environments where different parts of several +large archives are being accessed. This cache is also cleaned out +during grooming. .TP .B "\-\-fdcache\-mintmp=NUM" -Configure a disk space threshold for emergency flushing of the cache. -The filesystem holding the cache is checked periodically. If the -available space falls below the given percentage, the cache is -flushed, and the fdcache will stay disabled until the next groom +Configure a disk space threshold for emergency flushing of the caches. +The filesystem holding the caches is checked periodically. If the +available space falls below the given percentage, the caches are +flushed, and the fdcaches will stay disabled until the next groom cycle. This mechanism, along a few associated /metrics on the webapi, are intended to give an operator notice about storage scarcity - which can translate to RAM scarcity if the disk happens to be on a RAM
☺ Buildbot (GNU Toolchain): elfutils - build successful (master)
A restored build has been detected on builder elfutils-debian-armhf while building elfutils. Full details are available at: https://builder.sourceware.org/buildbot/#builders/6/builds/19 Build state: build successful Revision: c7982c9e3bee93422dd140568587e2796e0c96ca Worker: debian-armhf Build Reason: (unknown) Blamelist: Frank Ch. Eigler , Noah Sanci Steps: - 0: worker_preparation ( success ) - 1: set package name ( success ) - 2: git checkout ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/2/logs/stdio - 3: autoreconf ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/3/logs/stdio - 4: configure ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/4/logs/stdio - 5: get version ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/5/logs/stdio - property changes: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/5/logs/property_changes - 6: make ( warnings ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/6/logs/stdio - warnings (3): https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/6/logs/warnings__3_ - 7: make check ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/7/logs/stdio - test-suite.log: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/7/logs/test-suite_log - 8: prep ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/8/logs/stdio - 9: fetch ['tests/*.trs', 'tests/*.log'] ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/9/logs/stdio - 10: fetch ['elfutils-*/_build/sub/tests/*.trs', 'elfut ( skipped ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/10/logs/stdio - 11: fetch ['config.log'] ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/11/logs/stdio - 12: fetch ['elfutils-*/_build/sub/config.log'] ( skipped ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/12/logs/stdio - 13: pass .bunsen.source.gitname ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/13/logs/stdio - 14: pass .bunsen.source.gitrepo ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/14/logs/stdio - 15: upload to bunsen ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/15/logs/stdio - 16: clean up ( success ) Logs: - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/19/steps/16/logs/stdio
