refactor reclaim function

This series adds nilfs_xreclaim_segment() api function to the GC library, and modifies cleneard so that it uses the extended API function.

nilfs_xreclaim_segment() is an enhanced version of the existing GC function (nilfs_reclaim_segment).

The nilfs_xreclaim_segment() works like nilfs_reclaim_segment() except the following differences:

- takes GC parameters in an expandable way.
(with nilfs_reclaim_params struct)

- can output statistics information of selected segments, for instance, the number live or defunct blocks, etc.

- takes a dry-run argument

- no longer returns the number of cleaned segments. The value will be stored in cleaned_segs member of nilfs_reclaim_stat struct.

This series is intended to give a framework for the forthcoming enhancement of garbage collection library and cleanerd daemon.

nilfs_reclaim_segment() function is still maintained for compatibility reason.

Hi Andreas,

Quoted 21 lines · Ryusuke Konishi

This series adds nilfs_xreclaim_segment() api function to the GC library, and modifies cleneard so that it uses the extended API function.

nilfs_xreclaim_segment() is an enhanced version of the existing GC function (nilfs_reclaim_segment).

The nilfs_xreclaim_segment() works like nilfs_reclaim_segment() except the following differences:

- takes GC parameters in an expandable way.
(with nilfs_reclaim_params struct)

- can output statistics information of selected segments, for instance, the number live or defunct blocks, etc.

- takes a dry-run argument

- no longer returns the number of cleaned segments. The value will be stored in cleaned_segs member of nilfs_reclaim_stat struct.

Quoted 5 lines

This series is intended to give a framework for the forthcoming enhancement of garbage collection library and cleanerd daemon.

nilfs_reclaim_segment() function is still maintained for compatibility reason.

Can you rebase your GC patchset on this?

With this option the user can specify the minimum number of reclaimable blocks of a segment, before it can be cleaned. This is a threshold for the GC to prevent needless moving of data. If there are less reclaimable blocks in a segment than the specified number, the GC will abort and try again with a different segment.

If there are less clean segments than min_clean_segments, mc_min_reclaimable_blocks is used instead of min_reclaimable_blocks. This allows for more flexibility in configuring the GC.

The number of blocks can be specified in percent of a segment or in bytes.

If the use_set_suinfo switch is not set, the optimization is completely disabled.

Signed-off-by Andreas Rohner <[email protected]>

---
 man/nilfs_cleanerd.conf.5         |  22 +++++
 sbin/cleanerd/cldconfig.c         | 168 ++++++++++++++++++++++++++++----------
 sbin/cleanerd/cldconfig.h         |   8 ++
 sbin/cleanerd/cleanerd.c          |   9 ++
 sbin/cleanerd/nilfs_cleanerd.conf |  13 +++
 5 files changed, 178 insertions(+), 42 deletions(-)

man/nilfs_cleanerd.conf.5 +22 -0

index 8f3fcd2..4315ecd 100644
@@ -79,6 +79,28 @@ Specify whether to use \fBmmap\fP(2) for reading segments.  At
 present, this option is enabled if supported regardless of this
 directive.
 .TP
+.B use_set_suinfo
+Specify whether to use the set_suinfo ioctl if it is supported. This is
+necessary for the \fBmin_reclaimable_blocks\fP feature. By disabling this
+switch \fBmin_reclaimable_blocks\fP is also disabled.
+.TP
+.B min_reclaimable_blocks
+Specify the minimum number of reclaimable blocks in a segment before
+it can be cleaned.
+.TP
+.B mc_min_reclaimable_blocks
+Specify the minimum number of reclaimable blocks in a segment before
+it can be cleaned. if clean segments < min_clean_segments.
+.PP
+\fBmin_reclaimable_blocks\fP and \fBmc_min_reclaimable_blocks\fP may
+be followed by a percent sign or the following multiplicative suffixes:
+kB 1000, K 1024, MB 1000*1000, M 1024*1024, GB 1000*1000*1000, G
+1024*1024*1024, and so on for T, P, E.  If the argument is followed by
+a percent sign, it represents the ratio of blocks in a segment.
+.PP
+The default values of \fBmin_reclaimable_blocks\fP and
+\fBmc_min_reclaimable_blocks\fP are 5 percent and 1 percent respectively.
+.TP
 .B log_priority
 Gives the verbosity level that is used when logging messages from
 \fBnilfs_cleanerd\fP(8).  The possible values are: \fBemerg\fP,

sbin/cleanerd/cldconfig.c +0 -0

index 270d360..b69f060 100644

sbin/cleanerd/cldconfig.c +125 -42

+	unsigned long long bytes = param->num;
+
+	switch (param->unit) {
+	case NILFS_SIZE_UNIT_KB:
+		bytes *= 1000ULL;
+		break;
+	case NILFS_SIZE_UNIT_KIB:
+		bytes <<= 10;
+		break;
+	case NILFS_SIZE_UNIT_MB:
+		bytes *= 1000000ULL;
+		break;
+	case NILFS_SIZE_UNIT_MIB:
+		bytes <<= 20;
+		break;
+	case NILFS_SIZE_UNIT_GB:
+		bytes *= 1000000000ULL;
+		break;
+	case NILFS_SIZE_UNIT_GIB:
+		bytes <<= 30;
+		break;
+	case NILFS_SIZE_UNIT_TB:
+		bytes *= 1000000000000ULL;
+		break;
+	case NILFS_SIZE_UNIT_TIB:
+		bytes <<= 40;
+		break;
+	case NILFS_SIZE_UNIT_PB:
+		bytes *= 1000000000000000ULL;
+		break;
+	case NILFS_SIZE_UNIT_PIB:
+		bytes <<= 50;
+		break;
+	case NILFS_SIZE_UNIT_EB:
+		bytes *= 1000000000000000000ULL;
+		break;
+	case NILFS_SIZE_UNIT_EIB:
+		bytes <<= 60;
+		break;
+	default:
+		assert(0);
+	}
+
+	return bytes;
+}
+
+static unsigned long long
 nilfs_convert_size_to_nsegments(struct nilfs *nilfs, struct nilfs_param *param)
 {
 	unsigned long long ret, segment_size, bytes;

@@ -287,48 +335,7 @@ nilfs_convert_size_to_nsegments(struct nilfs *nilfs, struct nilfs_param *param)
 	} else if (param->unit == NILFS_SIZE_UNIT_PERCENT) {
 		ret = (nilfs_get_nsegments(nilfs) * param->num + 99) / 100;
 	} else {
-		bytes = param->num;
-
-		switch (param->unit) {
-		case NILFS_SIZE_UNIT_KB:
-			bytes *= 1000ULL;
-			break;
-		case NILFS_SIZE_UNIT_KIB:
-			bytes <<= 10;
-			break;
-		case NILFS_SIZE_UNIT_MB:
-			bytes *= 1000000ULL;
-			break;
-		case NILFS_SIZE_UNIT_MIB:
-			bytes <<= 20;
-			break;
-		case NILFS_SIZE_UNIT_GB:
-			bytes *= 1000000000ULL;
-			break;
-		case NILFS_SIZE_UNIT_GIB:
-			bytes <<= 30;
-			break;
-		case NILFS_SIZE_UNIT_TB:
-			bytes *= 1000000000000ULL;
-			break;
-		case NILFS_SIZE_UNIT_TIB:
-			bytes <<= 40;
-			break;
-		case NILFS_SIZE_UNIT_PB:
-			bytes *= 1000000000000000ULL;
-			break;
-		case NILFS_SIZE_UNIT_PIB:
-			bytes <<= 50;
-			break;
-		case NILFS_SIZE_UNIT_EB:
-			bytes *= 1000000000000000000ULL;
-			break;
-		case NILFS_SIZE_UNIT_EIB:
-			bytes <<= 60;
-			break;
-		default:
-			assert(0);
-		}
+		bytes = nilfs_convert_units_to_bytes(param);
 		segment_size = nilfs_get_block_size(nilfs) *
 			nilfs_get_blocks_per_segment(nilfs);
 		ret = (bytes + segment_size - 1) / segment_size;

@@ -455,6 +462,52 @@ nilfs_cldconfig_handle_mc_nsegments_per_clean(struct nilfs_cldconfig *config,
 	return 0;
 }
 
+static unsigned long long
+nilfs_convert_size_to_blocks_per_segment(struct nilfs *nilfs,
+					 struct nilfs_param *param)
+{
+	unsigned long long ret, segment_size, block_size, bytes;
+
+	if (param->unit == NILFS_SIZE_UNIT_NONE) {
+		ret = param->num;
+	} else if (param->unit == NILFS_SIZE_UNIT_PERCENT) {
+		ret = (nilfs_get_blocks_per_segment(nilfs) * param->num) / 100;
+	} else {
+		block_size = nilfs_get_block_size(nilfs);
+		segment_size = block_size *
+				nilfs_get_blocks_per_segment(nilfs);
+		bytes = nilfs_convert_units_to_bytes(param) % segment_size;
+		ret = (bytes + block_size - 1) / block_size;
+	}
+	return ret;
+}
+
+static int
+nilfs_cldconfig_handle_min_reclaimable_blocks(struct nilfs_cldconfig *config,
+					      char **tokens, size_t ntoks,
+					      struct nilfs *nilfs)
+{
+	struct nilfs_param param;
+
+	if (nilfs_cldconfig_get_size_argument(tokens, ntoks, &param) == 0)
+		config->cf_min_reclaimable_blocks =
+			nilfs_convert_size_to_blocks_per_segment(nilfs, &param);
+	return 0;
+}
+
+static int
+nilfs_cldconfig_handle_mc_min_reclaimable_blocks(struct nilfs_cldconfig *config,
+						 char **tokens, size_t ntoks,
+						 struct nilfs *nilfs)
+{
+	struct nilfs_param param;
+
+	if (nilfs_cldconfig_get_size_argument(tokens, ntoks, &param) == 0)
+		config->cf_mc_min_reclaimable_blocks =
+			nilfs_convert_size_to_blocks_per_segment(nilfs, &param);
+	return 0;
+}
+
 static int
 nilfs_cldconfig_handle_cleaning_interval(struct nilfs_cldconfig *config,
 					 char **tokens, size_t ntoks,

@@ -490,6 +543,14 @@ static int nilfs_cldconfig_handle_use_mmap(struct nilfs_cldconfig *config,
 	return 0;
 }
 
+static int nilfs_cldconfig_handle_use_set_suinfo(struct nilfs_cldconfig *config,
+						 char **tokens, size_t ntoks,
+						 struct nilfs *nilfs)
+{
+	config->cf_use_set_suinfo = 1;
+	return 0;
+}
+
 static const struct nilfs_cldconfig_log_priority
 nilfs_cldconfig_log_priority_table[] = {
 	{"emerg",	LOG_EMERG},

@@ -576,6 +637,18 @@ nilfs_cldconfig_keyword_table[] = {
 		"log_priority", 2, 2,
 		nilfs_cldconfig_handle_log_priority
 	},
+	{
+		"min_reclaimable_blocks", 2, 2,
+		nilfs_cldconfig_handle_min_reclaimable_blocks
+	},
+	{
+		"mc_min_reclaimable_blocks", 2, 2,
+		nilfs_cldconfig_handle_mc_min_reclaimable_blocks
+	},
+	{
+		"use_set_suinfo", 1, 1,
+		nilfs_cldconfig_handle_use_set_suinfo
+	},
 };
 
 #define NILFS_CLDCONFIG_NKEYWORDS			\

@@ -640,7 +713,18 @@ static void nilfs_cldconfig_set_default(struct nilfs_cldconfig *config,
 	config->cf_retry_interval.tv_sec = NILFS_CLDCONFIG_RETRY_INTERVAL;
 	config->cf_retry_interval.tv_usec = 0;
 	config->cf_use_mmap = NILFS_CLDCONFIG_USE_MMAP;
+	config->cf_use_set_suinfo = NILFS_CLDCONFIG_USE_SET_SUINFO;
 	config->cf_log_priority = NILFS_CLDCONFIG_LOG_PRIORITY;
+
+	param.num = NILFS_CLDCONFIG_MIN_RECLAIMABLE_BLOCKS;
+	param.unit = NILFS_CLDCONFIG_MIN_RECLAIMABLE_BLOCKS_UNIT;
+	config->cf_min_reclaimable_blocks =
+		nilfs_convert_size_to_blocks_per_segment(nilfs, &param);
+
+	param.num = NILFS_CLDCONFIG_MC_MIN_RECLAIMABLE_BLOCKS;
+	param.unit = NILFS_CLDCONFIG_MC_MIN_RECLAIMABLE_BLOCKS_UNIT;
+	config->cf_mc_min_reclaimable_blocks =
+		nilfs_convert_size_to_blocks_per_segment(nilfs, &param);
 }
 
 static inline int iseol(int c)

sbin/cleanerd/cldconfig.h +0 -0

index 188ce9b..e7ac72e 100644

sbin/cleanerd/cldconfig.h +7 -0

 	int cf_log_priority;
+	unsigned long cf_min_reclaimable_blocks;
+	unsigned long cf_mc_min_reclaimable_blocks;
 };
 
 #define NILFS_CLDCONFIG_SELECTION_POLICY_IMPORTANCE	\

@@ -119,7 +122,12 @@ struct nilfs_cldconfig {
 #define NILFS_CLDCONFIG_MC_CLEANING_INTERVAL		1
 #define NILFS_CLDCONFIG_RETRY_INTERVAL			60
 #define NILFS_CLDCONFIG_USE_MMAP			1
+#define NILFS_CLDCONFIG_USE_SET_SUINFO			0
 #define NILFS_CLDCONFIG_LOG_PRIORITY			LOG_INFO
+#define NILFS_CLDCONFIG_MIN_RECLAIMABLE_BLOCKS		5
+#define NILFS_CLDCONFIG_MIN_RECLAIMABLE_BLOCKS_UNIT	NILFS_SIZE_UNIT_PERCENT
+#define NILFS_CLDCONFIG_MC_MIN_RECLAIMABLE_BLOCKS	1
+#define NILFS_CLDCONFIG_MC_MIN_RECLAIMABLE_BLOCKS_UNIT	NILFS_SIZE_UNIT_PERCENT
 
 #define NILFS_CLDCONFIG_NSEGMENTS_PER_CLEAN_MAX	32
 

sbin/cleanerd/cleanerd.c +0 -0

index 1d09b5b..b441448 100644

sbin/cleanerd/cleanerd.c +8 -0

 	__u64 prev_nongc_ctime;
 	mqd_t recvq;
 	char *recvq_name;

@@ -273,6 +274,8 @@ static int nilfs_cleanerd_reconfig(struct nilfs_cleanerd *cleanerd,
 	} else {
 		cleanerd->ncleansegs = config->cf_nsegments_per_clean;
 		cleanerd->cleaning_interval = config->cf_cleaning_interval;
+		cleanerd->min_reclaimable_blocks =
+				config->cf_min_reclaimable_blocks;
 		syslog(LOG_INFO, "configuration file reloaded");
 	}
 	return ret;

@@ -1238,10 +1241,14 @@ static int nilfs_cleanerd_handle_clean_check(struct nilfs_cleanerd *cleanerd,
 		/* disk space is close to limit -- accelerate cleaning */
 		cleanerd->ncleansegs = config->cf_mc_nsegments_per_clean;
 		cleanerd->cleaning_interval = config->cf_mc_cleaning_interval;
+		cleanerd->min_reclaimable_blocks =
+				config->cf_mc_min_reclaimable_blocks;
 	} else {
 		/* continue to run */
 		cleanerd->ncleansegs = config->cf_nsegments_per_clean;
 		cleanerd->cleaning_interval = config->cf_cleaning_interval;
+		cleanerd->min_reclaimable_blocks =
+				config->cf_min_reclaimable_blocks;
 	}
 
 	return 0; /* do gc */

@@ -1442,6 +1449,8 @@ static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
 
 	cleanerd->ncleansegs = cleanerd->config.cf_nsegments_per_clean;
 	cleanerd->cleaning_interval = cleanerd->config.cf_cleaning_interval;
+	cleanerd->min_reclaimable_blocks =
+			cleanerd->config.cf_min_reclaimable_blocks;
 
 
 	if (nilfs_cleanerd_automatic_suspend(cleanerd))

sbin/cleanerd/nilfs_cleanerd.conf +0 -0

index 26872aa..f88a57a 100644
@@ -51,6 +51,19 @@ mc_cleaning_interval	1
 # Retry interval in seconds.

sbin/cleanerd/nilfs_cleanerd.conf +1 -0

+
 # Use mmap when reading segments if supported.
 use_mmap
 

Signature · 6 lines

1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

This option for the nilfs object corresponds to the use_set_suinfo flag in the configuration file. This patch adds the flag itself plus some convenience functions.

The option is set after the configuration file was read in.

Signed-off-by Andreas Rohner <[email protected]>

---
 include/nilfs.h          |  7 ++++++-
 lib/nilfs.c              | 28 ++++++++++++++++++++++++++++
 sbin/cleanerd/cleanerd.c |  6 ++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

include/nilfs.h +6 -1

index 56286a9..b5f85d3 100644
@@ -128,7 +128,8 @@ struct nilfs {
 #define NILFS_OPEN_RDWR		0x0008	/* Open NILFS API in read/write mode */
 #define NILFS_OPEN_GCLK		0x1000	/* Open GC lock primitive */
 
-#define NILFS_OPT_MMAP	0x01
+#define NILFS_OPT_MMAP		0x01
+#define NILFS_OPT_SET_SUINFO	0x02
 
 
 struct nilfs *nilfs_open(const char *, const char *, int);

@@ -140,6 +141,10 @@ void nilfs_opt_clear_mmap(struct nilfs *);
 int nilfs_opt_set_mmap(struct nilfs *);
 int nilfs_opt_test_mmap(struct nilfs *);
 
+void nilfs_opt_clear_set_suinfo(struct nilfs *);
+int nilfs_opt_set_set_suinfo(struct nilfs *);
+int nilfs_opt_test_set_suinfo(struct nilfs *);
+
 nilfs_cno_t nilfs_get_oldest_cno(struct nilfs *);
 
 struct nilfs_super_block *nilfs_get_sb(struct nilfs *);

lib/nilfs.c +28 -0

index c1771e8..93822de 100644
@@ -288,6 +288,34 @@ int nilfs_opt_test_mmap(struct nilfs *nilfs)
 	return !!(nilfs->n_opts & NILFS_OPT_MMAP);
 }
 
+/**
+ * nilfs_opt_set_set_suinfo - set set_suinfo option
+ * @nilfs: nilfs object
+ */
+int nilfs_opt_set_set_suinfo(struct nilfs *nilfs)
+{
+	nilfs->n_opts |= NILFS_OPT_SET_SUINFO;
+	return 0;
+}
+
+/**
+ * nilfs_opt_clear_set_suinfo - clear set_suinfo option
+ * @nilfs: nilfs object
+ */
+void nilfs_opt_clear_set_suinfo(struct nilfs *nilfs)
+{
+	nilfs->n_opts &= ~NILFS_OPT_SET_SUINFO;
+}
+
+/**
+ * nilfs_opt_test_set_suinfo - test whether set_suinfo option is set or not
+ * @nilfs: nilfs object
+ */
+int nilfs_opt_test_set_suinfo(struct nilfs *nilfs)
+{
+	return !!(nilfs->n_opts & NILFS_OPT_SET_SUINFO);
+}
+
 static int nilfs_open_sem(struct nilfs *nilfs)
 {
 	char semnambuf[NAME_MAX - 4];

sbin/cleanerd/cleanerd.c +6 -0

index b441448..970816f 100644
@@ -247,6 +247,12 @@ static int nilfs_cleanerd_config(struct nilfs_cleanerd *cleanerd,
 	else
 		nilfs_opt_clear_mmap(cleanerd->nilfs);
 #endif	/* HAVE_MMAP */
+
+	if (cleanerd->config.cf_use_set_suinfo)
+		nilfs_opt_set_set_suinfo(cleanerd->nilfs);
+	else
+		nilfs_opt_clear_set_suinfo(cleanerd->nilfs);
+
 	nilfs_cleanerd_set_log_priority(cleanerd);
 
 	if (protection_period != ULONG_MAX) {

Signature · 6 lines

1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

This patch adds a command line parameter for nilfs-clean to allow the manual override of the min_reclaimable_blocks option.

Signed-off-by Andreas Rohner <[email protected]>

---
 include/nilfs_cleaner.h        | 19 +++++++++---------
 man/nilfs-clean.8              |  4 ++++
 sbin/cleanerd/cleanerd.c       | 19 ++++++++++++++++++
 sbin/nilfs-clean/nilfs-clean.c | 44 ++++++++++++++++++++++++++++++++++++------
 4 files changed, 71 insertions(+), 15 deletions(-)

include/nilfs_cleaner.h +10 -9

index 0bf02aa..874c17a 100644
@@ -46,17 +46,18 @@ struct nilfs_cleaner_args {
 	uint64_t start_segnum;	/* start segment number */
 	uint64_t nsegs;		/* number of segments */
 	uint32_t runtime; /* runtime in seconds */
-	uint32_t pad2;
+	uint32_t min_reclaimable_blocks;
 };
 /* valid flags */
-#define NILFS_CLEANER_ARG_PROTECTION_PERIOD	(1 << 0)
-#define NILFS_CLEANER_ARG_NSEGMENTS_PER_CLEAN	(1 << 1)
-#define NILFS_CLEANER_ARG_CLEANING_INTERVAL	(1 << 2)
-#define NILFS_CLEANER_ARG_USAGE_RATE_THRESHOLD	(1 << 3) /* reserved */
-#define NILFS_CLEANER_ARG_START_SEGNUM		(1 << 4) /* reserved */
-#define NILFS_CLEANER_ARG_NSEGS			(1 << 5) /* reserved */
-#define NILFS_CLEANER_ARG_NPASSES		(1 << 6) /* reserved */
-#define NILFS_CLEANER_ARG_RUNTIME		(1 << 7) /* reserved */
+#define NILFS_CLEANER_ARG_PROTECTION_PERIOD		(1 << 0)
+#define NILFS_CLEANER_ARG_NSEGMENTS_PER_CLEAN		(1 << 1)
+#define NILFS_CLEANER_ARG_CLEANING_INTERVAL		(1 << 2)
+#define NILFS_CLEANER_ARG_USAGE_RATE_THRESHOLD		(1 << 3) /* reserved */
+#define NILFS_CLEANER_ARG_START_SEGNUM			(1 << 4) /* reserved */
+#define NILFS_CLEANER_ARG_NSEGS				(1 << 5) /* reserved */
+#define NILFS_CLEANER_ARG_NPASSES			(1 << 6) /* reserved */
+#define NILFS_CLEANER_ARG_RUNTIME			(1 << 7) /* reserved */
+#define NILFS_CLEANER_ARG_MIN_RECLAIMABLE_BLOCKS	(1 << 8)
 
 enum {
 	NILFS_CLEANER_STATUS_IDLE,

man/nilfs-clean.8 +4 -0

index 04e11c6..94267e1 100644
@@ -43,6 +43,10 @@ Display cleaner status.
 \fB\-h\fR, \fB\-\-help\fR
 Display help message and exit.
 .TP
+\fB\-m\fR, \fB\-\-min\-reclaimable\-blocks=\fICOUNT\fR
+Specify the minimum number of reclaimable blocks in a segment before
+it can be cleaned.
+.TP
 \fB\-p\fR, \fB\-\-protection-period=\fIinterval\fR
 Set protection period for a cleaner run.  The \fIinterval\fR parameter
 is an integer value and specifies the minimum time that deleted or

sbin/cleanerd/cleanerd.c +0 -0

index 970816f..a3f62b0 100644

sbin/cleanerd/cleanerd.c +8 -0

 };
 
 /**

@@ -468,6 +469,14 @@ nilfs_cleanerd_protection_period(struct nilfs_cleanerd *cleanerd)
 		&cleanerd->config.cf_protection_period;
 }
 
+static unsigned long
+nilfs_cleanerd_min_reclaimable_blocks(struct nilfs_cleanerd *cleanerd)
+{
+	return cleanerd->running == 2 ?
+		cleanerd->mm_min_reclaimable_blocks :
+		cleanerd->min_reclaimable_blocks;
+}
+
 static void
 nilfs_cleanerd_reduce_ncleansegs_for_retry(struct nilfs_cleanerd *cleanerd)
 {

@@ -1010,6 +1019,16 @@ static int nilfs_cleanerd_cmd_run(struct nilfs_cleanerd *cleanerd,
 		cleanerd->mm_cleaning_interval =
 			cleanerd->cleaning_interval;
 	}

sbin/cleanerd/cleanerd.c +0 -0

 	/* number of passes */
 	if (req2->args.valid & NILFS_CLEANER_ARG_NPASSES) {
 		if (!req2->args.npasses)

sbin/nilfs-clean/nilfs-clean.c +38 -6

index 55abd55..f77fdf7 100644
@@ -77,6 +77,7 @@ const static struct option long_option[] = {
 	{"stop", no_argument, NULL, 'b'},
 	{"suspend", no_argument, NULL, 's'},
 	{"speed", required_argument, NULL, 'S'},
+	{"min-reclaimable-blocks", required_argument, NULL, 'm'},
 	{"verbose", no_argument, NULL, 'v'},
 	{"version", no_argument, NULL, 'V'},
 	{NULL, 0, NULL, 0}

@@ -90,6 +91,9 @@ const static struct option long_option[] = {
 	"  -l, --status\t\tdisplay cleaner status\n"			\
 	"  -p, --protection-period=SECONDS\n"				\
 	"               \t\tspecify protection period\n"		\
+	"  -m, --min-reclaimable-blocks=COUNT\n"			\
+	"               \t\tset minimum number of reclaimable blocks\n"	\
+	"               \t\tbefore a segment can be cleaned\n"		\
 	"  -q, --quit\t\tshutdown cleaner\n"				\
 	"  -r, --resume\t\tresume cleaner\n"				\
 	"  -s, --suspend\t\tsuspend cleaner\n"				\

@@ -98,9 +102,10 @@ const static struct option long_option[] = {
 	"  -v, --verbose\t\tverbose mode\n"				\
 	"  -V, --version\t\tdisplay version and exit\n"
 #else
-#define NILFS_CLEAN_USAGE						\
-	"Usage: %s [-b] [-c [conffile]] [-h] [-l] [-p protection-period]" \
-	"          [-q] [-r] [-s] [-S gc-speed] [-v] [-V] [device]\n"
+#define NILFS_CLEAN_USAGE						  \
+	"Usage: %s [-b] [-c [conffile]] [-h] [-l] [-m blocks]\n"	  \
+	"          [-p protection-period] [-q] [-r] [-s] [-S gc-speed]\n" \
+	"          [-v] [-V] [device]\n"
 #endif	/* _GNU_SOURCE */
 
 

@@ -124,6 +129,7 @@ static const char *conffile = NULL;
 static unsigned long protection_period = ULONG_MAX;
 static int nsegments_per_clean = 2;
 static struct timespec cleaning_interval = { 0, 100000000 };   /* 100 msec */
+static unsigned long min_reclaimable_blocks = 20;  /* about 1% with 8MB segs */
 
 static sigjmp_buf nilfs_clean_env;
 static struct nilfs_cleaner *nilfs_cleaner;

@@ -164,9 +170,11 @@ static int nilfs_clean_do_run(struct nilfs_cleaner *cleaner)
 	args.nsegments_per_clean = nsegments_per_clean;
 	args.cleaning_interval = cleaning_interval.tv_sec;
 	args.cleaning_interval_nsec = cleaning_interval.tv_nsec;
+	args.min_reclaimable_blocks = min_reclaimable_blocks;
 	args.valid = (NILFS_CLEANER_ARG_NPASSES |
 		      NILFS_CLEANER_ARG_CLEANING_INTERVAL |
-		      NILFS_CLEANER_ARG_NSEGMENTS_PER_CLEAN);
+		      NILFS_CLEANER_ARG_NSEGMENTS_PER_CLEAN |
+		      NILFS_CLEANER_ARG_MIN_RECLAIMABLE_BLOCKS);
 
 	if (protection_period != ULONG_MAX) {
 		args.protection_period = protection_period;

@@ -426,6 +434,26 @@ failed_too_large:
 	return -1;
 }
 
+static int nilfs_clean_parse_min_reclaimable(const char *arg)
+{
+	unsigned long blocks;
+	char *endptr;
+
+	blocks = strtoul(arg, &endptr, 10);
+	if (endptr == arg || endptr[0] != '\0') {
+		myprintf(_("Error: invalid reclaimable blocks: %s\n"), arg);
+		return -1;
+	}
+
+	if (blocks == ULONG_MAX) {
+		myprintf(_("Error: value too large: %s\n"), arg);
+		return -1;
+	}
+
+	min_reclaimable_blocks = blocks;
+	return 0;
+}
+
 static void nilfs_clean_parse_options(int argc, char *argv[])
 {
 #ifdef _GNU_SOURCE

@@ -434,10 +462,10 @@ static void nilfs_clean_parse_options(int argc, char *argv[])
 	int c;
 
 #ifdef _GNU_SOURCE
-	while ((c = getopt_long(argc, argv, "bc::hlp:qrsS:vV",
+	while ((c = getopt_long(argc, argv, "bc::hlm:p:qrsS:vV",
 				long_option, &option_index)) >= 0) {
 #else
-	while ((c = getopt(argc, argv, "bc::hlp:qrsS:vV")) >= 0) {
+	while ((c = getopt(argc, argv, "bc::hlm:p:qrsS:vV")) >= 0) {
 #endif	/* _GNU_SOURCE */
 		switch (c) {
 		case 'b':

@@ -455,6 +483,10 @@ static void nilfs_clean_parse_options(int argc, char *argv[])
 		case 'l':
 			clean_cmd = NILFS_CLEAN_CMD_INFO;
 			break;
+		case 'm':
+			if (nilfs_clean_parse_min_reclaimable(optarg) < 0)
+				exit(EXIT_FAILURE);
+			break;
 		case 'p':
 			if (nilfs_clean_parse_protection_period(optarg) < 0)
 				exit(EXIT_FAILURE);

Signature · 6 lines

1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

This patch adds library support for the NILFS_IOCTL_SET_SUINFO ioctl. A new function nilfs_set_suinfo and the structure nilfs_suinfo_update, which contains the update information, are added.

With this function the segment usage information can be updated from userspace, which can be used as a shortcut for certain GC operations.

Signed-off-by Andreas Rohner <[email protected]>

---
 include/nilfs.h     |  2 ++
 include/nilfs2_fs.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 lib/nilfs.c         | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)

include/nilfs.h +2 -0

index b5f85d3..da18e24 100644
@@ -304,6 +304,8 @@ int nilfs_delete_checkpoint(struct nilfs *, nilfs_cno_t);
 int nilfs_get_cpstat(const struct nilfs *, struct nilfs_cpstat *);
 ssize_t nilfs_get_suinfo(const struct nilfs *, __u64, struct nilfs_suinfo *,
 			 size_t);
+ssize_t nilfs_set_suinfo(struct nilfs *, struct nilfs_suinfo_update *,
+			 size_t);
 int nilfs_get_sustat(const struct nilfs *, struct nilfs_sustat *);
 ssize_t nilfs_get_vinfo(const struct nilfs *, struct nilfs_vinfo *, size_t);
 ssize_t nilfs_get_bdescs(const struct nilfs *, struct nilfs_bdesc *, size_t);

include/nilfs2_fs.h +43 -0

index e674f44..cf8f633 100644
@@ -714,6 +714,47 @@ static inline int nilfs_suinfo_clean(const struct nilfs_suinfo *si)
 }
 
 /* ioctl */
+/**
+ * nilfs_suinfo_update - segment usage information update
+ * @sup_segnum: segment number
+ * @sup_flags: flags for which fields are active in sup_sui
+ * @sup_reserved: reserved necessary for alignment
+ * @sup_sui: segment usage information
+ */
+struct nilfs_suinfo_update {
+	__u64 sup_segnum;
+	__u32 sup_flags;
+	__u32 sup_reserved;
+	struct nilfs_suinfo sup_sui;
+};
+
+enum {
+	NILFS_SUINFO_UPDATE_LASTMOD,
+	NILFS_SUINFO_UPDATE_NBLOCKS,
+	NILFS_SUINFO_UPDATE_FLAGS,
+};
+
+#define NILFS_SUINFO_UPDATE_FNS(flag, name)				\
+static inline void							\
+nilfs_suinfo_update_set_##name(struct nilfs_suinfo_update *sup)		\
+{									\
+	sup->sup_flags |= 1UL << NILFS_SUINFO_UPDATE_##flag;		\
+}									\
+static inline void							\
+nilfs_suinfo_update_clear_##name(struct nilfs_suinfo_update *sup)	\
+{									\
+	sup->sup_flags &= ~(1UL << NILFS_SUINFO_UPDATE_##flag);		\
+}									\
+static inline int							\
+nilfs_suinfo_update_##name(const struct nilfs_suinfo_update *sup)	\
+{									\
+	return !!(sup->sup_flags & (1UL << NILFS_SUINFO_UPDATE_##flag));\
+}
+
+NILFS_SUINFO_UPDATE_FNS(LASTMOD, lastmod)
+NILFS_SUINFO_UPDATE_FNS(NBLOCKS, nblocks)
+NILFS_SUINFO_UPDATE_FNS(FLAGS, flags)
+
 enum {
 	NILFS_CHECKPOINT,
 	NILFS_SNAPSHOT,

@@ -867,5 +908,7 @@ struct nilfs_bdesc {
 	_IOW(NILFS_IOCTL_IDENT, 0x8B, __u64)
 #define NILFS_IOCTL_SET_ALLOC_RANGE  \
 	_IOW(NILFS_IOCTL_IDENT, 0x8C, __u64[2])
+#define NILFS_IOCTL_SET_SUINFO  \
+	_IOW(NILFS_IOCTL_IDENT, 0x8D, struct nilfs_argv)
 
 #endif	/* _LINUX_NILFS_FS_H */

lib/nilfs.c +3 -0

index 93822de..e6f7c1e 100644
@@ -596,6 +596,41 @@ ssize_t nilfs_get_suinfo(const struct nilfs *nilfs, __u64 segnum,
 }
 
 /**
+ * nilfs_set_suinfo - sets segment usage info
+ * @nilfs: nilfs object
+ * @sup: an array of nilfs_suinfo_update structs

lib/nilfs.c +6 -0

+{
+	struct nilfs_argv argv;
+
+	if (nilfs->n_iocfd < 0) {
+		errno = EBADF;
+		return -1;

lib/nilfs.c +6 -0

+		return -1;
+	}
+	return 0;
+}
+
+/**
  * nilfs_get_sustat - get segment usage statistics
  * @nilfs: nilfs object
  * @sustat: buffer of a nilfs_sustat struct to store statistics in

Signature · 6 lines

1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 12 lines

/**
+ * nilfs_set_suinfo - sets segment usage info
+ * @nilfs: nilfs object
+ * @sup: an array of nilfs_suinfo_update structs
+ * @nsup: number of elements in sup
+ *
+ * Description: Takes an array of nilfs_suinfo_update structs and updates
+ * segment usage info accordingly. Only the fields indicated by sup_flags
+ * are updated.
+ *
+ * Return Value: On success, 0 is returned. On error, -1 is returned.
+ */

lib/nilfs.c ssize_t nilfs_get_suinfo(const struct nilfs *nilfs, __u64 segnum, · line 611

Quoted 2 lines

lib/nilfs.c +2 -0

+ssize_t nilfs_set_suinfo(struct nilfs *nilfs,
+			 struct nilfs_suinfo_update *sup, size_t nsup)

The type of return value of this function should be "int" because it doesn't return the count of obtained suinfo items.

Quoted 6 lines

lib/nilfs.c +6 -0

+{
+	struct nilfs_argv argv;
+
+	if (nilfs->n_iocfd < 0) {
+		errno = EBADF;
+		return -1;

lib/nilfs.c ssize_t nilfs_get_suinfo(const struct nilfs *nilfs, __u64 segnum, · line 627

Quoted 10 lines

lib/nilfs.c +10 -0

+	}
+
+	argv.v_base = (unsigned long)sup;
+	argv.v_nmembs = nsup;
+	argv.v_size = sizeof(struct nilfs_suinfo_update);
+	argv.v_index = 0;
+	argv.v_flags = 0;
+	if (ioctl(nilfs->n_iocfd, NILFS_IOCTL_SET_SUINFO, &argv) < 0) {
+		if (errno == ENOTTY)
+			nilfs_opt_clear_set_suinfo(nilfs);

This invalidation should be done in utils or upper library. Simply return here as follows:

     return ioctl(nilfs->n_iocfd, NILFS_IOCTL_SET_SUINFO, &argv);
}

Hi Ryusuke,

Quoted 3 lines · Ryusuke Konishi

Quoted 3 lines

/**
+ * nilfs_set_suinfo - sets segment usage info
+ * @nilfs: nilfs object

Quoted 4 lines

This invalidation should be done in utils or upper library. Simply return here as follows:

Quoted 2 lines

     return ioctl(nilfs->n_iocfd, NILFS_IOCTL_SET_SUINFO, &argv);
}

Thanks for your review. I have fixed everything you pointed out. Sorry for all the missing stuff. I will wait with sending in a new version of the patch set, until you finished your review. I suspect you will also find some things in patch 5, since it is the crucial part of the implementation.

This patch adds an additional parameter min_reclaimable_blks to the nilfs_reclaim_params structure. This parameter specifies the minimum number of reclaimable blocks in a segment, before it can be cleaned. If a segment is below this threshold, it is considered to be not worth cleaning, because all the live blocks would need to be moved to a new segment, which is expensive, and the number of reclaimable blocks is too low. But it is still necessary to update the segment usage information to turn the old segment into a new one.

This is basically a shortcut to cleaning the segment. It is still necessary to read the segment summary information, but the writing of the live blocks can be skipped if it's not worth it.

This optimization can be enabled and disabled by the use_set_suinfo switch in the configuration file. If the kernel doesn't support the set_suinfo ioctl it returns the error number ENOTTY, which also permanently disables the optimization.

Signed-off-by Andreas Rohner <[email protected]>

---
 include/nilfs_gc.h       |  7 +++---
 lib/gc.c                 | 63 +++++++++++++++++++++++++++++++++++++++++++++---
 sbin/cleanerd/cleanerd.c | 18 +++++++++++---
 3 files changed, 77 insertions(+), 11 deletions(-)

include/nilfs_gc.h +4 -3

index e49cbf4..1196c55 100644
@@ -17,18 +17,19 @@
 /* flags for nilfs_reclaim_params struct */
 #define NILFS_RECLAIM_PARAM_PROTSEQ	(1UL << 0)
 #define NILFS_RECLAIM_PARAM_PROTCNO	(1UL << 1)
-#define __NR_NILFS_RECLAIM_PARAMS	2
+#define NILFS_RECLAIM_PARAM_DEFERRABLE	(1UL << 2)
+#define __NR_NILFS_RECLAIM_PARAMS	3
 
 /**
  * struct nilfs_reclaim_params - structure to specify GC parameters
  * @flags: flags of valid fields
- * @reserved: padding bytes
+ * @min_reclaimable_blks: minimum number of reclaimable blocks
  * @protseq: start of sequence number of protected segments
  * @protcno: start number of checkpoint to be protected
  */
 struct nilfs_reclaim_params {
 	unsigned long flags;
-	unsigned long reserved;
+	unsigned long min_reclaimable_blks;
 	__u64 protseq;
 	nilfs_cno_t protcno;
 };

lib/gc.c +20 -3

index 71c7307..4aa6a2c 100644
@@ -29,6 +29,10 @@
 #include <syslog.h>
 #endif	/* HAVE_SYSLOG_H */
 
+#if HAVE_SYS_TIME_H
+#include <sys/time.h>
+#endif	/* HAVE_SYS_TIME */
+
 #include <errno.h>
 #include <assert.h>
 #include <stdarg.h>

@@ -617,8 +621,11 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs,
 	struct nilfs_vector *vdescv, *bdescv, *periodv, *vblocknrv;
 	sigset_t sigset, oldset, waitset;
 	nilfs_cno_t protcno;
-	ssize_t n, ret = -1;
+	ssize_t n, i, ret = -1;
 	size_t nblocks;
+	__u32 reclaimable_blocks;
+	struct nilfs_suinfo_update *supv;
+	struct timeval tv;
 
 	if (!(params->flags & NILFS_RECLAIM_PARAM_PROTSEQ) ||
 	    (params->flags & (~0UL << __NR_NILFS_RECLAIM_PARAMS))) {

@@ -703,13 +710,16 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs,
 	if (ret < 0)
 		goto out_lock;
 
+	reclaimable_blocks = (nilfs_get_blocks_per_segment(nilfs) * n) -
+			(nilfs_vector_get_size(vdescv) +
+			nilfs_vector_get_size(bdescv));
+
 	if (stat) {
 		stat->live_pblks = nilfs_vector_get_size(bdescv);
 		stat->defunct_pblks = nblocks - stat->live_pblks;
 
 		stat->live_blks = stat->live_vblks + stat->live_pblks;
-		stat->defunct_blks = n * nilfs_get_blocks_per_segment(nilfs) -
-			stat->live_blks;
+		stat->defunct_blks = reclaimable_blocks;
 	}
 	if (dryrun)
 		goto out_lock;

@@ -725,6 +735,51 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs,
 		goto out_lock;
 	}
 
+	/* if there are less reclaimable blocks than the
+	 * minimal threshold try to update suinfo
+	 * instead of cleaning */
+	if ((params->flags & NILFS_RECLAIM_PARAM_DEFERRABLE) &&
+			reclaimable_blocks < params->min_reclaimable_blks * n) {
+		if (stat)
+			stat->deferred_segs = n;

Hi Ryusuke,

Quoted 118 lines · Andreas Rohner

This patch adds an additional parameter min_reclaimable_blks to the nilfs_reclaim_params structure. This parameter specifies the minimum number of reclaimable blocks in a segment, before it can be cleaned. If a segment is below this threshold, it is considered to be not worth cleaning, because all the live blocks would need to be moved to a new segment, which is expensive, and the number of reclaimable blocks is too low. But it is still necessary to update the segment usage information to turn the old segment into a new one.

This is basically a shortcut to cleaning the segment. It is still necessary to read the segment summary information, but the writing of the live blocks can be skipped if it's not worth it.

This optimization can be enabled and disabled by the use_set_suinfo switch in the configuration file. If the kernel doesn't support the set_suinfo ioctl it returns the error number ENOTTY, which also permanently disables the optimization.

Signed-off-by Andreas Rohner <[email protected]>

---
 include/nilfs_gc.h       |  7 +++---
 lib/gc.c                 | 63 +++++++++++++++++++++++++++++++++++++++++++++---
 sbin/cleanerd/cleanerd.c | 18 +++++++++++---
 3 files changed, 77 insertions(+), 11 deletions(-)

include/nilfs_gc.h +4 -3

index e49cbf4..1196c55 100644
@@ -17,18 +17,19 @@
 /* flags for nilfs_reclaim_params struct */
 #define NILFS_RECLAIM_PARAM_PROTSEQ	(1UL << 0)
 #define NILFS_RECLAIM_PARAM_PROTCNO	(1UL << 1)
-#define __NR_NILFS_RECLAIM_PARAMS	2
+#define NILFS_RECLAIM_PARAM_DEFERRABLE	(1UL << 2)
+#define __NR_NILFS_RECLAIM_PARAMS	3
 
 /**
  * struct nilfs_reclaim_params - structure to specify GC parameters
  * @flags: flags of valid fields
- * @reserved: padding bytes
+ * @min_reclaimable_blks: minimum number of reclaimable blocks
  * @protseq: start of sequence number of protected segments
  * @protcno: start number of checkpoint to be protected
  */
 struct nilfs_reclaim_params {
 	unsigned long flags;
-	unsigned long reserved;
+	unsigned long min_reclaimable_blks;
 	__u64 protseq;
 	nilfs_cno_t protcno;
 };

lib/gc.c +27 -3

index 71c7307..4aa6a2c 100644
@@ -29,6 +29,10 @@
 #include <syslog.h>
 #endif	/* HAVE_SYSLOG_H */
 
+#if HAVE_SYS_TIME_H
+#include <sys/time.h>
+#endif	/* HAVE_SYS_TIME */
+
 #include <errno.h>
 #include <assert.h>
 #include <stdarg.h>

@@ -617,8 +621,11 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs,
 	struct nilfs_vector *vdescv, *bdescv, *periodv, *vblocknrv;
 	sigset_t sigset, oldset, waitset;
 	nilfs_cno_t protcno;
-	ssize_t n, ret = -1;
+	ssize_t n, i, ret = -1;
 	size_t nblocks;
+	__u32 reclaimable_blocks;
+	struct nilfs_suinfo_update *supv;
+	struct timeval tv;
 
 	if (!(params->flags & NILFS_RECLAIM_PARAM_PROTSEQ) ||
 	    (params->flags & (~0UL << __NR_NILFS_RECLAIM_PARAMS))) {

@@ -703,13 +710,16 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs,
 	if (ret < 0)
 		goto out_lock;
 
+	reclaimable_blocks = (nilfs_get_blocks_per_segment(nilfs) * n) -
+			(nilfs_vector_get_size(vdescv) +
+			nilfs_vector_get_size(bdescv));
+
 	if (stat) {
 		stat->live_pblks = nilfs_vector_get_size(bdescv);
 		stat->defunct_pblks = nblocks - stat->live_pblks;
 
 		stat->live_blks = stat->live_vblks + stat->live_pblks;
-		stat->defunct_blks = n * nilfs_get_blocks_per_segment(nilfs) -
-			stat->live_blks;
+		stat->defunct_blks = reclaimable_blocks;
 	}
 	if (dryrun)
 		goto out_lock;

@@ -725,6 +735,51 @@ int nilfs_xreclaim_segment(struct nilfs *nilfs,
 		goto out_lock;
 	}
 
+	/* if there are less reclaimable blocks than the
+	 * minimal threshold try to update suinfo
+	 * instead of cleaning */
+	if ((params->flags & NILFS_RECLAIM_PARAM_DEFERRABLE) &&
+			reclaimable_blocks < params->min_reclaimable_blks * n) {
+		if (stat)
+			stat->deferred_segs = n;
+
+		ret = gettimeofday(&tv, NULL);
+		if (ret < 0)
+			goto out_lock;
+
+		supv = malloc(sizeof(struct nilfs_suinfo_update) * n);
+		if (supv == NULL) {

lib/gc.c int nilfs_xreclaim_segment(struct nilfs *nilfs, · line 754

Quoted 3 lines

lib/gc.c +3 -0

+			ret = -1;
+			goto out_lock;
+		}

I have already changed nilfs_xreclaim_segments to use nilfs_vector_create for supv instead of malloc and free directly. Don't know why I originally used malloc, since the rest of the function uses the nilfs_vector API.

This patch adds a flag, that enables the GC to skip the timeout in certain situations. For example if the cleaning of some segments was deferred to a later time, then no real progress has been made. Apart from reading a few summary blocks, no data was read or written to disk. In this situation it makes sense to skip the normal timeout once and immediately try to clean the next set of segments.

Unfortunately it is not possible, to directly continue the cleaning loop, because this would lead to an unresponsive GC process. Therefore the timeout is simply set to 0 seconds.

Signed-off-by Andreas Rohner <[email protected]>

---
 sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

sbin/cleanerd/cleanerd.c +0 -0

index 1374e38..e1bd558 100644

sbin/cleanerd/cleanerd.c +1 -1

 	int shutdown;
 	long ncleansegs;
 	struct timeval cleaning_interval;

@@ -894,7 +895,7 @@ static int nilfs_cleanerd_recalc_interval(struct nilfs_cleanerd *cleanerd,
 	interval = nilfs_cleanerd_cleaning_interval(cleanerd);
 	/* timercmp() does not work for '>=' or '<='. */
 	/* curr >= target */
-	if (!timercmp(&curr, &cleanerd->target, <)) {
+	if (!timercmp(&curr, &cleanerd->target, <) || cleanerd->no_timeout) {
 		cleanerd->timeout.tv_sec = 0;
 		cleanerd->timeout.tv_usec = 0;
 		timeradd(&curr, interval, &cleanerd->target);

sbin/cleanerd/cleanerd.c +0 -2

 	memset(&stat, 0, sizeof(stat));
 	ret = nilfs_xreclaim_segment(cleanerd->nilfs, segnums, nsegs, 0,

@@ -1409,16 +1411,18 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 		goto out;
 	}
 
-	if (stat.cleaned_segs > 0) {
-		if (stat.deferred_segs > 0) {

sbin/cleanerd/cleanerd.c +4 -0

+	} else if (stat.cleaned_segs > 0) {
+		for (i = 0; i < stat.cleaned_segs; i++)
+			syslog(LOG_DEBUG, "segment %llu cleaned",
+			       (unsigned long long)segnums[i]);
 		nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
 		cleanerd->fallback = 0;

sbin/cleanerd/cleanerd.c +1 -0

 		cleanerd->retry_cleaning = 0;

@@ -1475,6 +1479,7 @@ static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
 	cleanerd->running = 1;
 	cleanerd->fallback = 0;
 	cleanerd->retry_cleaning = 0;
+	cleanerd->no_timeout = 0;
 	nilfs_cnoconv_reset(cleanerd->cnoconv);
 	nilfs_gc_logger = syslog;
 

Signature · 6 lines

1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 22 lines · Andreas Rohner

This patch adds a flag, that enables the GC to skip the timeout in certain situations. For example if the cleaning of some segments was deferred to a later time, then no real progress has been made. Apart from reading a few summary blocks, no data was read or written to disk. In this situation it makes sense to skip the normal timeout once and immediately try to clean the next set of segments.

Unfortunately it is not possible, to directly continue the cleaning loop, because this would lead to an unresponsive GC process. Therefore the timeout is simply set to 0 seconds.

Signed-off-by Andreas Rohner <[email protected]>

---
 sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

sbin/cleanerd/cleanerd.c +0 -0

index 1374e38..e1bd558 100644

sbin/cleanerd/cleanerd.c struct nilfs_cleanerd { · line 170

Quoted 4 lines

sbin/cleanerd/cleanerd.c +1 -0

@@ -167,6 +167,7 @@ struct nilfs_cleanerd {
 	int running;
 	int fallback;
 	int retry_cleaning;
+	int no_timeout;

Corresponding comment is missing for this one, too.

Quoted 13 lines

sbin/cleanerd/cleanerd.c +1 -1

 	int shutdown;
 	long ncleansegs;
 	struct timeval cleaning_interval;

@@ -894,7 +895,7 @@ static int nilfs_cleanerd_recalc_interval(struct nilfs_cleanerd *cleanerd,
 	interval = nilfs_cleanerd_cleaning_interval(cleanerd);
 	/* timercmp() does not work for '>=' or '<='. */
 	/* curr >= target */
-	if (!timercmp(&curr, &cleanerd->target, <)) {
+	if (!timercmp(&curr, &cleanerd->target, <) || cleanerd->no_timeout) {
 		cleanerd->timeout.tv_sec = 0;
 		cleanerd->timeout.tv_usec = 0;
 		timeradd(&curr, interval, &cleanerd->target);

sbin/cleanerd/cleanerd.c static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd, · line 1399

Quoted 4 lines

sbin/cleanerd/cleanerd.c +1 -0

@@ -1395,6 +1396,7 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 		       "number: %m");
 		goto out;
 	}
+	cleanerd->no_timeout = 0;

This one is needed for the else case of nilfs_cleanerd_clean_loop() ?

                if (ns > 0) {
                        ret = nilfs_cleanerd_clean_segments(
                                cleanerd, segnums, ns, sustat.ss_prot_seq,
                                prottime, &ndone);
                        if (ret < 0)
                                return -1;
                } else {
                        cleanerd->retry_cleaning = 0;

Quoted 9 lines

sbin/cleanerd/cleanerd.c +0 -2

 	memset(&stat, 0, sizeof(stat));
 	ret = nilfs_xreclaim_segment(cleanerd->nilfs, segnums, nsegs, 0,

@@ -1409,16 +1411,18 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 		goto out;
 	}
 
-	if (stat.cleaned_segs > 0) {
-		if (stat.deferred_segs > 0) {

sbin/cleanerd/cleanerd.c static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd, · line 1415

Quoted 10 lines

sbin/cleanerd/cleanerd.c +2 -8

-			for (i = 0; i < stat.cleaned_segs; i++)
-				syslog(LOG_DEBUG, "segment %llu deferred",
-				       (unsigned long long)segnums[i]);
-		} else {
-			for (i = 0; i < stat.cleaned_segs; i++)
-				syslog(LOG_DEBUG, "segment %llu cleaned",
-				       (unsigned long long)segnums[i]);
-		}
+	if (stat.deferred_segs > 0) {
+		for (i = 0; i < stat.cleaned_segs; i++)

Should be stat.deferred_segs ?

Looks like you took the meaning of stat.cleaned_segs differently.

I meant stat.cleaned_segs is decreased if stat.deferred_segs > 0.

So, the following relation will be fulfilled.

cleaned_segs + deferred_segs + protected_segs == nsegs (number of requested segments)

Quoted 5 lines

sbin/cleanerd/cleanerd.c +5 -0

+			syslog(LOG_DEBUG, "segment %llu deferred",
+			       (unsigned long long)segnums[i]);
+		nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
+		cleanerd->fallback = 0;
+		cleanerd->retry_cleaning = 0;

sbin/cleanerd/cleanerd.c static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd, · line 1421

Quoted 1 line

sbin/cleanerd/cleanerd.c +1 -0

+		cleanerd->no_timeout = 1;

So, I think this should be set only if
stat.deferred_segs > 0 && stat.cleaned_segs == 0

Regards,
Ryusuke Konishi

Quoted 15 lines

sbin/cleanerd/cleanerd.c +5 -0

+	} else if (stat.cleaned_segs > 0) {
+		for (i = 0; i < stat.cleaned_segs; i++)
+			syslog(LOG_DEBUG, "segment %llu cleaned",
+			       (unsigned long long)segnums[i]);
 		nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
 		cleanerd->fallback = 0;
 		cleanerd->retry_cleaning = 0;

@@ -1475,6 +1479,7 @@ static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
 	cleanerd->running = 1;
 	cleanerd->fallback = 0;
 	cleanerd->retry_cleaning = 0;
+	cleanerd->no_timeout = 0;
 	nilfs_cnoconv_reset(cleanerd->cnoconv);
 	nilfs_gc_logger = syslog;
 

Quoted 7 lines

Signature · 6 lines

1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 58 lines · Ryusuke Konishi

Quoted 22 lines · Andreas Rohner

This patch adds a flag, that enables the GC to skip the timeout in certain situations. For example if the cleaning of some segments was deferred to a later time, then no real progress has been made. Apart from reading a few summary blocks, no data was read or written to disk. In this situation it makes sense to skip the normal timeout once and immediately try to clean the next set of segments.

Unfortunately it is not possible, to directly continue the cleaning loop, because this would lead to an unresponsive GC process. Therefore the timeout is simply set to 0 seconds.

Signed-off-by Andreas Rohner <[email protected]>

---
 sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

sbin/cleanerd/cleanerd.c +0 -0

index 1374e38..e1bd558 100644

sbin/cleanerd/cleanerd.c struct nilfs_cleanerd { · line 170

Quoted 4 lines

sbin/cleanerd/cleanerd.c +1 -0

@@ -167,6 +167,7 @@ struct nilfs_cleanerd {
 	int running;
 	int fallback;
 	int retry_cleaning;
+	int no_timeout;

Corresponding comment is missing for this one, too.

Quoted 13 lines

sbin/cleanerd/cleanerd.c +1 -1

 	int shutdown;
 	long ncleansegs;
 	struct timeval cleaning_interval;

@@ -894,7 +895,7 @@ static int nilfs_cleanerd_recalc_interval(struct nilfs_cleanerd *cleanerd,
 	interval = nilfs_cleanerd_cleaning_interval(cleanerd);
 	/* timercmp() does not work for '>=' or '<='. */
 	/* curr >= target */
-	if (!timercmp(&curr, &cleanerd->target, <)) {
+	if (!timercmp(&curr, &cleanerd->target, <) || cleanerd->no_timeout) {
 		cleanerd->timeout.tv_sec = 0;
 		cleanerd->timeout.tv_usec = 0;
 		timeradd(&curr, interval, &cleanerd->target);

sbin/cleanerd/cleanerd.c static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd, · line 1399

Quoted 4 lines

sbin/cleanerd/cleanerd.c +1 -0

@@ -1395,6 +1396,7 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 		       "number: %m");
 		goto out;
 	}
+	cleanerd->no_timeout = 0;

This one is needed for the else case of nilfs_cleanerd_clean_loop() ?

                if (ns > 0) {
                        ret = nilfs_cleanerd_clean_segments(
                                cleanerd, segnums, ns, sustat.ss_prot_seq,
                                prottime, &ndone);
                        if (ret < 0)
                                return -1;
                } else {
                        cleanerd->retry_cleaning = 0;

Review

Quoted 2 lines

(quoted diff) +1 -0

+ 			cleanerd->no_timeout = 0;
                }

It is important to make sure that no_timeout is set to 1 for only one iteration of nilfs_cleanerd_clean_loop. Otherwise it would permanently disable the timeout. But you are right, this is probably not a good place to do it. In version 5 I moved it.