Attention is currently required from: jolly, pespin.
6 comments:
File src/core/osmo_io_internal.h:
Patch Set #2, Line 110: void *read_ring[IOFD_MSGHDR_READ_SQES];
some comments here would be useful to explan what those fields are and how hey are used (not just the read_ring now, but also the previously-introduced read_msghdr, read_len, ...
Also: why are those pointers void? if it points to a ring, shouldn't it be 'struct io_uring *' for read_ring and write_ring?
File src/core/osmo_io_uring.c:
Patch Set #2, Line 67: g_io_uring_queue
pleaes use a more descriptive name. The variable is not a qeuue, but it is the size of the ring. Something like g_io_uring_size or g_io_uring_num_entries would be more descriptive.
Patch Set #2, Line 113: OSMO_IO_URING_QUEUE
like the variable name in the code, the environment variable name shold be self-descriptive. Also, since it's the *initial* size, something like OSMO_IO_URING_INITIAL_{SIZE,NUM_ENTRIES} would make most sense.
Naming does matter...
Patch Set #2, Line 163: osmo_iofd_uring_get_sqe
the 'osmo_' prefix is reserverd for exported functions. You are adding a static function, it should not have an osmo_ prefix.
Patch Set #2, Line 172: LOGP(DLIO, LOGL_NOTICE, "io_uring too small to handle all SQEs, increasing size to %d.\n", g_io_uring_queue);
minor: while the current implementation alsways doubles the size, it might still be useful to print the old [too small] size?
is there code to clean up the old ring after completion of the last cqe of the old ring?
To view, visit change 40725. To unsubscribe, or for help writing mail filters, visit settings.