append_challenged_posix_file_impl::read_dma: allow iovec to cross _lo…

…gical_size Currently append_challenged_posix_file_impl::read_dma trims the iov vector if it goes beyond _logical_size causing short reads if the file length is unaligned. We have to allow the iovec covering EOF to extend beyond it up to an offset aligned on disk_read_dma_alignment. Add a unit test that reproduces this case and passes with the fix. Test: unit(dev) Signed-off-by: Benny Halevy <[email protected]> Message-Id: <[email protected]>
StevenBH · Dec 1, 2020 · 8b400c7 · 8b400c7
1 parent 6cf0d55
commit 8b400c7
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 2 deletions.
diff --git a/src/core/file.cc b/src/core/file.cc
@@ -640,10 +640,10 @@ append_challenged_posix_file_impl::read_dma(uint64_t pos, std::vector<iovec> iov
     }
     size_t len = 0;
     auto i = iov.begin();
-    while (i != iov.end() && pos + len + i->iov_len <= _logical_size) {
+    auto aligned_logical_size = align_up<uint64_t>(_logical_size, _disk_read_dma_alignment);
+    while (i != iov.end() && pos + len + i->iov_len <= aligned_logical_size) {
         len += i++->iov_len;
     }
-    auto aligned_logical_size = align_up<uint64_t>(_logical_size, _disk_read_dma_alignment);
     if (i != iov.end()) {
         auto last_len = pos + len + i->iov_len - aligned_logical_size;
         if (last_len) {

diff --git a/tests/unit/file_io_test.cc b/tests/unit/file_io_test.cc
@@ -21,6 +21,7 @@
 
 #include <seastar/testing/test_case.hh>
 #include <seastar/testing/thread_test_case.hh>
+#include <seastar/testing/test_runner.hh>
 
 #include <seastar/core/seastar.hh>
 #include <seastar/core/semaphore.hh>
@@ -712,3 +713,44 @@ SEASTAR_TEST_CASE(test_destruct_append_challenged_file_after_read) {
         f.dma_read(0, buf.get(), 4096).get();
     });
 }
+
+SEASTAR_TEST_CASE(test_dma_iovec) {
+    return tmp_dir::do_with_thread([] (tmp_dir& t) {
+        static constexpr size_t alignment = 4096;
+        auto wbuf = allocate_aligned_buffer<char>(alignment, alignment);
+        size_t size = 1234;
+        std::fill_n(wbuf.get(), alignment, char(0));
+        std::fill_n(wbuf.get(), size, char(42));
+        std::vector<iovec> iovecs;
+
+        auto filename = (t.get_path() / "testfile.tmp").native();
+        auto f = open_file_dma(filename, open_flags::rw | open_flags::create).get0();
+        iovecs.push_back(iovec{ wbuf.get(), alignment });
+        auto count = f.dma_write(0, iovecs).get0();
+        BOOST_REQUIRE_EQUAL(count, alignment);
+        f.truncate(size).get();
+        f.close().get();
+
+        auto rbuf = allocate_aligned_buffer<char>(alignment, alignment);
+
+        // this tests the posix_file_impl
+        f = open_file_dma(filename, open_flags::ro).get0();
+        std::fill_n(rbuf.get(), alignment, char(0));
+        iovecs.clear();
+        iovecs.push_back(iovec{ rbuf.get(), alignment });
+        count = f.dma_read(0, iovecs).get0();
+        BOOST_REQUIRE_EQUAL(count, size);
+
+        BOOST_REQUIRE(std::equal(wbuf.get(), wbuf.get() + alignment, rbuf.get(), rbuf.get() + alignment));
+
+        // this tests the append_challenged_posix_file_impl
+        f = open_file_dma(filename, open_flags::rw).get0();
+        std::fill_n(rbuf.get(), alignment, char(0));
+        iovecs.clear();
+        iovecs.push_back(iovec{ rbuf.get(), alignment });
+        count = f.dma_read(0, iovecs).get0();
+        BOOST_REQUIRE_EQUAL(count, size);
+
+        BOOST_REQUIRE(std::equal(wbuf.get(), wbuf.get() + alignment, rbuf.get(), rbuf.get() + alignment));
+    });
+}