[PATCH 0/2] refactor reclaim function

[PATCH 0/2] refactor reclaim function

[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.

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.

Regards,
Ryusuke Konishi
---
Ryusuke Konishi (2):
      lib/gc.c: refactor reclaim function
      cleanerd: use nilfs_xreclaim_segment()

 include/nilfs_gc.h       |   60 +++++++++++++++++++++++++
 lib/gc.c                 |  111 +++++++++++++++++++++++++++++++++++++++-------
 sbin/cleanerd/cleanerd.c |   57 +++++++++++++++---------
 3 files changed, 192 insertions(+), 36 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] lib/gc.c: refactor reclaim function

[Ryusuke Konishi]

Add the following structures and functions to libnilfsgc library
and its header file (i.e. nilfs_gc.h):

 - nilfs_xreclaim_segment() function
 - nilfs_reclaim_params struct
 - nilfs_reclaim_stat struct
 - nilfs_assess_segment() inline function

nilfs_xreclaim_segment() function is the enhanced version of
nilfs_reclaim_segment().  This function takes GC parameters in an
expandable way using nilfs_reclaim_params structure, and outputs
statistics information of garbage collection including count of
live/dead blocks if a valid pointer to nilfs_reclaim_stat structure is
given.  This function also takes a dryrun argument, which allows
callers to get the statistics information without actually doing GC.

Developers can use an inline function nilfs_assess_segment() to get
the statistics information hiding the dryrun option.

Old api function nilfs_reclaim_segment() is still available for
compatibility reason, but the implementation is replaced by
nilfs_xreclaim_segment() function.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
---
 include/nilfs_gc.h |   60 ++++++++++++++++++++++++++++
 lib/gc.c           |  111 +++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 156 insertions(+), 15 deletions(-)

diff --git a/include/nilfs_gc.h b/include/nilfs_gc.h
index 72e9947..e49cbf4 100644
--- a/include/nilfs_gc.h
+++ b/include/nilfs_gc.h
@@ -14,10 +14,70 @@
 #include <sys/types.h>
 #include "nilfs.h"
 
+/* 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
+
+/**
+ * struct nilfs_reclaim_params - structure to specify GC parameters
+ * @flags: flags of valid fields
+ * @reserved: padding bytes
+ * @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;
+	__u64 protseq;
+	nilfs_cno_t protcno;
+};
+
+/**
+ * struct nilfs_reclaim_stat - structure to store GC statistics
+ * @exflags: flags for extended fields (reserved)
+ * @cleaned_segs: number of cleaned segments
+ * @protected_segs: number of protected (deselected) segments
+ * @deferred_segs: number of deferred segments
+ * @live_blks: number of live (in-use) blocks
+ * @live_vblks: number of live (in-use) virtual blocks
+ * @live_pblks: number of live (in-use) DAT file blocks
+ * @defunct_blks: number of defunct (reclaimable) blocks
+ * @defunct_vblks: number of defunct (reclaimable) virtual blocks
+ * @defunct_pblks: number of defunct (reclaimable) DAT file blocks
+ * @freed_vblks: number of freed virtual blocks
+ */
+struct nilfs_reclaim_stat {
+	unsigned long exflags;
+	size_t cleaned_segs;
+	size_t protected_segs;
+	size_t deferred_segs;
+	size_t live_blks;
+	size_t live_vblks;
+	size_t live_pblks;
+	size_t defunct_blks;
+	size_t defunct_vblks;
+	size_t defunct_pblks;
+	size_t freed_vblks;
+};
+
 ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
 			      __u64 *segnums, size_t nsegs,
 			      __u64 protseq, nilfs_cno_t protcno);
 
+int nilfs_xreclaim_segment(struct nilfs *nilfs,
+			   __u64 *segnums, size_t nsegs, int dryrun,
+			   const struct nilfs_reclaim_params *params,
+			   struct nilfs_reclaim_stat *stat);
+
+static inline int
+nilfs_assess_segment(struct nilfs *nilfs,
+		     __u64 *segnums, size_t nsegs,
+		     const struct nilfs_reclaim_params *params,
+		     struct nilfs_reclaim_stat *stat)
+{
+	return nilfs_xreclaim_segment(nilfs, segnums, nsegs, 1, params, stat);
+}
 
 static inline int nilfs_suinfo_reclaimable(const struct nilfs_suinfo *si)
 {
diff --git a/lib/gc.c b/lib/gc.c
index 1152299..71c7307 100644
--- a/lib/gc.c
+++ b/lib/gc.c
@@ -601,20 +601,34 @@ static int nilfs_toss_bdescs(struct nilfs_vector *bdescv)
 }
 
 /**
- * nilfs_reclaim_segment - reclaim segments
+ * nilfs_xreclaim_segment - reclaim segments (enhanced API)
  * @nilfs: nilfs object
  * @segnums: array of segment numbers storing selected segments
  * @nsegs: size of the @segnums array
- * @protseq: start of sequence number of protected segments
- * @protcno: start checkpoint number of protected period
+ * @dryrun: dry-run flag
+ * @params: reclaim parameters
+ * @stat: reclaim statistics
  */
-ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
-			      __u64 *segnums, size_t nsegs,
-			      __u64 protseq, nilfs_cno_t protcno)
+int nilfs_xreclaim_segment(struct nilfs *nilfs,
+			   __u64 *segnums, size_t nsegs, int dryrun,
+			   const struct nilfs_reclaim_params *params,
+			   struct nilfs_reclaim_stat *stat)
 {
 	struct nilfs_vector *vdescv, *bdescv, *periodv, *vblocknrv;
 	sigset_t sigset, oldset, waitset;
+	nilfs_cno_t protcno;
 	ssize_t n, ret = -1;
+	size_t nblocks;
+
+	if (!(params->flags & NILFS_RECLAIM_PARAM_PROTSEQ) ||
+	    (params->flags & (~0UL << __NR_NILFS_RECLAIM_PARAMS))) {
+		/*
+		 * The protseq parameter is mandatory.  Unknown
+		 * parameters are rejected.
+		 */
+		errno = EINVAL;
+		return -1;
+	}
 
 	if (nsegs == 0)
 		return 0;
@@ -623,8 +637,7 @@ ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
 	bdescv = nilfs_vector_create(sizeof(struct nilfs_bdesc));
 	periodv = nilfs_vector_create(sizeof(struct nilfs_period));
 	vblocknrv = nilfs_vector_create(sizeof(__u64));
-	if (vdescv == NULL || bdescv == NULL || periodv == NULL ||
-	    vblocknrv == NULL)
+	if (!vdescv || !bdescv || !periodv || !vblocknrv)
 		goto out_vec;
 
 	sigemptyset(&sigset);
@@ -633,7 +646,7 @@ ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
 	ret = sigprocmask(SIG_BLOCK, &sigset, &oldset);
 	if (ret < 0) {
 		nilfs_gc_logger(LOG_ERR, "cannot block signals: %s",
-				     strerror(errno));
+				strerror(errno));
 		goto out_vec;
 	}
 
@@ -641,31 +654,66 @@ ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
 	if (ret < 0)
 		goto out_sig;
 
-	n = nilfs_acc_blocks(nilfs, segnums, nsegs, protseq, vdescv, bdescv);
-	if (n <= 0) {
+	/* count blocks */
+	n = nilfs_acc_blocks(nilfs, segnums, nsegs, params->protseq, vdescv,
+			     bdescv);
+	if (n < 0) {
 		ret = n;
 		goto out_lock;
 	}
+	if (stat) {
+		stat->cleaned_segs = n;
+		stat->protected_segs = nsegs - n;
+		stat->deferred_segs = 0;
+	}
+	if (n == 0) {
+		ret = 0;
+		goto out_lock;
+	}
 
+	/* toss virtual blocks */
 	ret = nilfs_get_vdesc(nilfs, vdescv);
 	if (ret < 0)
 		goto out_lock;
 
+	nblocks = nilfs_vector_get_size(vdescv);
+	protcno = (params->flags & NILFS_RECLAIM_PARAM_PROTCNO) ?
+		params->protcno : NILFS_CNO_MAX;
+
 	ret = nilfs_toss_vdescs(nilfs, vdescv, periodv, vblocknrv, protcno);
 	if (ret < 0)
 		goto out_lock;
 
+	if (stat) {
+		stat->live_vblks = nilfs_vector_get_size(vdescv);
+		stat->defunct_vblks = nblocks - stat->live_vblks;
+		stat->freed_vblks = nilfs_vector_get_size(vblocknrv);
+	}
+
 	nilfs_vector_sort(vdescv, nilfs_comp_vdesc_blocknr);
 	nilfs_unify_period(periodv);
 
+	/* toss DAT file blocks */
 	ret = nilfs_get_bdesc(nilfs, bdescv);
 	if (ret < 0)
 		goto out_lock;
 
+	nblocks = nilfs_vector_get_size(bdescv);
 	ret = nilfs_toss_bdescs(bdescv);
 	if (ret < 0)
 		goto out_lock;
 
+	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;
+	}
+	if (dryrun)
+		goto out_lock;
+
 	ret = sigpending(&waitset);
 	if (ret < 0) {
 		nilfs_gc_logger(LOG_ERR, "cannot test signals: %s",
@@ -690,13 +738,14 @@ ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
 	if (ret < 0) {
 		nilfs_gc_logger(LOG_ERR, "cannot clean segments: %s",
 				strerror(errno));
-	} else {
-		ret = n;
 	}
 
 out_lock:
-	if (nilfs_unlock_cleaner(nilfs) < 0)
-		ret = -1;
+	if (nilfs_unlock_cleaner(nilfs) < 0) {
+		nilfs_gc_logger(LOG_CRIT, "failed to unlock cleaner: %s",
+				strerror(errno));
+		exit(1);
+	}
 
 out_sig:
 	sigprocmask(SIG_SETMASK, &oldset, NULL);
@@ -710,6 +759,38 @@ out_vec:
 		nilfs_vector_destroy(periodv);
 	if (vblocknrv != NULL)
 		nilfs_vector_destroy(vblocknrv);
+	/*
+	 * Flags of valid fields in stat->exflags must be unset.
+	 */
+	return ret;
+}
 
+/**
+ * nilfs_reclaim_segment - reclaim segments
+ * @nilfs: nilfs object
+ * @segnums: array of segment numbers storing selected segments
+ * @nsegs: size of the @segnums array
+ * @protseq: start of sequence number of protected segments
+ * @protcno: start checkpoint number of protected period
+ */
+ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
+			      __u64 *segnums, size_t nsegs,
+			      __u64 protseq, nilfs_cno_t protcno)
+{
+	struct nilfs_reclaim_params params;
+	struct nilfs_reclaim_stat stat;
+	int ret;
+
+	params.flags =
+		NILFS_RECLAIM_PARAM_PROTSEQ | NILFS_RECLAIM_PARAM_PROTCNO;
+	params.reserved = 0;
+	params.protseq = protseq;
+	params.protcno = protcno;
+	memset(&stat, 0, sizeof(stat));
+
+	ret = nilfs_xreclaim_segment(nilfs, segnums, nsegs, 0,
+				     &params, &stat);
+	if (!ret)
+		ret = stat.cleaned_segs;
 	return ret;
 }
-- 
1.7.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] cleanerd: use nilfs_xreclaim_segment()

[Ryusuke Konishi]

Refactor nilfs_cleanerd_clean_segments() with nilfs_xreclaim_segment()
function.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
---
 sbin/cleanerd/cleanerd.c |   57 +++++++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
index 742ab98..1d09b5b 100644
--- a/sbin/cleanerd/cleanerd.c
+++ b/sbin/cleanerd/cleanerd.c
@@ -1338,31 +1338,49 @@ static void nilfs_cleanerd_progress(struct nilfs_cleanerd *cleanerd, int nsegs)
 	}
 }
 
-static ssize_t nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
-					     __u64 *segnums, size_t nsegs,
-					     __u64 protseq, __u64 prottime)
+static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
+					 __u64 *segnums, size_t nsegs,
+					 __u64 protseq, __u64 prottime,
+					 size_t *ncleaned)
 {
-	nilfs_cno_t protcno;
+	struct nilfs_reclaim_params params;
+	struct nilfs_reclaim_stat stat;
 	int ret, i;
 
-	ret = nilfs_cnoconv_time2cno(cleanerd->cnoconv, prottime, &protcno);
+	params.flags =
+		NILFS_RECLAIM_PARAM_PROTSEQ | NILFS_RECLAIM_PARAM_PROTCNO;
+	params.reserved = 0;
+	params.protseq = protseq;
+
+	ret = nilfs_cnoconv_time2cno(cleanerd->cnoconv, prottime,
+				     &params.protcno);
 	if (ret < 0) {
 		syslog(LOG_ERR, "cannot convert protection time to checkpoint "
 		       "number: %m");
 		goto out;
 	}
 
-	ret = nilfs_reclaim_segment(cleanerd->nilfs, segnums, nsegs,
-				    protseq, protcno);
-	if (ret > 0) {
-		for (i = 0; i < ret; i++)
+	memset(&stat, 0, sizeof(stat));
+	ret = nilfs_xreclaim_segment(cleanerd->nilfs, segnums, nsegs, 0,
+				     &params, &stat);
+	if (ret < 0) {
+		if (errno == ENOMEM) {
+			nilfs_cleanerd_reduce_ncleansegs_for_retry(cleanerd);
+			cleanerd->fallback = 1;
+			*ncleaned = 0;
+			ret = 0;
+		}
+		goto out;
+	}
+
+	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, ret);
+		nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
 		cleanerd->fallback = 0;
 		cleanerd->retry_cleaning = 0;
-
-	} else if (ret == 0) {
+	} else {
 		syslog(LOG_DEBUG, "no segments cleaned");
 
 		if (!cleanerd->retry_cleaning &&
@@ -1375,12 +1393,8 @@ static ssize_t nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 		} else {
 			cleanerd->retry_cleaning = 0;
 		}
-
-	} else if (ret < 0 && errno == ENOMEM) {
-		nilfs_cleanerd_reduce_ncleansegs_for_retry(cleanerd);
-		cleanerd->fallback = 1;
-		ret = 0;
 	}
+	*ncleaned = stat.cleaned_segs;
 out:
 	return ret;
 }
@@ -1395,7 +1409,8 @@ static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
 	__u64 prottime = 0, oldest = 0;
 	__u64 segnums[NILFS_CLDCONFIG_NSEGMENTS_PER_CLEAN_MAX];
 	sigset_t sigset;
-	int ns, ndone, ret;
+	size_t ndone;
+	int ns, ret;
 
 	sigemptyset(&sigset);
 	if (sigprocmask(SIG_SETMASK, &sigset, NULL) < 0) {
@@ -1466,10 +1481,10 @@ static int nilfs_cleanerd_clean_loop(struct nilfs_cleanerd *cleanerd)
 		       ns, (ns <= 1) ? "" : "s");
 		ndone = 0;
 		if (ns > 0) {
-			ndone = nilfs_cleanerd_clean_segments(
+			ret = nilfs_cleanerd_clean_segments(
 				cleanerd, segnums, ns, sustat.ss_prot_seq,
-				prottime);
-			if (ndone < 0)
+				prottime, &ndone);
+			if (ret < 0)
 				return -1;
 		} else {
 			cleanerd->retry_cleaning = 0;
-- 
1.7.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] refactor reclaim function

[Ryusuke Konishi]

Hi Andreas,
On Wed,  5 Feb 2014 03:37:24 +0900, Ryusuke Konishi wrote:
> 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.

Can you rebase your GC patchset on this?

Regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] refactor reclaim function

[Andreas Rohner]

Hi Ryusuke,

On 2014-02-04 19:42, Ryusuke Konishi wrote:
> Hi Andreas,
> On Wed,  5 Feb 2014 03:37:24 +0900, Ryusuke Konishi wrote:
>> 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.
> 
> Can you rebase your GC patchset on this?

Yes, I'll send you the patches as soon as possible.

Best regards,
Andreas Rohner
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 0/6] nilfs-utils: shortcut for certain GC operations

[Andreas Rohner]

Hi,

This patch set adds an optimized version of 
nilfs_xreclaim_segments, which has an additional parameter 
min_reclaimable_blks. 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.

Additionally new options are introduced for the configuration file 
and nilfs-clean to allow the user to specify the threshold. 

This is potentially useful for all gc policies, but it is especially
beneficial for the timestamp policy. Lets assume for example a NILFS2
volume with 20% static files and lets assume these static files 
are in the oldest segments. The current timestamp policy will 
select the oldest segments and, since the data is static, move 
them mostly unchanged to new segments. After a while they will 
become the oldest segments again. Then timestamp will move them 
again. These moving operations are expensive and unnecessary.

Regards,
Andreas Rohner

---
v3->v4
* nilfs-utils: Rebase on refactored nilfs_reclaim_segments (Ryusuke)
* nilfs-utils: Add no_timeout if cleaning was deferred
v2->v3
* Alignment in nilfs_suinfo_update
* Add missing comments
* nilfs2: Extra validity checks in nilfs_sufile_set_suinfo
* nilfs2: Update sh_ncleansegs, sh_ndirtysegs if flags change
* nilfs2: Fix bugs in nilfs_sufile_set_suinfo
* nilfs2: Use vmalloc instead of ioctl_wrap_copy
* nilfs-utils: Separate nilfs_relaim_segments_with_threshold
* nilfs-utils: Allow different units for min_reclaimable_blocks
* nilfs-utils: Introduce flag to disable the optimization
* nilfs-utils: Disable optimization if kernel retruns ENOTTY
* nilfs-utils: Remove EGCTRYAGAIN error return code
* nilfs-utils: Rename option to min_reclaimable_blocks
v1->v2
* Implementation of NILFS_IOCTL_SET_SUINFO
* Added mc_min_free_blocks_threshold config option
  (if clean segments < min_clean_segments)
* Added new command line param for nilfs-clean
* Update man- and config-file
--
Andreas Rohner (6):
  nilfs-utils: cldconfig add an option to set min. reclaimable blocks
  nilfs-utils: add NILFS_OPT_SET_SUINFO
  nilfs-utils: nilfs-clean add cmdline param min-reclaimable-blocks
  nilfs-utils: add suport for NILFS_IOCTL_SET_SUINFO ioctl
  nilfs-utils: add optimized version of nilfs_reclaim_segments
  nilfs-utils: add a no_timeout flag to enable faster loop

 include/nilfs.h                   |   9 +-
 include/nilfs2_fs.h               |  43 ++++++++++
 include/nilfs_cleaner.h           |  19 +++--
 include/nilfs_gc.h                |   7 +-
 lib/gc.c                          |  63 +++++++++++++-
 lib/nilfs.c                       |  63 ++++++++++++++
 man/nilfs-clean.8                 |   4 +
 man/nilfs_cleanerd.conf.5         |  22 +++++
 sbin/cleanerd/cldconfig.c         | 168 ++++++++++++++++++++++++++++----------
 sbin/cleanerd/cldconfig.h         |   8 ++
 sbin/cleanerd/cleanerd.c          |  55 ++++++++++++-
 sbin/cleanerd/nilfs_cleanerd.conf |  13 +++
 sbin/nilfs-clean/nilfs-clean.c    |  44 ++++++++--
 13 files changed, 450 insertions(+), 68 deletions(-)

-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 1/6] nilfs-utils: cldconfig add an option to set min. reclaimable blocks

[Andreas Rohner]

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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
---
 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(-)

diff --git a/man/nilfs_cleanerd.conf.5 b/man/nilfs_cleanerd.conf.5
index 8f3fcd2..4315ecd 100644
--- a/man/nilfs_cleanerd.conf.5
+++ b/man/nilfs_cleanerd.conf.5
@@ -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,
diff --git a/sbin/cleanerd/cldconfig.c b/sbin/cleanerd/cldconfig.c
index 270d360..b69f060 100644
--- a/sbin/cleanerd/cldconfig.c
+++ b/sbin/cleanerd/cldconfig.c
@@ -278,6 +278,54 @@ nilfs_cldconfig_handle_protection_period(struct nilfs_cldconfig *config,
 }
 
 static unsigned long long
+nilfs_convert_units_to_bytes(struct nilfs_param *param) {
+	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)
diff --git a/sbin/cleanerd/cldconfig.h b/sbin/cleanerd/cldconfig.h
index 188ce9b..e7ac72e 100644
--- a/sbin/cleanerd/cldconfig.h
+++ b/sbin/cleanerd/cldconfig.h
@@ -101,7 +101,10 @@ struct nilfs_cldconfig {
 	struct timeval cf_mc_cleaning_interval;
 	struct timeval cf_retry_interval;
 	int cf_use_mmap;
+	int cf_use_set_suinfo;
 	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
 
diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
index 1d09b5b..b441448 100644
--- a/sbin/cleanerd/cleanerd.c
+++ b/sbin/cleanerd/cleanerd.c
@@ -172,6 +172,7 @@ struct nilfs_cleanerd {
 	struct timeval cleaning_interval;
 	struct timeval target;
 	struct timeval timeout;
+	unsigned long min_reclaimable_blocks;
 	__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))
diff --git a/sbin/cleanerd/nilfs_cleanerd.conf b/sbin/cleanerd/nilfs_cleanerd.conf
index 26872aa..f88a57a 100644
--- a/sbin/cleanerd/nilfs_cleanerd.conf
+++ b/sbin/cleanerd/nilfs_cleanerd.conf
@@ -51,6 +51,19 @@ mc_cleaning_interval	1
 # Retry interval in seconds.
 retry_interval		60
 
+# Specify the minimum number of reclaimable blocks in a segment
+# before it can be cleaned.
+min_reclaimable_blocks	5%
+
+# Specify the minimum number of reclaimable blocks in a segment
+# before it can be cleaned.
+# if clean segments < min_clean_segments
+mc_min_reclaimable_blocks	1%
+
+# enable set_suinfo ioctl if supported
+# (needed for min_reclaimable_blocks)
+use_set_suinfo
+
 # Use mmap when reading segments if supported.
 use_mmap
 
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/6] nilfs-utils: add NILFS_OPT_SET_SUINFO

[Andreas Rohner]

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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
---
 include/nilfs.h          |  7 ++++++-
 lib/nilfs.c              | 28 ++++++++++++++++++++++++++++
 sbin/cleanerd/cleanerd.c |  6 ++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/nilfs.h b/include/nilfs.h
index 56286a9..b5f85d3 100644
--- a/include/nilfs.h
+++ b/include/nilfs.h
@@ -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 *);
diff --git a/lib/nilfs.c b/lib/nilfs.c
index c1771e8..93822de 100644
--- a/lib/nilfs.c
+++ b/lib/nilfs.c
@@ -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];
diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
index b441448..970816f 100644
--- a/sbin/cleanerd/cleanerd.c
+++ b/sbin/cleanerd/cleanerd.c
@@ -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) {
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/6] nilfs-utils: nilfs-clean add cmdline param min-reclaimable-blocks

[Andreas Rohner]

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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
---
 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(-)

diff --git a/include/nilfs_cleaner.h b/include/nilfs_cleaner.h
index 0bf02aa..874c17a 100644
--- a/include/nilfs_cleaner.h
+++ b/include/nilfs_cleaner.h
@@ -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,
diff --git a/man/nilfs-clean.8 b/man/nilfs-clean.8
index 04e11c6..94267e1 100644
--- a/man/nilfs-clean.8
+++ b/man/nilfs-clean.8
@@ -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
diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
index 970816f..a3f62b0 100644
--- a/sbin/cleanerd/cleanerd.c
+++ b/sbin/cleanerd/cleanerd.c
@@ -185,6 +185,7 @@ struct nilfs_cleanerd {
 	long mm_ncleansegs;
 	struct timeval mm_protection_period;
 	struct timeval mm_cleaning_interval;
+	unsigned long mm_min_reclaimable_blocks;
 };
 
 /**
@@ -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;
 	}
+	/* minimal reclaimable blocks */
+	if ((req2->args.valid & NILFS_CLEANER_ARG_MIN_RECLAIMABLE_BLOCKS)
+		&& req2->args.min_reclaimable_blocks <=
+			nilfs_get_blocks_per_segment(cleanerd->nilfs)) {
+		cleanerd->mm_min_reclaimable_blocks =
+			req2->args.min_reclaimable_blocks;
+	} else {
+		cleanerd->mm_min_reclaimable_blocks =
+			cleanerd->min_reclaimable_blocks;
+	}
 	/* number of passes */
 	if (req2->args.valid & NILFS_CLEANER_ARG_NPASSES) {
 		if (!req2->args.npasses)
diff --git a/sbin/nilfs-clean/nilfs-clean.c b/sbin/nilfs-clean/nilfs-clean.c
index 55abd55..f77fdf7 100644
--- a/sbin/nilfs-clean/nilfs-clean.c
+++ b/sbin/nilfs-clean/nilfs-clean.c
@@ -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);
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 4/6] nilfs-utils: add suport for NILFS_IOCTL_SET_SUINFO ioctl

[Andreas Rohner]

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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
---
 include/nilfs.h     |  2 ++
 include/nilfs2_fs.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 lib/nilfs.c         | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)

diff --git a/include/nilfs.h b/include/nilfs.h
index b5f85d3..da18e24 100644
--- a/include/nilfs.h
+++ b/include/nilfs.h
@@ -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);
diff --git a/include/nilfs2_fs.h b/include/nilfs2_fs.h
index e674f44..cf8f633 100644
--- a/include/nilfs2_fs.h
+++ b/include/nilfs2_fs.h
@@ -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 */
diff --git a/lib/nilfs.c b/lib/nilfs.c
index 93822de..e6f7c1e 100644
--- a/lib/nilfs.c
+++ b/lib/nilfs.c
@@ -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
+ * @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.
+ */
+ssize_t nilfs_set_suinfo(struct nilfs *nilfs,
+			 struct nilfs_suinfo_update *sup, size_t nsup)
+{
+	struct nilfs_argv argv;
+
+	if (nilfs->n_iocfd < 0) {
+		errno = EBADF;
+		return -1;
+	}
+
+	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);
+		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
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 5/6] nilfs-utils: add optimized version of nilfs_xreclaim_segments

[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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
---
 include/nilfs_gc.h       |  7 +++---
 lib/gc.c                 | 63 +++++++++++++++++++++++++++++++++++++++++++++---
 sbin/cleanerd/cleanerd.c | 18 +++++++++++---
 3 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/include/nilfs_gc.h b/include/nilfs_gc.h
index e49cbf4..1196c55 100644
--- a/include/nilfs_gc.h
+++ b/include/nilfs_gc.h
@@ -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;
 };
diff --git a/lib/gc.c b/lib/gc.c
index 71c7307..4aa6a2c 100644
--- a/lib/gc.c
+++ b/lib/gc.c
@@ -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) {
+			ret = -1;
+			goto out_lock;
+		}
+
+		for (i = 0; i < n; ++i) {
+			supv[i].sup_segnum = segnums[i];
+			supv[i].sup_flags = 0;
+			nilfs_suinfo_update_set_lastmod(&supv[i]);
+			supv[i].sup_sui.sui_lastmod = tv.tv_sec;
+		}
+
+		ret = nilfs_set_suinfo(nilfs, supv, n);
+		free(supv);
+
+		if (ret == 0)
+			goto out_lock;
+
+		if (ret < 0 && errno != ENOTTY) {
+			nilfs_gc_logger(LOG_ERR, "cannot set suinfo: %s",
+					strerror(errno));
+			goto out_lock;
+		}
+
+		/* errno == ENOTTY */
+		nilfs_gc_logger(LOG_WARNING,
+				"set_suinfo ioctl is not supported");
+		if (stat)
+			stat->deferred_segs = 0;
+		/* Try nilfs_clean_segments */
+	}
+
 	ret = nilfs_clean_segments(nilfs,
 				   nilfs_vector_get_data(vdescv),
 				   nilfs_vector_get_size(vdescv),
@@ -783,7 +838,7 @@ ssize_t nilfs_reclaim_segment(struct nilfs *nilfs,
 
 	params.flags =
 		NILFS_RECLAIM_PARAM_PROTSEQ | NILFS_RECLAIM_PARAM_PROTCNO;
-	params.reserved = 0;
+	params.min_reclaimable_blks = 0;
 	params.protseq = protseq;
 	params.protcno = protcno;
 	memset(&stat, 0, sizeof(stat));
diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
index a3f62b0..1374e38 100644
--- a/sbin/cleanerd/cleanerd.c
+++ b/sbin/cleanerd/cleanerd.c
@@ -1381,7 +1381,11 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 
 	params.flags =
 		NILFS_RECLAIM_PARAM_PROTSEQ | NILFS_RECLAIM_PARAM_PROTCNO;
-	params.reserved = 0;
+	if (nilfs_opt_test_set_suinfo(cleanerd->nilfs))
+		params.flags |= NILFS_RECLAIM_PARAM_DEFERRABLE;
+
+	params.min_reclaimable_blks =
+			nilfs_cleanerd_min_reclaimable_blocks(cleanerd);
 	params.protseq = protseq;
 
 	ret = nilfs_cnoconv_time2cno(cleanerd->cnoconv, prottime,
@@ -1406,9 +1410,15 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 	}
 
 	if (stat.cleaned_segs > 0) {
-		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++)
+				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]);
+		}
 		nilfs_cleanerd_progress(cleanerd, stat.cleaned_segs);
 		cleanerd->fallback = 0;
 		cleanerd->retry_cleaning = 0;
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 6/6] nilfs-utils: add a no_timeout flag to enable faster loop

[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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
---
 sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
index 1374e38..e1bd558 100644
--- a/sbin/cleanerd/cleanerd.c
+++ b/sbin/cleanerd/cleanerd.c
@@ -167,6 +167,7 @@ struct nilfs_cleanerd {
 	int running;
 	int fallback;
 	int retry_cleaning;
+	int no_timeout;
 	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);
@@ -1395,6 +1396,7 @@ static int nilfs_cleanerd_clean_segments(struct nilfs_cleanerd *cleanerd,
 		       "number: %m");
 		goto out;
 	}
+	cleanerd->no_timeout = 0;
 
 	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) {
-			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++)
+			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;
+		cleanerd->no_timeout = 1;
+	} 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;
 
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/6] nilfs-utils: cldconfig add an option to set min. reclaimable blocks

[Ryusuke Konishi]

On Wed,  5 Feb 2014 03:16:34 +0100, Andreas Rohner wrote:
> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
> ---
>  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(-)
> 
> diff --git a/man/nilfs_cleanerd.conf.5 b/man/nilfs_cleanerd.conf.5
> index 8f3fcd2..4315ecd 100644
> --- a/man/nilfs_cleanerd.conf.5
> +++ b/man/nilfs_cleanerd.conf.5
> @@ -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,
> diff --git a/sbin/cleanerd/cldconfig.c b/sbin/cleanerd/cldconfig.c
> index 270d360..b69f060 100644
> --- a/sbin/cleanerd/cldconfig.c
> +++ b/sbin/cleanerd/cldconfig.c
> @@ -278,6 +278,54 @@ nilfs_cldconfig_handle_protection_period(struct nilfs_cldconfig *config,
>  }
>  
>  static unsigned long long
> +nilfs_convert_units_to_bytes(struct nilfs_param *param) {

Adding "const" qualifier is preferred for this argument.  And, please
put the opening brace at the beginning of the next line for functions:

static unsigned long long
nilfs_convert_units_to_bytes(const struct nilfs_param *param)
{


> +	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)
> diff --git a/sbin/cleanerd/cldconfig.h b/sbin/cleanerd/cldconfig.h
> index 188ce9b..e7ac72e 100644
> --- a/sbin/cleanerd/cldconfig.h
> +++ b/sbin/cleanerd/cldconfig.h
> @@ -101,7 +101,10 @@ struct nilfs_cldconfig {
>  	struct timeval cf_mc_cleaning_interval;
>  	struct timeval cf_retry_interval;
>  	int cf_use_mmap;
> +	int cf_use_set_suinfo;

Corresponding comment is missing.

>  	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
>  
> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
> index 1d09b5b..b441448 100644
> --- a/sbin/cleanerd/cleanerd.c
> +++ b/sbin/cleanerd/cleanerd.c
> @@ -172,6 +172,7 @@ struct nilfs_cleanerd {
>  	struct timeval cleaning_interval;
>  	struct timeval target;
>  	struct timeval timeout;
> +	unsigned long min_reclaimable_blocks;

Ditto.

>  	__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))
> diff --git a/sbin/cleanerd/nilfs_cleanerd.conf b/sbin/cleanerd/nilfs_cleanerd.conf
> index 26872aa..f88a57a 100644
> --- a/sbin/cleanerd/nilfs_cleanerd.conf
> +++ b/sbin/cleanerd/nilfs_cleanerd.conf
> @@ -51,6 +51,19 @@ mc_cleaning_interval	1
>  # Retry interval in seconds.
>  retry_interval		60
>  
> +# Specify the minimum number of reclaimable blocks in a segment
> +# before it can be cleaned.
> +min_reclaimable_blocks	5%
> +
> +# Specify the minimum number of reclaimable blocks in a segment
> +# before it can be cleaned.
> +# if clean segments < min_clean_segments
> +mc_min_reclaimable_blocks	1%

Description on suffixes is needed:

# The argument of min_reclaimable_blocks and mc_min_reclaimable_blocks
# can be followed by a percent sign (%) or one of the following
# multiplicative suffixes similar to min_clean_segments.
#
# If the argument is followed by "%", it represents a ratio for the
# number of blocks per segment.

> +
> +# enable set_suinfo ioctl if supported
> +# (needed for min_reclaimable_blocks)
> +use_set_suinfo

This parameter should be commented out at present.

Thanks,
Ryusuke Konishi

> +
>  # Use mmap when reading segments if supported.
>  use_mmap
>  
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/6] nilfs-utils: add NILFS_OPT_SET_SUINFO

[Ryusuke Konishi]

On Wed,  5 Feb 2014 03:16:35 +0100, Andreas Rohner wrote:
> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>

This looks OK to me.

Regards,
Ryusuke Konishi

> ---
>  include/nilfs.h          |  7 ++++++-
>  lib/nilfs.c              | 28 ++++++++++++++++++++++++++++
>  sbin/cleanerd/cleanerd.c |  6 ++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/include/nilfs.h b/include/nilfs.h
> index 56286a9..b5f85d3 100644
> --- a/include/nilfs.h
> +++ b/include/nilfs.h
> @@ -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 *);
> diff --git a/lib/nilfs.c b/lib/nilfs.c
> index c1771e8..93822de 100644
> --- a/lib/nilfs.c
> +++ b/lib/nilfs.c
> @@ -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];
> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
> index b441448..970816f 100644
> --- a/sbin/cleanerd/cleanerd.c
> +++ b/sbin/cleanerd/cleanerd.c
> @@ -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) {
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/6] nilfs-utils: nilfs-clean add cmdline param min-reclaimable-blocks

[Ryusuke Konishi]

On Wed,  5 Feb 2014 03:16:36 +0100, Andreas Rohner wrote:
> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
> ---
>  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(-)
> 
> diff --git a/include/nilfs_cleaner.h b/include/nilfs_cleaner.h
> index 0bf02aa..874c17a 100644
> --- a/include/nilfs_cleaner.h
> +++ b/include/nilfs_cleaner.h
> @@ -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,
> diff --git a/man/nilfs-clean.8 b/man/nilfs-clean.8
> index 04e11c6..94267e1 100644
> --- a/man/nilfs-clean.8
> +++ b/man/nilfs-clean.8
> @@ -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
> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
> index 970816f..a3f62b0 100644
> --- a/sbin/cleanerd/cleanerd.c
> +++ b/sbin/cleanerd/cleanerd.c
> @@ -185,6 +185,7 @@ struct nilfs_cleanerd {
>  	long mm_ncleansegs;
>  	struct timeval mm_protection_period;
>  	struct timeval mm_cleaning_interval;
> +	unsigned long mm_min_reclaimable_blocks;

Corresponding comment should be added.

>  };
>  
>  /**
> @@ -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;
>  	}
> +	/* minimal reclaimable blocks */
> +	if ((req2->args.valid & NILFS_CLEANER_ARG_MIN_RECLAIMABLE_BLOCKS)
> +		&& req2->args.min_reclaimable_blocks <=
> +			nilfs_get_blocks_per_segment(cleanerd->nilfs)) {
> +		cleanerd->mm_min_reclaimable_blocks =
> +			req2->args.min_reclaimable_blocks;
> +	} else {
> +		cleanerd->mm_min_reclaimable_blocks =
> +			cleanerd->min_reclaimable_blocks;
> +	}


If the value of req2->args.min_reclaimable_blocks is out of range,
NILFS_CLEANER_RSP_NACK should be returned:

	if (req2->args.valid & NILFS_CLEANER_ARG_MIN_RECLAIMABLE_BLOCKS) {
		if (req2->args.min_reclaimable_blocks >
		    nilfs_get_blocks_per_segment(cleanerd->nilfs))
			goto error_inval;
		cleanerd->mm_min_reclaimable_blocks =
			req2->args.min_reclaimable_blocks;
	} else {
		cleanerd->mm_min_reclaimable_blocks =
			cleanerd->min_reclaimable_blocks;
	}


Regards,
Ryusuke Konishi

>  	/* number of passes */
>  	if (req2->args.valid & NILFS_CLEANER_ARG_NPASSES) {
>  		if (!req2->args.npasses)
> diff --git a/sbin/nilfs-clean/nilfs-clean.c b/sbin/nilfs-clean/nilfs-clean.c
> index 55abd55..f77fdf7 100644
> --- a/sbin/nilfs-clean/nilfs-clean.c
> +++ b/sbin/nilfs-clean/nilfs-clean.c
> @@ -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);
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/6] nilfs-utils: add suport for NILFS_IOCTL_SET_SUINFO ioctl

[Ryusuke Konishi]

On Wed,  5 Feb 2014 03:16:37 +0100, Andreas Rohner wrote:
> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
> ---
>  include/nilfs.h     |  2 ++
>  include/nilfs2_fs.h | 43 +++++++++++++++++++++++++++++++++++++++++++
>  lib/nilfs.c         | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
> 
> diff --git a/include/nilfs.h b/include/nilfs.h
> index b5f85d3..da18e24 100644
> --- a/include/nilfs.h
> +++ b/include/nilfs.h
> @@ -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);
> diff --git a/include/nilfs2_fs.h b/include/nilfs2_fs.h
> index e674f44..cf8f633 100644
> --- a/include/nilfs2_fs.h
> +++ b/include/nilfs2_fs.h
> @@ -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 */
> diff --git a/lib/nilfs.c b/lib/nilfs.c
> index 93822de..e6f7c1e 100644
> --- a/lib/nilfs.c
> +++ b/lib/nilfs.c
> @@ -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
> + * @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.
> + */
> +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.

> +{
> +	struct nilfs_argv argv;
> +
> +	if (nilfs->n_iocfd < 0) {
> +		errno = EBADF;
> +		return -1;
> +	}
> +
> +	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);
}


Regards,
Ryusuke Konishi


> +		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
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/6] nilfs-utils: add suport for NILFS_IOCTL_SET_SUINFO ioctl

[Andreas Rohner]

Hi Ryusuke,

On 2014-02-05 20:25, Ryusuke Konishi wrote:
>>  /**
>> + * 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.
>> + */
>> +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.
> 
>> +{
>> +	struct nilfs_argv argv;
>> +
>> +	if (nilfs->n_iocfd < 0) {
>> +		errno = EBADF;
>> +		return -1;
>> +	}
>> +
>> +	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);
> }

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.

Best regards,
Andreas Rohner
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/6] nilfs-utils: add suport for NILFS_IOCTL_SET_SUINFO ioctl

[Ryusuke Konishi]

On Thu, 06 Feb 2014 00:43:01 +0100, Andreas Rohner wrote:
> Hi Ryusuke,
> 
> On 2014-02-05 20:25, Ryusuke Konishi wrote:
>>>  /**
>>> + * nilfs_set_suinfo - sets segment usage info
>>> + * @nilfs: nilfs object
..
>> 
>> 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);
>> }
> 
> 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.

Yes, I am still reviewing the remaining patches.
Please wait for it.

Thanks,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/6] nilfs-utils: add optimized version of nilfs_xreclaim_segments

[Andreas Rohner]

Hi Ryusuke,

On 2014-02-05 03:16, Andreas Rohner wrote:
> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
> ---
>  include/nilfs_gc.h       |  7 +++---
>  lib/gc.c                 | 63 +++++++++++++++++++++++++++++++++++++++++++++---
>  sbin/cleanerd/cleanerd.c | 18 +++++++++++---
>  3 files changed, 77 insertions(+), 11 deletions(-)
> 
> diff --git a/include/nilfs_gc.h b/include/nilfs_gc.h
> index e49cbf4..1196c55 100644
> --- a/include/nilfs_gc.h
> +++ b/include/nilfs_gc.h
> @@ -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;
>  };
> diff --git a/lib/gc.c b/lib/gc.c
> index 71c7307..4aa6a2c 100644
> --- a/lib/gc.c
> +++ b/lib/gc.c
> @@ -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) {
> +			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.

Regards,
Andreas Rohner
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/6] nilfs-utils: add optimized version of nilfs_xreclaim_segments

[Ryusuke Konishi]

On Thu, 06 Feb 2014 01:09:25 +0100, Andreas Rohner wrote:
> Hi Ryusuke,
> 
> On 2014-02-05 03:16, Andreas Rohner wrote:
>> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
>> ---
>>  include/nilfs_gc.h       |  7 +++---
>>  lib/gc.c                 | 63 +++++++++++++++++++++++++++++++++++++++++++++---
>>  sbin/cleanerd/cleanerd.c | 18 +++++++++++---
>>  3 files changed, 77 insertions(+), 11 deletions(-)
>> 
>> diff --git a/include/nilfs_gc.h b/include/nilfs_gc.h
>> index e49cbf4..1196c55 100644
>> --- a/include/nilfs_gc.h
>> +++ b/include/nilfs_gc.h
>> @@ -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;
>>  };
>> diff --git a/lib/gc.c b/lib/gc.c
>> index 71c7307..4aa6a2c 100644
>> --- a/lib/gc.c
>> +++ b/lib/gc.c
>> @@ -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) {
>> +			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.

Ok, I skip this one to review new patches.

The current comments on this patch are as follows:

1. Multiline comments should be as follows:

 /*
  * if there are less reclaimable blocks than the minimal
  * threshold try to update suinfo instead of cleaning
  */

2. The flag name "NILFS_RECLAIM_PARAM_DEFERRABLE" looks confusing.
   Why not use NILFS_RECLAIM_PARAM_MIN_RECLAIMABLE_BLKS ?


Regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/6] nilfs-utils: add a no_timeout flag to enable faster loop

[Ryusuke Konishi]

On Wed,  5 Feb 2014 03:16:39 +0100, Andreas Rohner wrote:
> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
> ---
>  sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
> index 1374e38..e1bd558 100644
> --- a/sbin/cleanerd/cleanerd.c
> +++ b/sbin/cleanerd/cleanerd.c
> @@ -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.

>  	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);
> @@ -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;
+ 			cleanerd->no_timeout = 0;
                }

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

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

> +		cleanerd->no_timeout = 1;

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

Regards,
Ryusuke Konishi


> +	} 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;
>  
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/6] nilfs-utils: add a no_timeout flag to enable faster loop

[Andreas Rohner]

On 2014-02-06 02:16, Ryusuke Konishi wrote:
> On Wed,  5 Feb 2014 03:16:39 +0100, Andreas Rohner wrote:
>> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
>> ---
>>  sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
>>  1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
>> index 1374e38..e1bd558 100644
>> --- a/sbin/cleanerd/cleanerd.c
>> +++ b/sbin/cleanerd/cleanerd.c
>> @@ -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.
> 
>>  	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);
>> @@ -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;
> + 			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.

>>  
>>  	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) {
>> -			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)
> 
>> +			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;
> 
>> +		cleanerd->no_timeout = 1;
> 
> So, I think this should be set only if
> stat.deferred_segs > 0 && stat.cleaned_segs == 0

Ok I interpreted it to be a subset of cleaned_segs. So deferred_segs is
the number of segments out of cleaned_segs that were deferred. But your
interpretation makes more sense.

In version 5 I also renamed the return parameter ncleaned of
nilfs_cleanerd_clean_segments to ndone, meaning "cleaned or deferred".

Regards,
Andreas Rohner

> Regards,
> Ryusuke Konishi
> 
> 
>> +	} 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;
>>  
>> -- 
>> 1.8.5.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 6/6] nilfs-utils: add a no_timeout flag to enable faster loop

[Andreas Rohner]

On 2014-02-06 15:37, Andreas Rohner wrote:
> On 2014-02-06 02:16, Ryusuke Konishi wrote:
>> On Wed,  5 Feb 2014 03:16:39 +0100, Andreas Rohner wrote:
>>> 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 <andreas.rohner-hi6Y0CQ0nG0@public.gmane.org>
>>> ---
>>>  sbin/cleanerd/cleanerd.c | 27 ++++++++++++++++-----------
>>>  1 file changed, 16 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
>>> index 1374e38..e1bd558 100644
>>> --- a/sbin/cleanerd/cleanerd.c
>>> +++ b/sbin/cleanerd/cleanerd.c
>>> @@ -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.
>>
>>>  	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);
>>> @@ -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;
>> + 			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.

Sorry I misunderstood you there. Yes it is definitely needed in the else
case of nilfs_cleanerd_clean_loop. In version 5 I moved it to the top of
the loop altogether.

Regards,
Andreas Rohner
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html