Do not discard high 32 bits of length in send

[bitonic]

Fixes #1435

[bitonic]

@axboe regarding branch or not branch: imo a predictable branch is better than a cmov (as in, it'll be cheaper in practice). But if you really care about having a cmov, I can make sure it compiles to that.

[axboe]

I really think this should handle all the cases where this is true. And this is very much using a branch, even if it's marked unlikely. A cap function ala:

size_t mask = -(len > UINT32_MAX);
return (unsigned int )((len & ~mask) | (UINT32_MAX & mask));

would be more efficient, and that help can just be used anywhere.

Also note the format of git commits for liburing. Needs a proper commit subject, commit message, identity, and signed-off-by line.

[axboe]

Gotta run for now, I'll check back in tomorrow.

[bitonic]

@axboe I've pushed the bit twiddly version. I've also amended all similar call sites I could find with a __u32.*len grep.

Should I just put the commit to be signed off by you?

[bitonic]

@axboe is anything holding up the merge (apart from the commit message, which I'll amend if you confirm that I should put it as signed off by you)?

[axboe]

Sorry, OOO for a bit, back next week. Only thing holding this back is I'm still pondering if we shouldn't just document the fact that the API len is a 32-bit unsigned. I'm not really convinced that it's worth adding code to cap the transfer length.

[axboe]

{ should be on the next line. Minor style issue.

[axboe]

Oh and this one should just be a static inline, not IOURINGINLINE.

[bitonic]

I've fixed both issues.

[bitonic]

Sorry, OOO for a bit, back next week. Only thing holding this back is I'm still pondering if we shouldn't just document the fact that the API len is a 32-bit unsigned. I'm not really convinced that it's worth adding code to cap the transfer length.

I think silently dropping the high 32 bits is terrible, documentation notwithstanding. send doesn't do that, so people will reasonably assume that the io_uring version will do the same, and it's a condition that will happen very rarely.

Are you concerned about performance? Capping the length will be immaterial compared to the cost of the place/pickup of the SQE onto the ring anyway.

[bitonic]

I think io_uring_prep_fadvise/io_uring_prep_madvise/io_uring_prep_provide_buffers are a lot more dubious (it should really have been a 64-bit length all along). But for send/recv there are basically no downsides.

[tavianator]

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

[tavianator]

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;

[bitonic]

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.

[tavianator]

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.

[bitonic]

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.

[tavianator]

Same here

[bitonic]

@axboe any news? liburing is still silently broken (both in the docs and in the types) if you do large writes.