From ada700b9062560dff1614fe2fe934bfaf6a9ca94 Mon Sep 17 00:00:00 2001
From: Cheng Chang <xcc@fb.com>
Date: Mon, 18 May 2020 17:23:22 -0700
Subject: [PATCH] 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
---
 env/io_posix.cc | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/env/io_posix.cc b/env/io_posix.cc
index ba4ee15ef..490848436 100644
--- a/env/io_posix.cc
+++ b/env/io_posix.cc
@@ -189,19 +189,19 @@ bool IsSyncFileRangeSupported(int fd) {
 /*
  * DirectIOHelper
  */
-#ifndef NDEBUG
 namespace {
 
 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) {
   return uintptr_t(ptr) % sector_size == 0;
 }
-
-}  // namespace
 #endif
+}  // namespace
 
 /*
  * PosixSequentialFile
@@ -276,7 +276,7 @@ IOStatus PosixSequentialFile::PositionedRead(uint64_t offset, size_t n,
     ptr += r;
     offset += 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
       // of the file.
       break;
@@ -711,13 +711,22 @@ IOStatus PosixRandomAccessFile::MultiRead(FSReadRequest* reqs,
           // comment
           // https://github.com/facebook/rocksdb/pull/6441#issuecomment-589843435
           // Fall back to pread in this case.
-          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());
+          if (use_direct_io() &&
+              !IsSectorAligned(req_wrap->finished_len,
+                               GetRequiredBufferAlignment())) {
+            // Bytes reads don't fill sectors. Should only happen at the end
+            // of the file.
+            req->result = Slice(req->scratch, req_wrap->finished_len);
+            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) {
           assert(bytes_read > 0);
           assert(bytes_read + req_wrap->finished_len < req->len);