Skip to content

Commit

Permalink
src/setup: use correct sqe size for unmapping
Browse files Browse the repository at this point in the history
For some reason only one path actually cares about the SQE size in
io_uring_queue_exit(). Move the code into the generic case, and use the
sqe_size variable for both cases.

Otherwise it's unmapping less than what was mapped, if SQE128 is used.
This can hold the fd pinned, and hence defer release of the ring until
the task exits (or randomly unmaps the remaining range, which likely
will never happen as liburing should've done that).

Link: #1310
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
axboe committed Dec 30, 2024
1 parent 1a19d04 commit 460c1b4
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions src/setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,18 +430,17 @@ __cold void io_uring_queue_exit(struct io_uring *ring)
{
struct io_uring_sq *sq = &ring->sq;
struct io_uring_cq *cq = &ring->cq;
size_t sqe_size;
size_t sqe_size = sizeof(struct io_uring_sqe);

if (ring->flags & IORING_SETUP_SQE128)
sqe_size <<= 1;

if (!sq->ring_sz && !(ring->int_flags & INT_FLAG_APP_MEM)) {
sqe_size = sizeof(struct io_uring_sqe);
if (ring->flags & IORING_SETUP_SQE128)
sqe_size += 64;
__sys_munmap(sq->sqes, sqe_size * sq->ring_entries);
io_uring_unmap_rings(sq, cq);
} else {
if (!(ring->int_flags & INT_FLAG_APP_MEM)) {
__sys_munmap(sq->sqes,
*sq->kring_entries * sizeof(struct io_uring_sqe));
__sys_munmap(sq->sqes, *sq->kring_entries * sqe_size);
io_uring_unmap_rings(sq, cq);
}
}
Expand Down

4 comments on commit 460c1b4

@suxinggm
Copy link

@suxinggm suxinggm commented on 460c1b4 Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, appreciate!!! It works fine for me now. This issue has been bothering me about one month.

@axboe
Copy link
Owner Author

@axboe axboe commented on 460c1b4 Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was definitely a funky one to figure out!

@suxinggm
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just using the nvme passthrough way to send write/read/trim/flush command together. And also I need to have a high speed. So I try this way. At the beginning, I only use qd32 for test, but after I run stress test, the system always report a "Resources Temporary Unavailable" . Then I found this issue step by step to reduce my code. lol.

@axboe
Copy link
Owner Author

@axboe axboe commented on 460c1b4 Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good reproducer that's condensed down to the bare minimum makes any bug shallow and easy to fix. So greatly appreciated. I'll close the issue, thanks for the report.

Please sign in to comment.