Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

liburing feature: return memory that'd be locked for a certain ring size #246

Closed
anarazel opened this issue Dec 4, 2020 · 15 comments
Closed

Comments

@anarazel
Copy link
Contributor

anarazel commented Dec 4, 2020

Hi,

For applications that do not run as root, and that are often run outside of very controlled environments, there is a tension between the number and size of rings used by default, and the likelihood of running into RLIMIT_MEMLOCK.

It'd be useful if liburing had a function that calculated how much memory a ring of a certain size would take. That'd make it easier for applications to tune themselves appropriately.

I can see two possible ways to implement this:

  • copy the ring_pages() logic from the kernel into liburing (but clamping etc make that a not entirely trivial)
  • add a kernel API for determining this, similar to IORING_REGISTER_PROBE

Regards,

Andres

@axboe
Copy link
Owner

axboe commented Dec 4, 2020

This should just go in liburing imho. I'm taking name suggestions :-)

@anarazel
Copy link
Contributor Author

anarazel commented Dec 4, 2020

This should just go in liburing imho. I'm taking name suggestions :-)

How about:

size_t io_uring_memlock_required(unsigned entries, unsigned flags);
size_t io_uring_memlock_required_params(unsigned entries, struct io_uring_params *p);

not super happy about the memlock_required. Perhaps memlock_size?

If you wanted to make it a bit more general, it could be something like

struct io_uring_sizes {
    size_t kernel_memlock;
    ...
};

void io_uring_get_sizes(struct io_uring_sizes *s, unsigned entries, unsigned flags);
void io_uring_get_sizes_params(struct io_uring_sizes *s, unsigned entries, struct io_uring_params *p);

or such, but I'm a bit doubtful it's warranted.

@axboe
Copy link
Owner

axboe commented Dec 8, 2020

I guess the trickier bit here is ensuring this works regardless of page size in the system, as that may impact it. I guess that's a vote for actually putting this in the kernel, outside of just having the one implementation of it...

@grooverdan
Copy link

Hi @anarazel,
Hi @axboe,

To add another vote, yes please. At the moment MariaDB has a really crap error message that only partially hints at a solution - https://github.com/MariaDB/server/blob/10.6/tpool/aio_liburing.cc#L41-L43.

@hassila
Copy link

hassila commented Apr 9, 2021

This should just go in liburing imho. I'm taking name suggestions :-)

How about:

size_t io_uring_memlock_required(unsigned entries, unsigned flags);
size_t io_uring_memlock_required_params(unsigned entries, struct io_uring_params *p);

not super happy about the memlock_required. Perhaps memlock_size?

I'd suggest small tweak (to the simple version):

size_t io_uring_mlock_size(unsigned entries, unsigned flags);
size_t io_uring_mlock_size_params(unsigned entries, struct io_uring_params *p);

@axboe
Copy link
Owner

axboe commented Apr 9, 2021

Question is how this should behave for 5.12 and newer, where we don't memlock anything? I guess we can just return 0 for that case.

@hassila
Copy link

hassila commented Apr 9, 2021

Seems very reasonable, yes.

@grooverdan
Copy link

Works for me.

@axboe
Copy link
Owner

axboe commented Apr 10, 2021

Can either (or both!) of you try that mlock-size branch? It provides the two requested helpers. I can some quick sanity checking here and it looks sane.

https://git.kernel.dk/cgit/liburing/commit/?h=mlock-size&id=ea0fd51b78b3c089add37e8c4ee61abd460beb7a

@grooverdan
Copy link

ok. I'll take a spin/test in the next few days. Thank you.

@grooverdan
Copy link

Can I just clarify the units returned?

Thread 1 "mysqld" hit Breakpoint 1, io_uring_mlock_size (entries=2048, flags=0) at setup.c:336
336		struct io_uring_params p = { .flags = flags, };
Missing separate debuginfos, use: dnf debuginfo-install openssl-libs-1.1.1k-1.fc33.x86_64 systemd-libs-246.13-1.fc33.x86_64
(gdb) c
Continuing.
2021-04-13 19:24:00 0 [Note] mysqld: io_uring_queue_init() failed with ENOMEM: try larger ulimit -l (2112 bytes required)

so 2112 is the raw value returned.

I'm getting ENOMEM on ulimit -l 128 but succeeding on ulimit -l 256.

No locked memory is currently used.

$ grep locked /proc/872691/limits 
Max locked memory         131072               131072               bytes     

$ grep  VmLck /proc/872691/status
VmLck:	       0 kB

@axboe
Copy link
Owner

axboe commented Apr 13, 2021

Hmm, that seems odd. A quick test here with (2048, 0) returns 262144, which seems more in line with what you need. The returned value is in bytes. Where's your 2112 coming from? Running with (1, 0) here returns 8192, 8k, or 4 pages.

@grooverdan
Copy link

Ok found it, a 2112 was my error. However running on an older kernel (5.11.10) without sufficient locked memory available causes io_uring_queue_init_params in io_uring_mlock_size_params to fail with ENOMEM.

This seems to be for the population of lp.features so the newer kernels can return 0. So that the added functions don't return -ENOMEM (I could of got that directly from io_uring_queue_init), how about:

diff --git a/src/setup.c b/src/setup.c
index 111db61..b117e32 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -287,10 +287,8 @@ ssize_t io_uring_mlock_size_params(unsigned entries, struct io_uring_params *p)
        ssize_t ret;
 
        ret = io_uring_queue_init_params(entries, &ring, &lp);
-       if (ret < 0)
-               return ret;
-
-       io_uring_queue_exit(&ring);
+       if (!ret)
+               io_uring_queue_exit(&ring);
 
        /*
         * Native workers imply using cgroup memory accounting, and hence no

lp retains its initial 0 values, lp.features & IORING_FEAT_NATIVE_WORKERS can never be true, and the calculation continues.

I've tested this an it generates the correct result for me:

$ ulimit -l 64
...

Thread 1 "mysqld" hit Breakpoint 1, io_uring_mlock_size (entries=2048, flags=0) at setup.c:334
334		struct io_uring_params p = { .flags = flags, };
Missing separate debuginfos, use: dnf debuginfo-install openssl-libs-1.1.1k-1.fc33.x86_64 systemd-libs-246.13-1.fc33.x86_64
(gdb) finish
Run till exit from #0  io_uring_mlock_size (entries=2048, flags=0) at setup.c:334
(anonymous namespace)::aio_uring::aio_uring (this=0x1ffb7f0, tpool=0x1fb7ba0, max_aio=2048) at /home/dan/repos/mariadb-server-10.6-test/tpool/aio_liburing.cc:39
39	    if (io_uring_queue_init(max_aio, &uring_, 0) != 0)
Value returned is $1 = 262144

@axboe
Copy link
Owner

axboe commented Apr 14, 2021

Yes, that's a good idea, I'll add that.

@axboe axboe closed this as completed in ea0fd51 Apr 14, 2021
@axboe
Copy link
Owner

axboe commented Apr 14, 2021

Done, and merged to master.

grooverdan added a commit to MariaDB/server that referenced this issue Apr 14, 2021
This gives the user the size required and how to set
memlock limits for the process.

Thanks Jens Axboe for providing this requested interface

ref: axboe/liburing#246

Also don't put \n on my_printf_error, its implicit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants