add MAX_ENTRIES+1 test

[YoSTEALTH]

Currently testing max_entries+1 segfaults! It should return -errno

#define MAX_ENTRIES 32768 + 1
io_uring_queue_init(MAX_ENTRIES, &ring, 0)

Thank You

[ammarfaizi2]

I can't reproduce it; it returns -22 on my machine. Can you give a more detailed reproducer?

ammarfaizi2@integral2:~/app/liburing$ git log -n1
commit c1cee95b0266c360a8017248704d46d94dd72922 (HEAD -> master, jens-tree-orig/master)
Merge: f1dfb94bdaf1de14 f1a3d813c2f41a7b
Author: Jens Axboe <axboe@kernel.dk>
Date:   Sun Apr 14 06:23:36 2024 -0600

    Merge branch 'fix_clear_flags' of https://github.com/gxuu/liburing

    * 'fix_clear_flags' of https://github.com/gxuu/liburing:
      Add initialization function of io_uring_sqe
ammarfaizi2@integral2:~/app/liburing$ 
ammarfaizi2@integral2:~/app/liburing$ cat q.c 

#include <liburing.h>
#include <stdio.h>

int main(void)
{
    struct io_uring ring;
    int ret;

    #define MAX_ENTRIES 32768 + 1
    ret = io_uring_queue_init(MAX_ENTRIES, &ring, 0);
    printf("ret: %d\n", ret);
    return 0;
}
ammarfaizi2@integral2:~/app/liburing$ 
ammarfaizi2@integral2:~/app/liburing$ gcc -O3 q.c -o q src/liburing.a
ammarfaizi2@integral2:~/app/liburing$ ./q
ret: -22
ammarfaizi2@integral2:~/app/liburing$ 

[alviroiskandar]

Currently testing max_entries+1 segfaults! It should return -errno

#define MAX_ENTRIES   32768 + 1
io_uring_queue_init(MAX_ENTRIES, &ring, 0)

What liburing commit you use? The current master branch doesn't reproduce on my machine too

viro@freezing-night:/tmp/liburing$ cat test.c
#include <liburing.h>
int main() {
#define MAX_ENTRIES 32768 + 1
struct io_uring ring;                         
return -io_uring_queue_init(MAX_ENTRIES, &ring, 0);
}
viro@freezing-night:/tmp/liburing$ gcc test.c -luring
viro@freezing-night:/tmp/liburing$ ./a.out
viro@freezing-night:/tmp/liburing$ echo $?
22
viro@freezing-night:/tmp/liburing$ 

[YoSTEALTH]

Thanks @ammarfaizi2 @alviroiskandar for testing this. Hmm... I am testing this on Python side weird why its raising segfault! Will look into it.

edit: when I run this like so it works!, looks like it was pytest that was glitching out and causing segfault!

import liburing

ring = liburing.io_uring()
liburing.io_uring_queue_init(32768+1, ring)

# OSError: [Errno 22] Invalid argument

I created this issue since I didn't noticed any test for MAX_ENTRIES+1 in liburing/test.

[isilence]

FWIW, any test like that would need to expect that >MAX_ENTRIES may actually succeed if something change in the future.

[YoSTEALTH]

FWIW, any test like that would need to expect that >MAX_ENTRIES may actually succeed if something change in the future.

Wouldn't it be prudent to add an 32768+1 check now, and if/when these values change to update that test to reflect new value?

[isilence]

Then one day a user would install a new kernel without upgrading liburing and see failed liburing tests. The user will go complaining that the kernel broke userspace, even though it was expected and predicted. It's definitely an unpleasant situation and better to be avoided.

[axboe]

Yep I would not test if that particular thing would fail, because what's the point? I'd leave the testing for the functional parts. Since we'll soon have non-contig ring/sqe allocations being a possibility, it'd be trivial to bump these values in the future and then the test would fail. Testing if MAX_ENTRIES+1 fails is pointless.

If it segfaults on your end, then that sounds like a test problem. The kernel checks for it and returns -EINVAL, currently, but may allow it in the future if the limit is raised.

[YoSTEALTH]

Cool. "non-contig ring/sqe"?

[axboe]

It's kernel side implementation specific, but the max sizes of rings was limited as the kernel had to allocate this memory such that it was physically contiguous. Starting with 6.10 that is no longer the case, hence it'd be very easy to relax this restriction and allow larger rings.