trpc-group / trpc-cpp

A pluggable, high-performance RPC framework written in cpp
Other
270 stars 78 forks source link

The return value from io_uring_submit is not being handled correctly. #131

Closed Tianpingan closed 3 months ago

Tianpingan commented 4 months ago

Hi, developers ! I want to try tRPC in my project, one that uses io_uring with SQ_POLL. However, I noticed a small mistake in async_io.cc when handling the return value of io_uring_submit(). When using SQ_POLL, io_uring_submit() may return a higher number of submitted entries than actually submitted

On success io_uring_submit(3) returns the number of submitted submission queue entries, if SQPOLL is not used. If SQPOLL is used, the return value may report a higher number of submitted entries than actually submitted. If the the user requires accurate information about how many submission queue entries have been successfully submitted, while using SQPOLL, the user must fall back to repeatedly submitting a single submission queue entry. On failure it returns -errno. from:https://man7.org/linux/man-pages/man3/io_uring_submit.3.html


// trpc/runtime/iomodel/async_io/async_io.cc
// SubmitOne:
int ret = io_uring_submit(ring);
if (TRPC_UNLIKELY(ret != 1)) {
trpc::object_pool::Delete<IOURingUserData>(user_data);
if (ret < 0) {
return MakeExceptionFuture<int32_t>(AsyncIOError(ret));
}
std::string msg = "Submit success but return not one, ret:" + std::to_string(ret);
return MakeExceptionFuture<int32_t>(AsyncIOError(AsyncIOError::SUBMIT_FAIL, msg));
}
submitted_++;

return user_data->pr.GetFuture();


Therefore, in the case of using SQ_POLL, `ret != 1` may not indicate a submission failure.

ref:
https://man7.org/linux/man-pages/man3/io_uring_submit.3.html
https://github.com/axboe/liburing/issues/88
https://github.com/axboe/liburing/pull/966
yanyupeng2018 commented 3 months ago

Hi, Tianpingan! There is indeed a problem when handling return value from io_uring_submit. We only considered submit one SQE at once before. We will fix it soon.

helloopenworld commented 3 months ago

fixed