Re-read the whole request in direct IO mode when IO uring returns partial result (#6853)

Summary:
If both direct IO and IO uring are enabled, when IO uring returns partial result, we'll try to read the remaining part of the request, but the starting address/offset of the remaining part might not be aligned to the block size, in direct IO mode, the unaligned offset causes bug.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/6853

Test Plan: run make check with both direct IO and IO uring enabled, this is covered by one of the continuous tests.

Reviewed By: anand1976

Differential Revision: D21603023

Pulled By: cheng-chang

fbshipit-source-id: 942f6a11ff21e1892af6c4464e02bab4c707787c
main
Cheng Chang 5 years ago committed by Facebook GitHub Bot
parent b9d65f5aa6
commit ada700b906
  1. 33
      env/io_posix.cc

33
env/io_posix.cc vendored

@ -189,19 +189,19 @@ bool IsSyncFileRangeSupported(int fd) {
/* /*
* DirectIOHelper * DirectIOHelper
*/ */
#ifndef NDEBUG
namespace { namespace {
bool IsSectorAligned(const size_t off, size_t sector_size) { bool IsSectorAligned(const size_t off, size_t sector_size) {
return off % sector_size == 0; assert((sector_size & (sector_size - 1)) == 0);
return (off & (sector_size - 1)) == 0;
} }
#ifndef NDEBUG
bool IsSectorAligned(const void* ptr, size_t sector_size) { bool IsSectorAligned(const void* ptr, size_t sector_size) {
return uintptr_t(ptr) % sector_size == 0; return uintptr_t(ptr) % sector_size == 0;
} }
} // namespace
#endif #endif
} // namespace
/* /*
* PosixSequentialFile * PosixSequentialFile
@ -276,7 +276,7 @@ IOStatus PosixSequentialFile::PositionedRead(uint64_t offset, size_t n,
ptr += r; ptr += r;
offset += r; offset += r;
left -= r; left -= r;
if (r % static_cast<ssize_t>(GetRequiredBufferAlignment()) != 0) { if (!IsSectorAligned(r, GetRequiredBufferAlignment())) {
// Bytes reads don't fill sectors. Should only happen at the end // Bytes reads don't fill sectors. Should only happen at the end
// of the file. // of the file.
break; break;
@ -711,13 +711,22 @@ IOStatus PosixRandomAccessFile::MultiRead(FSReadRequest* reqs,
// comment // comment
// https://github.com/facebook/rocksdb/pull/6441#issuecomment-589843435 // https://github.com/facebook/rocksdb/pull/6441#issuecomment-589843435
// Fall back to pread in this case. // Fall back to pread in this case.
Slice tmp_slice; if (use_direct_io() &&
req->status = !IsSectorAligned(req_wrap->finished_len,
Read(req->offset + req_wrap->finished_len, GetRequiredBufferAlignment())) {
req->len - req_wrap->finished_len, options, &tmp_slice, // Bytes reads don't fill sectors. Should only happen at the end
req->scratch + req_wrap->finished_len, dbg); // of the file.
req->result = req->result = Slice(req->scratch, req_wrap->finished_len);
Slice(req->scratch, req_wrap->finished_len + tmp_slice.size()); req->status = IOStatus::OK();
} else {
Slice tmp_slice;
req->status =
Read(req->offset + req_wrap->finished_len,
req->len - req_wrap->finished_len, options, &tmp_slice,
req->scratch + req_wrap->finished_len, dbg);
req->result =
Slice(req->scratch, req_wrap->finished_len + tmp_slice.size());
}
} else if (bytes_read < req_wrap->iov.iov_len) { } else if (bytes_read < req_wrap->iov.iov_len) {
assert(bytes_read > 0); assert(bytes_read > 0);
assert(bytes_read + req_wrap->finished_len < req->len); assert(bytes_read + req_wrap->finished_len < req->len);

Loading…
Cancel
Save