[PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[Andrii Nakryiko]

Instead of parsing text-based /proc/<pid>/maps file, try to use
PROCMAP_QUERY ioctl() to simplify and speed up data fetching.
This logic is used to do uprobe file offset calculation, so any bugs in
this logic would manifest as failing uprobe BPF selftests.

This also serves as a simple demonstration of one of the intended uses.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/test_progs.c    |   3 +
 tools/testing/selftests/bpf/test_progs.h    |   2 +
 tools/testing/selftests/bpf/trace_helpers.c | 104 +++++++++++++++++---
 3 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 60fafa2f1ed7..bf262b42a488 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -33,6 +33,8 @@ __weak void backtrace_symbols_fd(void *const *buffer, int size, int fd)
 	dprintf(fd, "<backtrace not supported>\n");
 }
 
+int env_verbosity = 0;
+
 static bool verbose(void)
 {
 	return env.verbosity > VERBOSE_NONE;
@@ -862,6 +864,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 				return -EINVAL;
 			}
 		}
+		env_verbosity = env->verbosity;
 
 		if (verbose()) {
 			if (setenv("SELFTESTS_VERBOSE", "1", 1) == -1) {
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index cb9d6d46826b..43e3136166b3 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -95,6 +95,8 @@ struct test_state {
 	FILE *stdout_saved;
 };
 
+extern int env_verbosity;
+
 struct test_env {
 	struct test_selector test_selector;
 	struct test_selector subtest_selector;
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index 465d196c7165..1bfd881c0e07 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -10,6 +10,8 @@
 #include <pthread.h>
 #include <unistd.h>
 #include <linux/perf_event.h>
+#include <linux/fs.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include "trace_helpers.h"
 #include <linux/limits.h>
@@ -244,29 +246,91 @@ int kallsyms_find(const char *sym, unsigned long long *addr)
 	return err;
 }
 
+#ifdef PROCMAP_QUERY
+int env_verbosity __weak = 0;
+
+int procmap_query(int fd, const void *addr, __u32 query_flags, size_t *start, size_t *offset, int *flags)
+{
+	char path_buf[PATH_MAX], build_id_buf[20];
+	struct procmap_query q;
+	int err;
+
+	memset(&q, 0, sizeof(q));
+	q.size = sizeof(q);
+	q.query_flags = query_flags;
+	q.query_addr = (__u64)addr;
+	q.vma_name_addr = (__u64)path_buf;
+	q.vma_name_size = sizeof(path_buf);
+	q.build_id_addr = (__u64)build_id_buf;
+	q.build_id_size = sizeof(build_id_buf);
+
+	err = ioctl(fd, PROCMAP_QUERY, &q);
+	if (err < 0) {
+		err = -errno;
+		if (err == -ENOTTY)
+			return -EOPNOTSUPP; /* ioctl() not implemented yet */
+		if (err == -ENOENT)
+			return -ESRCH; /* vma not found */
+		return err;
+	}
+
+	if (env_verbosity >= 1) {
+		printf("VMA FOUND (addr %08lx): %08lx-%08lx %c%c%c%c %08lx %02x:%02x %ld %s (build ID: %s, %d bytes)\n",
+		       (long)addr, (long)q.vma_start, (long)q.vma_end,
+		       (q.vma_flags & PROCMAP_QUERY_VMA_READABLE) ? 'r' : '-',
+		       (q.vma_flags & PROCMAP_QUERY_VMA_WRITABLE) ? 'w' : '-',
+		       (q.vma_flags & PROCMAP_QUERY_VMA_EXECUTABLE) ? 'x' : '-',
+		       (q.vma_flags & PROCMAP_QUERY_VMA_SHARED) ? 's' : 'p',
+		       (long)q.vma_offset, q.dev_major, q.dev_minor, (long)q.inode,
+		       q.vma_name_size ? path_buf : "",
+		       q.build_id_size ? "YES" : "NO",
+		       q.build_id_size);
+	}
+
+	*start = q.vma_start;
+	*offset = q.vma_offset;
+	*flags = q.vma_flags;
+	return 0;
+}
+#else
+int procmap_query(int fd, const void *addr, size_t *start, size_t *offset, int *flags)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 ssize_t get_uprobe_offset(const void *addr)
 {
-	size_t start, end, base;
-	char buf[256];
-	bool found = false;
+	size_t start, base, end;
 	FILE *f;
+	char buf[256];
+	int err, flags;
 
 	f = fopen("/proc/self/maps", "r");
 	if (!f)
 		return -errno;
 
-	while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
-		if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
-			found = true;
-			break;
+	/* requested executable VMA only */
+	err = procmap_query(fileno(f), addr, PROCMAP_QUERY_VMA_EXECUTABLE, &start, &base, &flags);
+	if (err == -EOPNOTSUPP) {
+		bool found = false;
+
+		while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
+			if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
+				found = true;
+				break;
+			}
+		}
+		if (!found) {
+			fclose(f);
+			return -ESRCH;
 		}
+	} else if (err) {
+		fclose(f);
+		return err;
 	}
-
 	fclose(f);
 
-	if (!found)
-		return -ESRCH;
-
 #if defined(__powerpc64__) && defined(_CALL_ELF) && _CALL_ELF == 2
 
 #define OP_RT_RA_MASK   0xffff0000UL
@@ -307,15 +371,25 @@ ssize_t get_rel_offset(uintptr_t addr)
 	size_t start, end, offset;
 	char buf[256];
 	FILE *f;
+	int err, flags;
 
 	f = fopen("/proc/self/maps", "r");
 	if (!f)
 		return -errno;
 
-	while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
-		if (addr >= start && addr < end) {
-			fclose(f);
-			return (size_t)addr - start + offset;
+	err = procmap_query(fileno(f), (const void *)addr, 0, &start, &offset, &flags);
+	if (err == 0) {
+		fclose(f);
+		return (size_t)addr - start + offset;
+	} else if (err != -EOPNOTSUPP) {
+		fclose(f);
+		return err;
+	} else if (err) {
+		while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
+			if (addr >= start && addr < end) {
+				fclose(f);
+				return (size_t)addr - start + offset;
+			}
 		}
 	}
 
-- 
2.43.5

Re: [PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[Jiri Olsa]

On Tue, Aug 06, 2024 at 04:03:19PM -0700, Andrii Nakryiko wrote:

SNIP

>  ssize_t get_uprobe_offset(const void *addr)
>  {
> -	size_t start, end, base;
> -	char buf[256];
> -	bool found = false;
> +	size_t start, base, end;
>  	FILE *f;
> +	char buf[256];
> +	int err, flags;
>  
>  	f = fopen("/proc/self/maps", "r");
>  	if (!f)
>  		return -errno;
>  
> -	while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> -		if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> -			found = true;
> -			break;
> +	/* requested executable VMA only */
> +	err = procmap_query(fileno(f), addr, PROCMAP_QUERY_VMA_EXECUTABLE, &start, &base, &flags);
> +	if (err == -EOPNOTSUPP) {
> +		bool found = false;
> +
> +		while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> +			if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> +				found = true;
> +				break;
> +			}
> +		}
> +		if (!found) {
> +			fclose(f);
> +			return -ESRCH;
>  		}
> +	} else if (err) {
> +		fclose(f);
> +		return err;

I feel like I commented on this before, so feel free to ignore me,
but this seems similar to the code below, could be in one function

anyway it's good for follow up

there was another selftest in the original patchset adding benchmark
for the procfs query interface, is it coming in as well?

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

>  	}
> -
>  	fclose(f);
>  
> -	if (!found)
> -		return -ESRCH;
> -
>  #if defined(__powerpc64__) && defined(_CALL_ELF) && _CALL_ELF == 2
>  
>  #define OP_RT_RA_MASK   0xffff0000UL
> @@ -307,15 +371,25 @@ ssize_t get_rel_offset(uintptr_t addr)
>  	size_t start, end, offset;
>  	char buf[256];
>  	FILE *f;
> +	int err, flags;
>  
>  	f = fopen("/proc/self/maps", "r");
>  	if (!f)
>  		return -errno;
>  
> -	while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
> -		if (addr >= start && addr < end) {
> -			fclose(f);
> -			return (size_t)addr - start + offset;
> +	err = procmap_query(fileno(f), (const void *)addr, 0, &start, &offset, &flags);
> +	if (err == 0) {
> +		fclose(f);
> +		return (size_t)addr - start + offset;
> +	} else if (err != -EOPNOTSUPP) {
> +		fclose(f);
> +		return err;
> +	} else if (err) {
> +		while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
> +			if (addr >= start && addr < end) {
> +				fclose(f);
> +				return (size_t)addr - start + offset;
> +			}
>  		}
>  	}
>  
> -- 
> 2.43.5
> 
> 

Re: [PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[Andrii Nakryiko]

On Wed, Aug 7, 2024 at 2:07 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Aug 06, 2024 at 04:03:19PM -0700, Andrii Nakryiko wrote:
>
> SNIP
>
> >  ssize_t get_uprobe_offset(const void *addr)
> >  {
> > -     size_t start, end, base;
> > -     char buf[256];
> > -     bool found = false;
> > +     size_t start, base, end;
> >       FILE *f;
> > +     char buf[256];
> > +     int err, flags;
> >
> >       f = fopen("/proc/self/maps", "r");
> >       if (!f)
> >               return -errno;
> >
> > -     while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > -             if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > -                     found = true;
> > -                     break;
> > +     /* requested executable VMA only */
> > +     err = procmap_query(fileno(f), addr, PROCMAP_QUERY_VMA_EXECUTABLE, &start, &base, &flags);
> > +     if (err == -EOPNOTSUPP) {
> > +             bool found = false;
> > +
> > +             while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > +                     if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > +                             found = true;
> > +                             break;
> > +                     }
> > +             }
> > +             if (!found) {
> > +                     fclose(f);
> > +                     return -ESRCH;
> >               }
> > +     } else if (err) {
> > +             fclose(f);
> > +             return err;
>
> I feel like I commented on this before, so feel free to ignore me,
> but this seems similar to the code below, could be in one function

Do you mean get_rel_offset()? That one is for data symbols (USDT
semaphores), so it a) doesn't do arch-specific adjustments and b)
doesn't filter by executable flag. So while the logic of parsing and
finding VMA is similar, conditions and adjustments are different. It
feels not worth combining them, tbh.

>
> anyway it's good for follow up
>
> there was another selftest in the original patchset adding benchmark
> for the procfs query interface, is it coming in as well?

I didn't plan to send it, given it's not really a test. But I can put
it on Github somewhere, probably, if it's useful.

>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> jirka
>
> >       }
> > -
> >       fclose(f);
> >
> > -     if (!found)
> > -             return -ESRCH;
> > -
> >  #if defined(__powerpc64__) && defined(_CALL_ELF) && _CALL_ELF == 2
> >
> >  #define OP_RT_RA_MASK   0xffff0000UL
> > @@ -307,15 +371,25 @@ ssize_t get_rel_offset(uintptr_t addr)
> >       size_t start, end, offset;
> >       char buf[256];
> >       FILE *f;
> > +     int err, flags;
> >
> >       f = fopen("/proc/self/maps", "r");
> >       if (!f)
> >               return -errno;
> >
> > -     while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
> > -             if (addr >= start && addr < end) {
> > -                     fclose(f);
> > -                     return (size_t)addr - start + offset;
> > +     err = procmap_query(fileno(f), (const void *)addr, 0, &start, &offset, &flags);
> > +     if (err == 0) {
> > +             fclose(f);
> > +             return (size_t)addr - start + offset;
> > +     } else if (err != -EOPNOTSUPP) {
> > +             fclose(f);
> > +             return err;
> > +     } else if (err) {
> > +             while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &offset) == 4) {
> > +                     if (addr >= start && addr < end) {
> > +                             fclose(f);
> > +                             return (size_t)addr - start + offset;
> > +                     }
> >               }
> >       }
> >
> > --
> > 2.43.5
> >
> >

Re: [PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[Alexei Starovoitov]

On Wed, Aug 7, 2024 at 8:17 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Aug 7, 2024 at 2:07 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Tue, Aug 06, 2024 at 04:03:19PM -0700, Andrii Nakryiko wrote:
> >
> > SNIP
> >
> > >  ssize_t get_uprobe_offset(const void *addr)
> > >  {
> > > -     size_t start, end, base;
> > > -     char buf[256];
> > > -     bool found = false;
> > > +     size_t start, base, end;
> > >       FILE *f;
> > > +     char buf[256];
> > > +     int err, flags;
> > >
> > >       f = fopen("/proc/self/maps", "r");
> > >       if (!f)
> > >               return -errno;
> > >
> > > -     while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > > -             if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > > -                     found = true;
> > > -                     break;
> > > +     /* requested executable VMA only */
> > > +     err = procmap_query(fileno(f), addr, PROCMAP_QUERY_VMA_EXECUTABLE, &start, &base, &flags);
> > > +     if (err == -EOPNOTSUPP) {
> > > +             bool found = false;
> > > +
> > > +             while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > > +                     if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > > +                             found = true;
> > > +                             break;
> > > +                     }
> > > +             }
> > > +             if (!found) {
> > > +                     fclose(f);
> > > +                     return -ESRCH;
> > >               }
> > > +     } else if (err) {
> > > +             fclose(f);
> > > +             return err;
> >
> > I feel like I commented on this before, so feel free to ignore me,
> > but this seems similar to the code below, could be in one function
>
> Do you mean get_rel_offset()? That one is for data symbols (USDT
> semaphores), so it a) doesn't do arch-specific adjustments and b)
> doesn't filter by executable flag. So while the logic of parsing and
> finding VMA is similar, conditions and adjustments are different. It
> feels not worth combining them, tbh.
>
> >
> > anyway it's good for follow up
> >
> > there was another selftest in the original patchset adding benchmark
> > for the procfs query interface, is it coming in as well?
>
> I didn't plan to send it, given it's not really a test. But I can put
> it on Github somewhere, probably, if it's useful.

With and without this selftest applied I see:
./test_progs -t uprobe
#416     uprobe:OK
#417     uprobe_autoattach:OK
[   47.448908] ref_ctr_offset mismatch. inode: 0x16b5f921 offset:
0x2d4297 ref_ctr_offset(old): 0x45e8b56 ref_ctr_offset(new): 0x45e8b54
#418/1   uprobe_multi_test/skel_api:OK

Is this a known issue?

Applied anyway.

Re: [PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[Andrii Nakryiko]

On Fri, Aug 23, 2024 at 7:48 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Aug 7, 2024 at 8:17 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Aug 7, 2024 at 2:07 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Tue, Aug 06, 2024 at 04:03:19PM -0700, Andrii Nakryiko wrote:
> > >
> > > SNIP
> > >
> > > >  ssize_t get_uprobe_offset(const void *addr)
> > > >  {
> > > > -     size_t start, end, base;
> > > > -     char buf[256];
> > > > -     bool found = false;
> > > > +     size_t start, base, end;
> > > >       FILE *f;
> > > > +     char buf[256];
> > > > +     int err, flags;
> > > >
> > > >       f = fopen("/proc/self/maps", "r");
> > > >       if (!f)
> > > >               return -errno;
> > > >
> > > > -     while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > > > -             if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > > > -                     found = true;
> > > > -                     break;
> > > > +     /* requested executable VMA only */
> > > > +     err = procmap_query(fileno(f), addr, PROCMAP_QUERY_VMA_EXECUTABLE, &start, &base, &flags);
> > > > +     if (err == -EOPNOTSUPP) {
> > > > +             bool found = false;
> > > > +
> > > > +             while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > > > +                     if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > > > +                             found = true;
> > > > +                             break;
> > > > +                     }
> > > > +             }
> > > > +             if (!found) {
> > > > +                     fclose(f);
> > > > +                     return -ESRCH;
> > > >               }
> > > > +     } else if (err) {
> > > > +             fclose(f);
> > > > +             return err;
> > >
> > > I feel like I commented on this before, so feel free to ignore me,
> > > but this seems similar to the code below, could be in one function
> >
> > Do you mean get_rel_offset()? That one is for data symbols (USDT
> > semaphores), so it a) doesn't do arch-specific adjustments and b)
> > doesn't filter by executable flag. So while the logic of parsing and
> > finding VMA is similar, conditions and adjustments are different. It
> > feels not worth combining them, tbh.
> >
> > >
> > > anyway it's good for follow up
> > >
> > > there was another selftest in the original patchset adding benchmark
> > > for the procfs query interface, is it coming in as well?
> >
> > I didn't plan to send it, given it's not really a test. But I can put
> > it on Github somewhere, probably, if it's useful.
>
> With and without this selftest applied I see:
> ./test_progs -t uprobe
> #416     uprobe:OK
> #417     uprobe_autoattach:OK
> [   47.448908] ref_ctr_offset mismatch. inode: 0x16b5f921 offset:
> 0x2d4297 ref_ctr_offset(old): 0x45e8b56 ref_ctr_offset(new): 0x45e8b54
> #418/1   uprobe_multi_test/skel_api:OK
>
> Is this a known issue?

Yeah, that's not due to my changes. It's an old warning in uprobe
internals, but I think we should remove it, because it can trivially
be triggered by a user. Which is what Jiri is doing intentionally in
one of selftests to test uprobe failure handling.

Jiri, maybe let's get rid of this warning?

>
> Applied anyway.

Thanks! I just found another auto-archived patch of mine, the one
adding multi-uprobe benchmarks (see patchworks). Please take a look
and maybe apply, when you get a chance.

Re: [PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[Jiri Olsa]

On Fri, Aug 23, 2024 at 09:53:03AM -0700, Andrii Nakryiko wrote:
> On Fri, Aug 23, 2024 at 7:48 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Aug 7, 2024 at 8:17 AM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Wed, Aug 7, 2024 at 2:07 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > > >
> > > > On Tue, Aug 06, 2024 at 04:03:19PM -0700, Andrii Nakryiko wrote:
> > > >
> > > > SNIP
> > > >
> > > > >  ssize_t get_uprobe_offset(const void *addr)
> > > > >  {
> > > > > -     size_t start, end, base;
> > > > > -     char buf[256];
> > > > > -     bool found = false;
> > > > > +     size_t start, base, end;
> > > > >       FILE *f;
> > > > > +     char buf[256];
> > > > > +     int err, flags;
> > > > >
> > > > >       f = fopen("/proc/self/maps", "r");
> > > > >       if (!f)
> > > > >               return -errno;
> > > > >
> > > > > -     while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > > > > -             if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > > > > -                     found = true;
> > > > > -                     break;
> > > > > +     /* requested executable VMA only */
> > > > > +     err = procmap_query(fileno(f), addr, PROCMAP_QUERY_VMA_EXECUTABLE, &start, &base, &flags);
> > > > > +     if (err == -EOPNOTSUPP) {
> > > > > +             bool found = false;
> > > > > +
> > > > > +             while (fscanf(f, "%zx-%zx %s %zx %*[^\n]\n", &start, &end, buf, &base) == 4) {
> > > > > +                     if (buf[2] == 'x' && (uintptr_t)addr >= start && (uintptr_t)addr < end) {
> > > > > +                             found = true;
> > > > > +                             break;
> > > > > +                     }
> > > > > +             }
> > > > > +             if (!found) {
> > > > > +                     fclose(f);
> > > > > +                     return -ESRCH;
> > > > >               }
> > > > > +     } else if (err) {
> > > > > +             fclose(f);
> > > > > +             return err;
> > > >
> > > > I feel like I commented on this before, so feel free to ignore me,
> > > > but this seems similar to the code below, could be in one function
> > >
> > > Do you mean get_rel_offset()? That one is for data symbols (USDT
> > > semaphores), so it a) doesn't do arch-specific adjustments and b)
> > > doesn't filter by executable flag. So while the logic of parsing and
> > > finding VMA is similar, conditions and adjustments are different. It
> > > feels not worth combining them, tbh.
> > >
> > > >
> > > > anyway it's good for follow up
> > > >
> > > > there was another selftest in the original patchset adding benchmark
> > > > for the procfs query interface, is it coming in as well?
> > >
> > > I didn't plan to send it, given it's not really a test. But I can put
> > > it on Github somewhere, probably, if it's useful.
> >
> > With and without this selftest applied I see:
> > ./test_progs -t uprobe
> > #416     uprobe:OK
> > #417     uprobe_autoattach:OK
> > [   47.448908] ref_ctr_offset mismatch. inode: 0x16b5f921 offset:
> > 0x2d4297 ref_ctr_offset(old): 0x45e8b56 ref_ctr_offset(new): 0x45e8b54
> > #418/1   uprobe_multi_test/skel_api:OK
> >
> > Is this a known issue?
> 
> Yeah, that's not due to my changes. It's an old warning in uprobe
> internals, but I think we should remove it, because it can trivially
> be triggered by a user. Which is what Jiri is doing intentionally in
> one of selftests to test uprobe failure handling.
> 
> Jiri, maybe let's get rid of this warning?

fine with me.. or change to pr_debug if that makes sense for anyone,
Oleg, any idea/preference?

jirka

> 
> >
> > Applied anyway.
> 
> Thanks! I just found another auto-archived patch of mine, the one
> adding multi-uprobe benchmarks (see patchworks). Please take a look
> and maybe apply, when you get a chance.

Re: [PATCH bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Tue,  6 Aug 2024 16:03:19 -0700 you wrote:
> Instead of parsing text-based /proc/<pid>/maps file, try to use
> PROCMAP_QUERY ioctl() to simplify and speed up data fetching.
> This logic is used to do uprobe file offset calculation, so any bugs in
> this logic would manifest as failing uprobe BPF selftests.
> 
> This also serves as a simple demonstration of one of the intended uses.
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: make use of PROCMAP_QUERY ioctl if available
    https://git.kernel.org/bpf/bpf-next/c/4e9e07603ecd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html