Skip to content

Do not discard high 32 bits of length in send #1436

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.

Sign up for GitHub

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

Open
bitonic wants to merge 1 commit into
base: master
Choose a base branch
from
+12 −6
Open

Do not discard high 32 bits of length in send #1436

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions src/include/liburing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,17 +919,23 @@ IOURINGINLINE void io_uring_prep_statx(struct io_uring_sqe *sqe, int dfd,
sqe->statx_flags = (__u32) flags;
}

static inline __u32 __io_uring_cap_len(size_t len)
{
size_t mask = -(size_t)(len > UINT32_MAX);
return (len & ~mask) | (UINT32_MAX & mask);
Copy link
Contributor

@tavianator tavianator Jul 24, 2025

Choose a reason for hiding this comment

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

Based on https://godbolt.org/z/zGeqGGMeh, I'd suggest

Suggested change
return (len & ~mask) | (UINT32_MAX & mask);
return len > UINT32_MAX ? UINT32_MAX : len;

Copy link
Author

@bitonic bitonic Jul 24, 2025

Choose a reason for hiding this comment

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

I had this back and forth with @axboe already. I think the cmov or branching version is better than the current version too (and moreover it'll leverage max instructions for architectures that have them, like aarch64). But since @axboe has stated that he prefers the bit twiddling version I'll let him decide.

Copy link
Contributor

@tavianator tavianator Jul 24, 2025

Choose a reason for hiding this comment

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

I read @axboe's comments here and on #1435, and to me they only indicated preference for something branchless over a branch. But all 3 alternatives I tried compile to branchless code, and only the last one is optimal on both GCC and Clang.

Copy link
Author

@bitonic bitonic Jul 24, 2025

Choose a reason for hiding this comment

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

That's not how I understood #1436 (comment) , given that it's replying to a comment that explicitly mentions cmov. But I'll let him comment.

}

IOURINGINLINE void io_uring_prep_fadvise(struct io_uring_sqe *sqe, int fd,
__u64 offset, __u32 len, int advice)
{
io_uring_prep_rw(IORING_OP_FADVISE, sqe, fd, NULL, (__u32) len, offset);
io_uring_prep_rw(IORING_OP_FADVISE, sqe, fd, NULL, __io_uring_cap_len(len), offset);
Copy link
Contributor

@tavianator tavianator Jul 24, 2025

Choose a reason for hiding this comment

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

len is a __u32 here, so __io_uring_cap_len is a no-op

sqe->fadvise_advice = (__u32) advice;
}

IOURINGINLINE void io_uring_prep_madvise(struct io_uring_sqe *sqe, void *addr,
__u32 length, int advice)
{
io_uring_prep_rw(IORING_OP_MADVISE, sqe, -1, addr, (__u32) length, 0);
io_uring_prep_rw(IORING_OP_MADVISE, sqe, -1, addr, __io_uring_cap_len(length), 0);
Copy link
Contributor

@tavianator tavianator Jul 24, 2025

Choose a reason for hiding this comment

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

Same here

sqe->fadvise_advice = (__u32) advice;
}

Expand All @@ -951,7 +957,7 @@ IOURINGINLINE void io_uring_prep_madvise64(struct io_uring_sqe *sqe, void *addr,
IOURINGINLINE void io_uring_prep_send(struct io_uring_sqe *sqe, int sockfd,
const void *buf, size_t len, int flags)
{
io_uring_prep_rw(IORING_OP_SEND, sqe, sockfd, buf, (__u32) len, 0);
io_uring_prep_rw(IORING_OP_SEND, sqe, sockfd, buf, __io_uring_cap_len(len), 0);
sqe->msg_flags = (__u32) flags;
}

Expand Down Expand Up @@ -983,7 +989,7 @@ IOURINGINLINE void io_uring_prep_send_zc(struct io_uring_sqe *sqe, int sockfd,
const void *buf, size_t len, int flags,
unsigned zc_flags)
{
io_uring_prep_rw(IORING_OP_SEND_ZC, sqe, sockfd, buf, (__u32) len, 0);
io_uring_prep_rw(IORING_OP_SEND_ZC, sqe, sockfd, buf, __io_uring_cap_len(len), 0);
sqe->msg_flags = (__u32) flags;
sqe->ioprio = zc_flags;
}
Expand Down Expand Up @@ -1021,7 +1027,7 @@ IOURINGINLINE void io_uring_prep_sendmsg_zc_fixed(struct io_uring_sqe *sqe,
IOURINGINLINE void io_uring_prep_recv(struct io_uring_sqe *sqe, int sockfd,
void *buf, size_t len, int flags)
{
io_uring_prep_rw(IORING_OP_RECV, sqe, sockfd, buf, (__u32) len, 0);
io_uring_prep_rw(IORING_OP_RECV, sqe, sockfd, buf, __io_uring_cap_len(len), 0);
sqe->msg_flags = (__u32) flags;
}

Expand Down Expand Up @@ -1131,7 +1137,7 @@ IOURINGINLINE void io_uring_prep_provide_buffers(struct io_uring_sqe *sqe,
void *addr, int len, int nr,
int bgid, int bid)
{
io_uring_prep_rw(IORING_OP_PROVIDE_BUFFERS, sqe, nr, addr, (__u32) len,
io_uring_prep_rw(IORING_OP_PROVIDE_BUFFERS, sqe, nr, addr, __io_uring_cap_len(len),
(__u64) bid);
sqe->buf_group = (__u16) bgid;
}
Expand Down