From 46b56661765ca395ce9b0dffa4f91ccebbed5b36 Mon Sep 17 00:00:00 2001
From: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Date: Tue, 6 Feb 2024 22:55:22 +0900
Subject: [PATCH] libnilfsgc, cleanerd, nilfs-resize: treat scrapped segments
 as unprotected

An issue exists where scrapped segments generated during recovery of
dsync blocks are incorrectly determined to be protected due to
comparisons using invalid segment summary sequence numbers on disk.

Segments that are incorrectly determined to be protected will not be
reclaimed for a long period of time, occupying disk space unnecessarily.
In addition, it may prevent nilfs-resize from shrinking the file system.

Fix these issues by using segment usage information to determine whether
a segment has been scrapped, in which case it is considered unprotected
without comparing sequence numbers.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
---
 include/nilfs_gc.h  | 30 ++++++++++++++++++++++++++++++
 lib/gc.c            | 11 +++++++++++
 sbin/cleanerd.c     |  9 ++++++---
 sbin/nilfs-resize.c | 18 ++++++++++++++++++
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/include/nilfs_gc.h b/include/nilfs_gc.h
index 2bf27a9..f818b7e 100644
--- a/include/nilfs_gc.h
+++ b/include/nilfs_gc.h
@@ -91,6 +91,36 @@ static inline int nilfs_suinfo_reclaimable(const struct nilfs_suinfo *si)
 		!nilfs_suinfo_active(si) && !nilfs_suinfo_error(si);
 }
 
+/**
+ * nilfs_suinfo_empty - determine whether a segment is empty based on its
+ *                      usage status
+ * @si: pointer to a segment usage information structure
+ *
+ * This function determines whether a segment is empty from the contents of
+ * @si.  If nilfs_suinfo_reclaimable() returns true and then this function
+ * also returns true, the segment is considered "scrapped" and treated as
+ * "unprotected" by GC.  And since the sui_nblocks value of the segment
+ * that the log writer is grabbing for the next write is also 0, this should
+ * normally be used in conjunction with nilfs_suinfo_reclaimable() to
+ * distinguish that state with the "active" flag.  (Note that the "active"
+ * flag is not a flag recorded on the media, but only visible via the API).
+ *
+ * This helper function is used to clarify that this purpose and caveat
+ * applies.
+ *
+ * The "scrapped" state of a segment (dirty and not active and
+ * sui_nblocks == 0) ensures that the segment is not accidentally allocated
+ * and overwritten during log writes in the recovery context, and that the
+ * segment is later freed by GC without being parsed.
+ *
+ * Return: true if the segment is empty, false otherwise.
+ */
+static inline int nilfs_suinfo_empty(const struct nilfs_suinfo *si)
+{
+	return si->sui_nblocks == 0;
+}
+
+
 extern void (*nilfs_gc_logger)(int priority, const char *fmt, ...);
 
 #endif /* NILFS_GC_H */
diff --git a/lib/gc.c b/lib/gc.c
index 9449352..a7e26f2 100644
--- a/lib/gc.c
+++ b/lib/gc.c
@@ -285,6 +285,17 @@ static ssize_t nilfs_acc_blocks(struct nilfs *nilfs,
 			continue;
 		}
 
+		if (nilfs_suinfo_empty(&si)) {
+			/*
+			 * "Scrapped" segment - the information in the segment
+			 * summary is not valid because it's unwritten.
+			 * Make it subject to reclaim without comparing
+			 * sequence numbers.
+			 */
+			i++;
+			continue;
+		}
+
 		ret = nilfs_get_segment(nilfs, segnums[i], &segment);
 		if (unlikely(ret < 0))
 			return -1;
diff --git a/sbin/cleanerd.c b/sbin/cleanerd.c
index 4d7a6da..f2d480d 100644
--- a/sbin/cleanerd.c
+++ b/sbin/cleanerd.c
@@ -563,9 +563,12 @@ static int nilfs_segments_still_reclaimable(struct nilfs *nilfs,
 	int i, ret;
 
 	for (i = 0; i < nsegs; i++) {
-		if (nilfs_get_suinfo(nilfs, segnumv[i], &si, 1) == 1 &&
-		    !nilfs_suinfo_reclaimable(&si))
-			continue;
+		if (nilfs_get_suinfo(nilfs, segnumv[i], &si, 1) == 1) {
+			if (!nilfs_suinfo_reclaimable(&si))
+				continue;
+			if (nilfs_suinfo_empty(&si))
+				return 1;  /* Found a scrapped segment */
+		}
 
 		ret = nilfs_segment_is_protected(nilfs, segnumv[i], protseq);
 		if (ret > 0)
diff --git a/sbin/nilfs-resize.c b/sbin/nilfs-resize.c
index 3f71a47..6d631d2 100644
--- a/sbin/nilfs-resize.c
+++ b/sbin/nilfs-resize.c
@@ -533,6 +533,13 @@ nilfs_resize_find_movable_segments(struct nilfs *nilfs, uint64_t start,
 			if (!nilfs_suinfo_reclaimable(&suinfo[i]))
 				continue;
 
+			if (nilfs_suinfo_empty(&suinfo[i])) {
+				/* Scrapped segments can be removed */
+				*snp++ = segnum;
+				rest--;
+				continue;
+			}
+
 			ret = nilfs_segment_is_protected(nilfs, segnum,
 							 sustat.ss_prot_seq);
 			if (unlikely(ret < 0)) {
@@ -778,6 +785,17 @@ static int nilfs_resize_verify_failure(struct nilfs *nilfs,
 		if (nilfs_get_suinfo(nilfs, segnumv[i], &si, 1) == 1) {
 			if (!nilfs_suinfo_reclaimable(&si))
 				reason |= NILFS_RESIZE_SEGMENT_UNRECLAIMABLE;
+			else if (nilfs_suinfo_empty(&si))
+				continue;  /* Scrapped segment */
+			if (nilfs_suinfo_active(&si)) {
+				/*
+				 * Active segments may not have been written
+				 * either, so we determine them to be protected
+				 * without reading the segment summary.
+				 */
+				reason |= NILFS_RESIZE_SEGMENT_PROTECTED;
+				continue;
+			}
 		}
 		ret = nilfs_segment_is_protected(nilfs, segnumv[i],
 						 sustat.ss_prot_seq);