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

@@ -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 */

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;

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)

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);