As bpf is not using memlock rlimit for memory accounting anymore,
let's remove the related code from libbpf.
Bpf operations can't fail because of exceeding the limit anymore.
Signed-off-by: Roman Gushchin <[email protected]>
---
tools/lib/bpf/libbpf.c | 31 +------------------------------
tools/lib/bpf/libbpf.h | 5 -----
2 files changed, 1 insertion(+), 35 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e51479d60285..841060f5cee3 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -112,32 +112,6 @@ void libbpf_print(enum libbpf_print_level level, const char *format, ...)
va_end(args);
}
-static void pr_perm_msg(int err)
-{
- struct rlimit limit;
- char buf[100];
-
- if (err != -EPERM || geteuid() != 0)
- return;
-
- err = getrlimit(RLIMIT_MEMLOCK, &limit);
- if (err)
- return;
-
- if (limit.rlim_cur == RLIM_INFINITY)
- return;
-
- if (limit.rlim_cur < 1024)
- snprintf(buf, sizeof(buf), "%zu bytes", (size_t)limit.rlim_cur);
- else if (limit.rlim_cur < 1024*1024)
- snprintf(buf, sizeof(buf), "%.1f KiB", (double)limit.rlim_cur / 1024);
- else
- snprintf(buf, sizeof(buf), "%.1f MiB", (double)limit.rlim_cur / (1024*1024));
-
- pr_warn("permission error while running as root; try raising 'ulimit -l'? current value: %s\n",
- buf);
-}
-
#define STRERR_BUFSIZE 128
/* Copied from tools/perf/util/util.h */
@@ -3420,8 +3394,7 @@ bpf_object__probe_loading(struct bpf_object *obj)
cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
pr_warn("Error in %s():%s(%d). Couldn't load trivial BPF "
"program. Make sure your kernel supports BPF "
- "(CONFIG_BPF_SYSCALL=y) and/or that RLIMIT_MEMLOCK is "
- "set to big enough value.\n", __func__, cp, ret);
+ "(CONFIG_BPF_SYSCALL=y)", __func__, cp, ret);
return -ret;
}
close(ret);
@@ -3918,7 +3891,6 @@ bpf_object__create_maps(struct bpf_object *obj)
err_out:
cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
pr_warn("map '%s': failed to create: %s(%d)\n", map->name, cp, err);
- pr_perm_msg(err);
for (j = 0; j < i; j++)
zclose(obj->maps[j].fd);
return err;
@@ -5419,7 +5391,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
ret = -errno;
cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warn("load bpf program failed: %s\n", cp);
- pr_perm_msg(ret);
if (log_buf && log_buf[0] != '\0') {
ret = -LIBBPF_ERRNO__VERIFY;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index c6813791fa7e..8d2f1194cb02 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -610,11 +610,6 @@ bpf_prog_linfo__lfind(const struct bpf_prog_linfo *prog_linfo,
/*
* Probe for supported system features
- *
- * Note that running many of these probes in a short amount of time can cause
- * the kernel to reach the maximal size of lockable memory allowed for the
- * user, causing subsequent probes to fail. In this case, the caller may want
- * to adjust that limit with setrlimit().
*/
LIBBPF_API bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
__u32 ifindex);
--
2.26.2
On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
>
> As bpf is not using memlock rlimit for memory accounting anymore,
> let's remove the related code from libbpf.
>
> Bpf operations can't fail because of exceeding the limit anymore.
>
They can't in the newest kernel, but libbpf will keep working and
supporting old kernels for a very long time now. So please don't
remove any of this.
But it would be nice to add a detection of whether kernel needs a
RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
detect this from user-space?
> Signed-off-by: Roman Gushchin <[email protected]>
> ---
> tools/lib/bpf/libbpf.c | 31 +------------------------------
> tools/lib/bpf/libbpf.h | 5 -----
> 2 files changed, 1 insertion(+), 35 deletions(-)
>
[...]
On Mon, Jul 27, 2020 at 3:07 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
> >
> > As bpf is not using memlock rlimit for memory accounting anymore,
> > let's remove the related code from libbpf.
> >
> > Bpf operations can't fail because of exceeding the limit anymore.
> >
>
> They can't in the newest kernel, but libbpf will keep working and
> supporting old kernels for a very long time now. So please don't
> remove any of this.
>
> But it would be nice to add a detection of whether kernel needs a
> RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
> detect this from user-space?
>
Agreed. We will need compatibility or similar detection for perf as well.
Thanks,
Song
On Mon, Jul 27, 2020 at 03:05:11PM -0700, Andrii Nakryiko wrote:
> On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
> >
> > As bpf is not using memlock rlimit for memory accounting anymore,
> > let's remove the related code from libbpf.
> >
> > Bpf operations can't fail because of exceeding the limit anymore.
> >
>
> They can't in the newest kernel, but libbpf will keep working and
> supporting old kernels for a very long time now. So please don't
> remove any of this.
Yeah, good point, agree.
So we just can drop this patch from the series, no other changes
are needed.
>
> But it would be nice to add a detection of whether kernel needs a
> RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
> detect this from user-space?
Hm, the best idea I can think of is to wait for -EPERM before bumping.
We can in theory look for the presence of memory.stat::percpu in cgroupfs,
but it's way to cryptic.
Thanks!
On Mon, Jul 27, 2020 at 4:15 PM Roman Gushchin <[email protected]> wrote:
>
> On Mon, Jul 27, 2020 at 03:05:11PM -0700, Andrii Nakryiko wrote:
> > On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
> > >
> > > As bpf is not using memlock rlimit for memory accounting anymore,
> > > let's remove the related code from libbpf.
> > >
> > > Bpf operations can't fail because of exceeding the limit anymore.
> > >
> >
> > They can't in the newest kernel, but libbpf will keep working and
> > supporting old kernels for a very long time now. So please don't
> > remove any of this.
>
> Yeah, good point, agree.
> So we just can drop this patch from the series, no other changes
> are needed.
>
> >
> > But it would be nice to add a detection of whether kernel needs a
> > RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
> > detect this from user-space?
>
> Hm, the best idea I can think of is to wait for -EPERM before bumping.
> We can in theory look for the presence of memory.stat::percpu in cgroupfs,
> but it's way to cryptic.
>
As I just mentioned on another thread, checking fdinfo's "memlock: 0"
should be reliable enough, no?
> Thanks!
On Mon, Jul 27, 2020 at 10:59:33PM -0700, Andrii Nakryiko wrote:
> On Mon, Jul 27, 2020 at 4:15 PM Roman Gushchin <[email protected]> wrote:
> >
> > On Mon, Jul 27, 2020 at 03:05:11PM -0700, Andrii Nakryiko wrote:
> > > On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
> > > >
> > > > As bpf is not using memlock rlimit for memory accounting anymore,
> > > > let's remove the related code from libbpf.
> > > >
> > > > Bpf operations can't fail because of exceeding the limit anymore.
> > > >
> > >
> > > They can't in the newest kernel, but libbpf will keep working and
> > > supporting old kernels for a very long time now. So please don't
> > > remove any of this.
> >
> > Yeah, good point, agree.
> > So we just can drop this patch from the series, no other changes
> > are needed.
> >
> > >
> > > But it would be nice to add a detection of whether kernel needs a
> > > RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
> > > detect this from user-space?
Btw, do you mean we should add a new function to the libbpf API?
Or just extend pr_perm_msg() to skip guessing on new kernels?
The problem with the latter one is that it's called on a failed attempt
to create a map, so unlikely we'll be able to create a new one just to test
for the "memlock" value. But it also raises a question what should we do
if the creation of this temporarily map fails? Assume the old kernel and
bump the limit?
Idk, maybe it's better to just leave the userspace code as it is for some time.
Thanks!
On Wed, Jul 29, 2020 at 6:38 PM Roman Gushchin <[email protected]> wrote:
>
> On Mon, Jul 27, 2020 at 10:59:33PM -0700, Andrii Nakryiko wrote:
> > On Mon, Jul 27, 2020 at 4:15 PM Roman Gushchin <[email protected]> wrote:
> > >
> > > On Mon, Jul 27, 2020 at 03:05:11PM -0700, Andrii Nakryiko wrote:
> > > > On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
> > > > >
> > > > > As bpf is not using memlock rlimit for memory accounting anymore,
> > > > > let's remove the related code from libbpf.
> > > > >
> > > > > Bpf operations can't fail because of exceeding the limit anymore.
> > > > >
> > > >
> > > > They can't in the newest kernel, but libbpf will keep working and
> > > > supporting old kernels for a very long time now. So please don't
> > > > remove any of this.
> > >
> > > Yeah, good point, agree.
> > > So we just can drop this patch from the series, no other changes
> > > are needed.
> > >
> > > >
> > > > But it would be nice to add a detection of whether kernel needs a
> > > > RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
> > > > detect this from user-space?
>
> Btw, do you mean we should add a new function to the libbpf API?
> Or just extend pr_perm_msg() to skip guessing on new kernels?
>
I think we have to do both. There is libbpf_util.h in libbpf, we could
add two functions there:
- libbpf_needs_memlock() that would return true/false if kernel is old
and needs RLIMIT_MEMLOCK
- as a convenience, we can also add libbpf_inc_memlock_by() and
libbpf_set_memlock_to(), which will optionally (if kernel needs it)
adjust RLIMIT_MEMLOCK?
I think for your patch set, given it's pretty big already, let's not
touch runqslower, libbpf, and perf code (I think samples/bpf are fine
to just remove memlock adjustment), and we'll deal with detection and
optional bumping of RLIMIT_MEMLOCK as a separate patch once your
change land.
> The problem with the latter one is that it's called on a failed attempt
> to create a map, so unlikely we'll be able to create a new one just to test
> for the "memlock" value. But it also raises a question what should we do
> if the creation of this temporarily map fails? Assume the old kernel and
> bump the limit?
Yeah, I think we'll have to make assumptions like that. Ideally, of
course, detection of this would be just a simple sysfs value or
something, don't know. Maybe there is already a way for kernel to
communicate something like that?
> Idk, maybe it's better to just leave the userspace code as it is for some time.
>
> Thanks!
On Thu, Jul 30, 2020 at 12:39:40PM -0700, Andrii Nakryiko wrote:
> On Wed, Jul 29, 2020 at 6:38 PM Roman Gushchin <[email protected]> wrote:
> >
> > On Mon, Jul 27, 2020 at 10:59:33PM -0700, Andrii Nakryiko wrote:
> > > On Mon, Jul 27, 2020 at 4:15 PM Roman Gushchin <[email protected]> wrote:
> > > >
> > > > On Mon, Jul 27, 2020 at 03:05:11PM -0700, Andrii Nakryiko wrote:
> > > > > On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <[email protected]> wrote:
> > > > > >
> > > > > > As bpf is not using memlock rlimit for memory accounting anymore,
> > > > > > let's remove the related code from libbpf.
> > > > > >
> > > > > > Bpf operations can't fail because of exceeding the limit anymore.
> > > > > >
> > > > >
> > > > > They can't in the newest kernel, but libbpf will keep working and
> > > > > supporting old kernels for a very long time now. So please don't
> > > > > remove any of this.
> > > >
> > > > Yeah, good point, agree.
> > > > So we just can drop this patch from the series, no other changes
> > > > are needed.
> > > >
> > > > >
> > > > > But it would be nice to add a detection of whether kernel needs a
> > > > > RLIMIT_MEMLOCK bump or not. Is there some simple and reliable way to
> > > > > detect this from user-space?
> >
> > Btw, do you mean we should add a new function to the libbpf API?
> > Or just extend pr_perm_msg() to skip guessing on new kernels?
> >
>
> I think we have to do both. There is libbpf_util.h in libbpf, we could
> add two functions there:
>
> - libbpf_needs_memlock() that would return true/false if kernel is old
> and needs RLIMIT_MEMLOCK
> - as a convenience, we can also add libbpf_inc_memlock_by() and
> libbpf_set_memlock_to(), which will optionally (if kernel needs it)
> adjust RLIMIT_MEMLOCK?
>
> I think for your patch set, given it's pretty big already, let's not
> touch runqslower, libbpf, and perf code (I think samples/bpf are fine
> to just remove memlock adjustment), and we'll deal with detection and
> optional bumping of RLIMIT_MEMLOCK as a separate patch once your
> change land.
Ok, works for me. Let me repost the kernel part + samples as v3.
>
>
> > The problem with the latter one is that it's called on a failed attempt
> > to create a map, so unlikely we'll be able to create a new one just to test
> > for the "memlock" value. But it also raises a question what should we do
> > if the creation of this temporarily map fails? Assume the old kernel and
> > bump the limit?
>
> Yeah, I think we'll have to make assumptions like that. Ideally, of
> course, detection of this would be just a simple sysfs value or
> something, don't know. Maybe there is already a way for kernel to
> communicate something like that?
For instance, we've /sys/kernel/cgroup/features for cgroup features:
it's a list of supported mount options for cgroup fs.
Idk if bpf deserves something similar, but as far as I remember,
we've discussed it a couple of years ago, and at that time the consensus
was that it's too hard to keep such list uptodate, so the userspace should
just try and fail. Idk if it's still valid.
Thank you!