From: Xuehan Xu <xxhdx1985126@gmail.com>
Date: Tue, 21 Mar 2023 05:52:43 +0000 (+0000)
Subject: crimson/os/seastore: don't set INVALID extents to CLEAN when reading
X-Git-Tag: v19.0.0~1532^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=729dabd8ff20a747e02c15113e005a6d61acc8d7;p=ceph.git

crimson/os/seastore: don't set INVALID extents to CLEAN when reading
extents

CLEAN_PENDING extents may be invalidated before read completes:

1. transaction A retired an laddr, which lead to a RetirePlaceHolder in
   Cache
2. transaction B try to read that extent, and replace A's
   RetirePlaceHolder with it;
3. transaction A commits and invalidate that extent;
4. transaction B complete reading that extent;

In this case, we shouldn't set the extent's state to CLEAN

Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
---

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 575d148408f65..253836276675d 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -1321,11 +1321,15 @@ private:
     ).safe_then(
       [extent=std::move(extent)]() mutable {
         LOG_PREFIX(Cache::read_extent);
-        extent->state = CachedExtent::extent_state_t::CLEAN;
-        /* TODO: crc should be checked against LBA manager */
-        extent->last_committed_crc = extent->get_crc32c();
-
-        extent->on_clean_read();
+	if (likely(extent->state == CachedExtent::extent_state_t::CLEAN_PENDING)) {
+	  extent->state = CachedExtent::extent_state_t::CLEAN;
+	  /* TODO: crc should be checked against LBA manager */
+	  extent->last_committed_crc = extent->get_crc32c();
+
+	  extent->on_clean_read();
+	} else {
+	  ceph_assert(!extent->is_valid());
+	}
         extent->complete_io();
         SUBDEBUG(seastore_cache, "read extent done -- {}", *extent);
         return get_extent_ertr::make_ready_future<TCachedExtentRef<T>>(