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

io_uring_prep_close_direct fails when IOSQE_FIXED_FILE is set #1325

Closed
FrothyB opened this issue Jan 7, 2025 · 4 comments
Closed

io_uring_prep_close_direct fails when IOSQE_FIXED_FILE is set #1325

FrothyB opened this issue Jan 7, 2025 · 4 comments

Comments

@FrothyB
Copy link

FrothyB commented Jan 7, 2025

I've identified some unexpected (at least to me) behavior when using IOSQE_FIXED_FILE with io_uring_prep_close_direct ops. When this flag is set, close operations consistently fail, while they succeed otherwise. The same flag has no effect on socket creation operations (using io_uring_prep_socket_direct_alloc or also io_uring_prep_socket_direct, the latter not demonstrated here). While the documentation doesn't state that IOSQE_FIXED_FILE should work with direct close operations or that it should be used, this was unexpected to me because most other direct ops require the flag, and especially because socket creation also seems to work fine with it.

I am on Linux 6.12.7 and liburing 2.8.

The attached code demonstrates this behavior by maintaining a rotating pool of socket file descriptors, where new sockets are created and old ones are closed to maintain a maximum number of open files. The code runs the same sequence twice - once with IOSQE_FIXED_FILE and once without. Here is the output I see:

liburing version: 2.8

=== Without IOSQE_FIXED_FILE ===

Created socket fd 0 at index 0
Created socket fd 1 at index 1
Created socket fd 2 at index 2
Closed fd 0
Created socket fd 0 at index 0
Closed fd 1
Created socket fd 1 at index 1
Closed fd 2
Created socket fd 2 at index 2
Closed fd 0
Closed fd 1
Closed fd 2

=== With IOSQE_FIXED_FILE ===

Created socket fd 0 at index 0
Created socket fd 1 at index 1
Created socket fd 2 at index 2
Close failed: Bad file descriptor
Created socket fd 3 at index 0
Close failed: Bad file descriptor
Created socket fd 4 at index 1
Close failed: Bad file descriptor
Created socket fd 5 at index 2
Close failed: Bad file descriptor
Close failed: Bad file descriptor
Close failed: Bad file descriptor
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include "liburing.h"

#define QUEUE_DEPTH 32
#define CONCURRENT_OPEN_MAX 3
#define TOTAL_TO_OPEN 6

struct op_data {
    int fd;
    int index;
};

static void handle_completion(struct io_uring_cqe *cqe, const char *op_type, int *fds, int idx) {
    if (cqe->res < 0) {
        fprintf(stderr, "%s failed: %s\n", op_type, strerror(-cqe->res));
        return;
    }
    if (strcmp(op_type, "Socket") == 0) {
        fds[idx] = cqe->res;
        printf("Created socket fd %d at index %d\n", cqe->res, idx);
    } else {
        printf("Closed fd %d\n", fds[idx]);
        fds[idx] = -1;
    }
}

void io_uring_do_op(struct io_uring *ring, const char* op, int* fds, int idx) {
    struct io_uring_cqe *cqe;
    if (io_uring_submit_and_wait(ring, 1) >= 0 && 
        io_uring_wait_cqe(ring, &cqe) >= 0) {
        handle_completion(cqe, op, fds, idx);
        io_uring_cqe_seen(ring, cqe);
    }
}

void manage_sockets(bool use_fixed_file) {
    struct io_uring ring;

    if (io_uring_queue_init(QUEUE_DEPTH, &ring, 0) < 0) {
        perror("io_uring_queue_init_params");
        return;
    }

    int fds[TOTAL_TO_OPEN];
    memset(fds, -1, sizeof(fds));
    io_uring_register_files(&ring, fds, TOTAL_TO_OPEN);
    
    int oldest_idx = 0;
    struct io_uring_sqe *sqe;

    for (int i = 0; i < TOTAL_TO_OPEN + CONCURRENT_OPEN_MAX; i++) {
        // Close oldest fd if needed
        if ((i >= CONCURRENT_OPEN_MAX || i >= TOTAL_TO_OPEN) && fds[oldest_idx] != -1) {
            sqe = io_uring_get_sqe(&ring);
            if (!sqe) continue;

            io_uring_prep_close_direct(sqe, fds[oldest_idx]);
            if (use_fixed_file) io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE);

            io_uring_do_op(&ring, "Close", fds, oldest_idx);
        }

        // Open new socket
        if (i < TOTAL_TO_OPEN) {
        sqe = io_uring_get_sqe(&ring);
        if (!sqe) continue;

        io_uring_prep_socket_direct_alloc(sqe, AF_INET, SOCK_STREAM, 0, 0);
        io_uring_sqe_set_flags(sqe, IOSQE_FIXED_FILE); //Has no effect as far as I can tell

        io_uring_do_op(&ring, "Socket", fds, oldest_idx);
        }

        oldest_idx = (oldest_idx + 1) % CONCURRENT_OPEN_MAX;
    }

    io_uring_queue_exit(&ring);
}

int main() {
    printf("liburing version: %d.%d\n", io_uring_major_version(), io_uring_minor_version());
    printf("\n=== Without IOSQE_FIXED_FILE ===\n\n");
    manage_sockets(false);
    printf("\n=== With IOSQE_FIXED_FILE ===\n\n");
    manage_sockets(true);
    return 0;
}
@axboe
Copy link
Owner

axboe commented Jan 7, 2025

Don't set IOSQE_FIXED_FILE on the close operation for fixed descriptors, you should just use io_uring_prep_close_direct() on the SQE. This is why you get -EBADF.

@axboe
Copy link
Owner

axboe commented Jan 7, 2025

Closing, reopen if there are further questions.

@axboe axboe closed this as completed Jan 7, 2025
@FrothyB
Copy link
Author

FrothyB commented Jan 7, 2025

Well, I'm already not setting it any more - hopefully at least this thread is helpful in case someone makes the same mistake in the future. It arose for me due to a helper function that was setting the flag for all my ops since I'm exclusively using registered fds.

Thanks for the input.

@axboe
Copy link
Owner

axboe commented Jan 7, 2025

It is a bit confusing imho, I'll add a blurb to the man page about it.

axboe added a commit that referenced this issue Jan 7, 2025
The application must not set IOSQE_FIXED_FILE for closing a direct
descriptor, the helper will correctly assign the direct descriptor
index to close.

Link: #1325
Signed-off-by: Jens Axboe <[email protected]>
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

2 participants