seastar icon indicating copy to clipboard operation
seastar copied to clipboard

reactor: implement read_some(fd, buffer, len) in io_uring

Open tchaikov opened this issue 2 years ago • 6 comments

in this change, reactor_backend_uring::read_some(pollable_fd_state& fd, void* buffer, size_t len) is also implemented with the speculated read, like the other overload read_some() methods in this class.

this change reverts 67a3951b2812b3a182030ab4441b6aea5543ccbd. it relies on newer kernel which does not suffer from the issue spotted in #1386. this change should address the test failure of Seastar.unit.fsnotifier test on 5.11 linux kernel after making io_uring the default backend.

Signed-off-by: Kefu Chai [email protected]

tchaikov avatar Oct 22 '22 04:10 tchaikov

27/73 Test #27: Seastar.unit.fsnotifier .......................***Failed    0.62 sec
[0/1] cd /home/circleci/project/build/debug/tests/unit && /home/circleci/project/build/debug/tests/unit/fsnotifier_test -- -c 2
Running 6 test cases...
WARNING: debug mode. Not for benchmarking or production
WARN  2022-10-22 05:02:17,510 [shard 1] seastar - Creation of perf_event based stall detector failed, falling back to posix timer: std::system_error (error system:13, perf_event_open() failed: Permission denied)
WARN  2022-10-22 05:02:17,510 [shard 0] seastar - Creation of perf_event based stall detector failed, falling back to posix timer: std::system_error (error system:13, perf_event_open() failed: Permission denied)
INFO  2022-10-22 05:02:17,529 [shard 1] seastar - Created fair group io-queue-0, capacity rate 2147483:2147483, limit 12582912, rate 16777216 (factor 1), threshold 2000
INFO  2022-10-22 05:02:17,531 [shard 1] seastar - IO queue uses 0.75ms latency goal for device 0
INFO  2022-10-22 05:02:17,532 [shard 1] seastar - Created io group dev(0), length limit 4194304:4194304, rate 2147483647:2147483647
INFO  2022-10-22 05:02:17,533 [shard 0] seastar - Created io queue dev(0) capacities: 512:2000:2000 1024:3000:3000 2048:5000:5000 4096:9000:9000 8192:17000:17000 16384:33000:33000 32768:65000:65000 65536:129000:129000 131072:257000:257000
random-seed=319783116
==238==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
unknown location(0): fatal error: in "test_notify_modify_close_delete": std::system_error: Resource temporarily unavailable
/home/circleci/project/src/testing/seastar_test.cc(43): last checkpoint
unknown location(0): fatal error: in "test_notify_overwrite": std::system_error: Resource temporarily unavailable
/home/circleci/project/src/testing/seastar_test.cc(43): last checkpoint
unknown location(0): fatal error: in "test_notify_create_delete_child": std::system_error: Resource temporarily unavailable
/home/circleci/project/src/testing/seastar_test.cc(43): last checkpoint
unknown location(0): fatal error: in "test_notify_open": std::system_error: Resource temporarily unavailable
/home/circleci/project/src/testing/seastar_test.cc(43): last checkpoint
unknown location(0): fatal error: in "test_notify_move": std::system_error: Resource temporarily unavailable
/home/circleci/project/src/testing/seastar_test.cc(43): last checkpoint
unknown location(0): fatal error: in "test_shutdown_notifier": std::system_error: Resource temporarily unavailable
/home/circleci/project/src/testing/seastar_test.cc(43): last checkpoint

still returns -EAGAIN.

tchaikov avatar Oct 22 '22 05:10 tchaikov

tested and failed at #1255, but i still think this change is an improvement.

tchaikov avatar Oct 22 '22 05:10 tchaikov

LGTM, thanks

xemul avatar Oct 25 '22 10:10 xemul

this change is marked as a draft. as it would fail the tests before we are able to test using newer linux kernels.

tchaikov avatar Jan 23 '23 10:01 tchaikov

this change is marked as a draft. as it would fail the tests before we are able to test using newer linux kernels.

We can't depend on the kernel version (except internally to enable/disable features).

avikivity avatar Jan 23 '23 11:01 avikivity

yeah, i am marking it as a draft. it can be marked ready for review only if we can depend on the kernel version for the reason you put above.

tchaikov avatar Jan 23 '23 11:01 tchaikov