On Tue, May 10, 2022 at 8:34 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
>
> Hi Andrii,
>
> On Sun, May 08, 2022 at 05:41:47PM -0700, Andrii Nakryiko wrote:
> > Kernel imposes a pretty particular restriction on ringbuf map size. It
> > has to be a power-of-2 multiple of page size. While generally this isn't
> > hard for user to satisfy, sometimes it's impossible to do this
> > declaratively in BPF source code or just plain inconvenient to do at
> > runtime.
> >
> > One such example might be BPF libraries that are supposed to work on
> > different architectures, which might not agree on what the common page
> > size is.
> >
> > Let libbpf find the right size for user instead, if it turns out to not
> > satisfy kernel requirements. If user didn't set size at all, that's most
> > probably a mistake so don't upsize such zero size to one full page,
> > though. Also we need to be careful about not overflowing __u32
> > max_entries.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> <snip>
>
> > +static size_t adjust_ringbuf_sz(size_t sz)
> > +{
> > + __u32 page_sz = sysconf(_SC_PAGE_SIZE);
> > + __u32 i, mul;
> > +
> > + /* if user forgot to set any size, make sure they see error */
> > + if (sz == 0)
> > + return 0;
> > + /* Kernel expects BPF_MAP_TYPE_RINGBUF's max_entries to be
> > + * a power-of-2 multiple of kernel's page size. If user diligently
> > + * satisified these conditions, pass the size through.
> > + */
> > + if ((sz % page_sz) == 0 && is_pow_of_2(sz / page_sz))
> > + return sz;
> > +
> > + /* Otherwise find closest (page_sz * power_of_2) product bigger than
> > + * user-set size to satisfy both user size request and kernel
> > + * requirements and substitute correct max_entries for map creation.
> > + */
> > + for (i = 0, mul = 1; ; i++, mul <<= 1) {
> > + if (mul > UINT_MAX / page_sz) /* prevent __u32 overflow */
> > + break;
> > + if (mul * page_sz > sz)
> > + return mul * page_sz;
> > + }
> > +
> > + /* if it's impossible to satisfy the conditions (i.e., user size is
> > + * very close to UINT_MAX but is not a power-of-2 multiple of
> > + * page_size) then just return original size and let kernel reject it
> > + */
> > + return sz;
> > +}
>
> This patch in -next as commit 0087a681fa8c ("libbpf: Automatically fix
> up BPF_MAP_TYPE_RINGBUF size, if necessary") breaks the build with tip
> of tree LLVM due to [1] strengthening -Wunused-but-set-variable:
>
> libbpf.c:4954:8: error: variable 'i' set but not used [-Werror,-Wunused-but-set-variable]
> __u32 i, mul;
> ^
> 1 error generated.
>
> Should i be removed or was it intended to be used somewhere that it is
> not?
my initial implementation had some hard limit on number of iterations
which I later removed. I'll drop the i completely, thanks for pointing
out.
>
> [1]: https://github.com/llvm/llvm-project/commit/2af845a6519c9cde5c8f58db5554f8b1084ce1ed
>
> Cheers,
> Nathan
