pp (protection period) mount option

pp (protection period) mount option

[David Smid]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Hi,

I have two nilfs2 partitions and want to set different protection period for
each of them. I know nilfs_cleanerd has an option -p which allows to override
the protection period but using it would require to mount both partitions and
launch their cleaner daemons manually or within a script.
Since I want to use fstab I had to implement this option as a mount.nilfs2 extra
option.

Not much was done to the kernel module: it just had to accept 'pp=' as a valid
mount option.
mount.nilfs2 was modified to parse this option and its value and to pass it to
nilfs_cleanerd command line.

See the patches.


Regards,

David Smid

[-- Attachment #2: nilfs-2.0.18_mount_pp_opt.patch --]
[-- Type: text/x-patch, Size: 758 bytes --]

--- fs/super.c.mount_p_opt	2009-11-22 05:03:04.000000000 +0100
+++ fs/super.c	2009-11-26 16:38:26.000000000 +0100
@@ -650,7 +650,8 @@
 enum {
 	Opt_err_cont, Opt_err_panic, Opt_err_ro,
 	Opt_barrier, Opt_snapshot, Opt_order,
-	Opt_err,
+	Opt_protperiod,
+	Opt_err, 
 };
 
 static match_table_t tokens = {
@@ -660,6 +661,7 @@
 	{Opt_barrier, "barrier=%s"},
 	{Opt_snapshot, "cp=%u"},
 	{Opt_order, "order=%s"},
+	{Opt_protperiod, "pp=%s"},
 	{Opt_err, NULL}
 };
 
@@ -728,6 +730,10 @@
 			sbi->s_snapshot_cno = option;
 			nilfs_set_opt(sbi, SNAPSHOT);
 			break;
+		case Opt_protperiod:
+			if (match_int(&args[0], &option) || option <= 0)
+				return 0;
+			break;
 		default:
 			printk(KERN_ERR
 			       "NILFS: Unrecognized mount option \"%s\"\n", p);

[-- Attachment #3: nilfs-utils-2.0.14_mount_pp_opt.patch --]
[-- Type: text/x-patch, Size: 4932 bytes --]

--- sbin/mount/mount.nilfs2.h.mount_p_opt	2009-11-26 08:04:17.000000000 +0100
+++ sbin/mount/mount.nilfs2.h	2009-11-26 17:23:13.000000000 +0100
@@ -13,6 +13,7 @@
 #define NILFS2_FS_NAME		"nilfs2"
 #define CLEANERD_NAME		"nilfs_cleanerd"
 #define PIDOPT_NAME		"gcpid"
+#define PPOPT_NAME		"pp"
 
 #define CLEANERD_WAIT_RETRY_COUNT	3
 #define CLEANERD_WAIT_RETRY_INTERVAL	2  /* in seconds */
--- sbin/mount/mount.nilfs2.c.mount_p_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/mount.nilfs2.c	2009-11-26 17:36:08.000000000 +0100
@@ -71,6 +71,10 @@
 const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
 typedef int gcpid_opt_t;
 
+const char pp_opt_fmt[] = PPOPT_NAME "=%d";
+const char pp_val_fmt[] = "%d";
+typedef int pp_opt_t;
+
 struct mount_options {
 	char *fstype;
 	char *opts;
@@ -408,6 +412,9 @@
 do_mount_one(struct nilfs_mount_info *mi, const struct mount_options *mo)
 {
 	int res, errsv;
+	int ppval;
+	char ppstrval[20];
+	char *pp;
 
 	res = mount(mi->device, mi->mntdir, fstype, mo->flags & ~MS_NOSYS,
 		    mo->extra_opts);
@@ -429,7 +436,13 @@
 		goto out;
 	if (check_mtab()) {
 		/* Restarting cleaner daemon */
-		if (start_cleanerd(mi->device, mi->mntdir, &mi->gcpid) == 0) {
+		if (find_opt(mo->extra_opts, pp_opt_fmt, &ppval) >= 0) {
+			snprintf(ppstrval, 19, pp_opt_fmt, ppval);
+			pp = ppstrval;
+		}
+		else
+			pp = NULL;
+		if (start_cleanerd(mi->device, mi->mntdir, pp, &mi->gcpid) == 0) {
 			if (verbose)
 				printf(_("%s: restarted %s\n"),
 				       progname, CLEANERD_NAME);
@@ -452,6 +465,9 @@
 	pid_t pid = (mi->type == RW2RW_REMOUNT) ? mi->gcpid : 0;
 	char *exopts;
 	int rungc;
+	int ppval;
+	char ppstrval[20];
+	char *pp;
 
 	rungc = !(mo->flags & MS_RDONLY) && !(mo->flags & MS_BIND) &&
 		(mi->type != RW2RW_REMOUNT || mi->gcpid == 0);
@@ -461,7 +477,14 @@
 		return;
 	}
 	if (rungc) {
-		if (start_cleanerd(mi->device, mi->mntdir, &pid) < 0)
+		if (find_opt(mo->extra_opts, pp_opt_fmt, &ppval) >= 0) {
+			snprintf(ppstrval, 19, pp_val_fmt, ppval);
+			pp = ppstrval;
+		}
+		else
+			pp = NULL;
+
+		if (start_cleanerd(mi->device, mi->mntdir, pp, &pid) < 0)
 			error(_("%s aborted"), CLEANERD_NAME);
 		else if (verbose)
 			printf(_("%s: started %s\n"), progname, CLEANERD_NAME);
@@ -470,7 +493,6 @@
 	exopts = fix_extra_opts_string(mo->extra_opts, pid);
 	mi->optstr = fix_opts_string(((mo->flags & ~MS_NOMTAB) | MS_NETDEV),
 				     exopts, NULL);
-		
 	update_mtab_entry(mi->device, mi->mntdir, fstype, mi->optstr, 0, 0,
 			  !mi->mounted);
 	my_free(exopts);
--- sbin/mount/umount.nilfs2.c.mount_p_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/umount.nilfs2.c	2009-11-26 17:39:03.000000000 +0100
@@ -69,6 +69,10 @@
 const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
 typedef int gcpid_opt_t;
 
+const char pp_opt_fmt[] = PPOPT_NAME "=%d";
+const char pp_val_fmt[] = "%d";
+typedef int pp_opt_t;
+
 struct umount_options {
 	int flags;
 	int force;
@@ -317,6 +321,9 @@
 	int res, alive = 0;
 	const char *loopdev;
 	pid_t pid;
+	int ppval;
+	char ppstrval[20];
+	char *pp;
 
 	if (streq (node, "/") || streq (node, "root"))
 		nomtab++;
@@ -349,7 +356,14 @@
 				      progname, spec);
 			}
 		} else if (alive && !check_cleanerd(spec, pid)) {
-			if (start_cleanerd(spec, node, &pid) == 0) {
+			if (find_opt(mc->m.mnt_opts, pp_opt_fmt, &ppval) >= 0) {
+				snprintf(ppstrval, 19, pp_val_fmt, ppval);
+				pp = ppstrval;
+			}
+			else
+				pp = NULL;
+
+			if (start_cleanerd(spec, node, pp, &pid) == 0) {
 				if (verbose)
 					printf(_("%s: restarted %s(pid=%d)\n"),
 					       progname, CLEANERD_NAME,
--- sbin/mount/cleaner_ctl.c.mount_p_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/cleaner_ctl.c	2009-11-26 08:03:35.000000000 +0100
@@ -45,6 +45,7 @@
 
 const char cleanerd[] = "/sbin/" CLEANERD_NAME;
 const char cleanerd_nofork_opt[] = "-n";
+const char cleanerd_protperiod_opt[] = "-p";
 
 extern char *progname;
 
@@ -54,7 +55,7 @@
 	return (kill(pid, 0) == 0);
 }
 
-int start_cleanerd(const char *device, const char *mntdir, pid_t *ppid)
+int start_cleanerd(const char *device, const char *mntdir, const char *protperiod, pid_t *ppid)
 {
 	const char *dargs[5];
 	struct stat statbuf;
@@ -80,6 +81,10 @@
 		}
 		dargs[i++] = cleanerd;
 		dargs[i++] = cleanerd_nofork_opt;
+		if (protperiod != NULL && strlen(protperiod) > 0) {
+			dargs[i++] = cleanerd_protperiod_opt;
+			dargs[i++] = protperiod;
+		}
 		dargs[i++] = device;
 		dargs[i++] = mntdir;
 		dargs[i] = NULL;
--- sbin/mount/mount.nilfs2.h.mount_p_opt	2009-11-26 17:41:12.000000000 +0100
+++ sbin/mount/mount.nilfs2.h	2009-11-26 17:43:47.000000000 +0100
@@ -19,7 +19,7 @@
 #define CLEANERD_WAIT_RETRY_INTERVAL	2  /* in seconds */
 
 
-extern int start_cleanerd(const char *, const char *, pid_t *);
+extern int start_cleanerd(const char *, const char *, const char *, pid_t *);
 extern int stop_cleanerd(const char *, pid_t);
 extern int check_cleanerd(const char *, pid_t);
 

[-- Attachment #4: Type: text/plain, Size: 158 bytes --]

_______________________________________________
users mailing list
users-JrjvKiOkagjYtjvyW6yDsg@public.gmane.org
https://www.nilfs.org/mailman/listinfo/users

Re: pp (protection period) mount option

[Ryusuke Konishi]

Hi!
On Fri, 27 Nov 2009 15:21:06 +0100, David Smid <david-0RIiUExYzkzCfDggNXIi3w@public.gmane.org> wrote:
> Hi,
> 
> I have two nilfs2 partitions and want to set different protection period for
> each of them. I know nilfs_cleanerd has an option -p which allows to override
> the protection period but using it would require to mount both partitions and
> launch their cleaner daemons manually or within a script.
> Since I want to use fstab I had to implement this option as a mount.nilfs2 extra
> option.
> 
> Not much was done to the kernel module: it just had to accept 'pp=' as a valid
> mount option.
> mount.nilfs2 was modified to parse this option and its value and to pass it to
> nilfs_cleanerd command line.
> 
> See the patches.
> 
> 
> Regards,
> 
> David Smid

Thank you for the patches.
I understood your intention.

One comment.  I don't like the dummy option is added to the kernel
code because protection_period is entirely independent with it.

Can you modify the patch so that mount.nilfs2 gets out the pp option
from the string passed to the mount system call?

A ready-made function, change_opt() would be available for that
purpose.

(only from extra_opts; mtab should include it to keep the value for
cleanerd_ctl, of course)

And, please inline patches in the mail; this makes it easy to comment
on them.

Thanks,
Ryusuke Konishi

Re: pp (protection period) mount option

[David Smid]

2009/11/27 Ryusuke Konishi <ryusuke@osrg.net>:
> Hi!
> On Fri, 27 Nov 2009 15:21:06 +0100, David Smid <david@unity-linux.org> wrote:
>> Hi,
>>
>> I have two nilfs2 partitions and want to set different protection period for
>> each of them. I know nilfs_cleanerd has an option -p which allows to override
>> the protection period but using it would require to mount both partitions and
>> launch their cleaner daemons manually or within a script.
>> Since I want to use fstab I had to implement this option as a mount.nilfs2 extra
>> option.
>>
>> Not much was done to the kernel module: it just had to accept 'pp=' as a valid
>> mount option.
>> mount.nilfs2 was modified to parse this option and its value and to pass it to
>> nilfs_cleanerd command line.
>>
>> See the patches.
>>
>>
>> Regards,
>>
>> David Smid
>
> Thank you for the patches.
> I understood your intention.
>
> One comment.  I don't like the dummy option is added to the kernel
> code because protection_period is entirely independent with it.
>
> Can you modify the patch so that mount.nilfs2 gets out the pp option
> from the string passed to the mount system call?
>
> A ready-made function, change_opt() would be available for that
> purpose.
>
> (only from extra_opts; mtab should include it to keep the value for
> cleanerd_ctl, of course)
>
> And, please inline patches in the mail; this makes it easy to comment
> on them.
>
> Thanks,
> Ryusuke Konishi
>

Thanks you for your valuable comments.

You are right, of course. No kernel code changes are needed.
Here is my take #2.

David


--- sbin/mount/cleaner_ctl.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/cleaner_ctl.c	2009-11-28 21:40:38.000000000 +0100
@@ -45,6 +45,7 @@

 const char cleanerd[] = "/sbin/" CLEANERD_NAME;
 const char cleanerd_nofork_opt[] = "-n";
+const char cleanerd_protperiod_opt[] = "-p";

 extern char *progname;

@@ -54,12 +55,13 @@
 	return (kill(pid, 0) == 0);
 }

-int start_cleanerd(const char *device, const char *mntdir, pid_t *ppid)
+int start_cleanerd(const char *device, const char *mntdir, unsigned
long protperiod, pid_t *ppid)
 {
-	const char *dargs[5];
+	const char *dargs[7];
 	struct stat statbuf;
 	int i = 0;
 	int res;
+	char buf[256];

 	if (stat(cleanerd, &statbuf) != 0) {
 		error(_("Warning: %s not found"), CLEANERD_NAME);
@@ -80,6 +82,11 @@
 		}
 		dargs[i++] = cleanerd;
 		dargs[i++] = cleanerd_nofork_opt;
+		if (protperiod != ULONG_MAX) {
+			dargs[i++] = cleanerd_protperiod_opt;
+			snprintf(buf, sizeof(buf), "%lu", protperiod);
+			dargs[i++] = buf;
+		}
 		dargs[i++] = device;
 		dargs[i++] = mntdir;
 		dargs[i] = NULL;
--- sbin/mount/mount.nilfs2.h.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/mount.nilfs2.h	2009-11-28 20:37:33.000000000 +0100
@@ -13,12 +13,13 @@
 #define NILFS2_FS_NAME		"nilfs2"
 #define CLEANERD_NAME		"nilfs_cleanerd"
 #define PIDOPT_NAME		"gcpid"
+#define PPOPT_NAME		"pp"

 #define CLEANERD_WAIT_RETRY_COUNT	3
 #define CLEANERD_WAIT_RETRY_INTERVAL	2  /* in seconds */


-extern int start_cleanerd(const char *, const char *, pid_t *);
+extern int start_cleanerd(const char *, const char *, unsigned long, pid_t *);
 extern int stop_cleanerd(const char *, pid_t);
 extern int check_cleanerd(const char *, pid_t);

--- sbin/mount/umount.nilfs2.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/umount.nilfs2.c	2009-11-28 21:40:23.000000000 +0100
@@ -69,6 +69,9 @@
 const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
 typedef int gcpid_opt_t;

+const char pp_opt_fmt[] = PPOPT_NAME "=%lu";
+typedef unsigned long pp_opt_t;
+
 struct umount_options {
 	int flags;
 	int force;
@@ -317,6 +320,7 @@
 	int res, alive = 0;
 	const char *loopdev;
 	pid_t pid;
+	pp_opt_t prot_period;

 	if (streq (node, "/") || streq (node, "root"))
 		nomtab++;
@@ -349,7 +353,10 @@
 				      progname, spec);
 			}
 		} else if (alive && !check_cleanerd(spec, pid)) {
-			if (start_cleanerd(spec, node, &pid) == 0) {
+			if (find_opt(mc->m.mnt_opts, pp_opt_fmt, &prot_period) < 0)
+				prot_period = ULONG_MAX;
+
+			if (start_cleanerd(spec, node, prot_period, &pid) == 0) {
 				if (verbose)
 					printf(_("%s: restarted %s(pid=%d)\n"),
 					       progname, CLEANERD_NAME,
--- sbin/mount/mount.nilfs2.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/mount.nilfs2.c	2009-11-28 21:39:27.000000000 +0100
@@ -71,6 +71,9 @@
 const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
 typedef int gcpid_opt_t;

+const char pp_opt_fmt[] = PPOPT_NAME "=%lu";
+typedef unsigned long pp_opt_t;
+
 struct mount_options {
 	char *fstype;
 	char *opts;
@@ -225,6 +228,17 @@
 	*opts = newopts;
 }

+static void update_pp_opt(char **opts, pp_opt_t protection_period)
+{
+	char buf[256], *newopts;
+	pp_opt_t oldpp;
+
+	snprintf(buf, sizeof(buf), pp_opt_fmt, protection_period);
+	newopts = change_opt(*opts, pp_opt_fmt, &oldpp, buf);
+	my_free(*opts);
+	*opts = newopts;
+}
+
 static char *fix_extra_opts_string(const char *extra_opts, pid_t gcpid)
 {
 	char buf[256];
@@ -408,9 +422,14 @@
 do_mount_one(struct nilfs_mount_info *mi, const struct mount_options *mo)
 {
 	int res, errsv;
+	char *exopts;
+	pp_opt_t prot_period;
+
+	prot_period = ULONG_MAX;
+	exopts = change_opt(mo->extra_opts, pp_opt_fmt, &prot_period, "");

 	res = mount(mi->device, mi->mntdir, fstype, mo->flags & ~MS_NOSYS,
-		    mo->extra_opts);
+		    exopts);
 	if (!res)
 		goto out;

@@ -429,11 +448,13 @@
 		goto out;
 	if (check_mtab()) {
 		/* Restarting cleaner daemon */
-		if (start_cleanerd(mi->device, mi->mntdir, &mi->gcpid) == 0) {
+		if (start_cleanerd(mi->device, mi->mntdir, prot_period, &mi->gcpid) == 0) {
 			if (verbose)
 				printf(_("%s: restarted %s\n"),
 				       progname, CLEANERD_NAME);
 			update_gcpid_opt(&mi->optstr, mi->gcpid);
+			if (prot_period != ULONG_MAX)
+				update_pp_opt(&mi->optstr, prot_period);
 			update_mtab_entry(mi->device, mi->mntdir, fstype,
 					  mi->optstr, 0, 0, !mi->mounted);
 		} else {
@@ -443,6 +464,7 @@
 	} else
 		printf(_("%s not restarted\n"), CLEANERD_NAME);
  out:
+	my_free(exopts);
 	return res;
 }

@@ -452,6 +474,7 @@
 	pid_t pid = (mi->type == RW2RW_REMOUNT) ? mi->gcpid : 0;
 	char *exopts;
 	int rungc;
+	pp_opt_t prot_period;

 	rungc = !(mo->flags & MS_RDONLY) && !(mo->flags & MS_BIND) &&
 		(mi->type != RW2RW_REMOUNT || mi->gcpid == 0);
@@ -461,7 +484,10 @@
 		return;
 	}
 	if (rungc) {
-		if (start_cleanerd(mi->device, mi->mntdir, &pid) < 0)
+		if (find_opt(mo->extra_opts, pp_opt_fmt, &prot_period) < 0)
+			prot_period = ULONG_MAX;
+
+		if (start_cleanerd(mi->device, mi->mntdir, prot_period, &pid) < 0)
 			error(_("%s aborted"), CLEANERD_NAME);
 		else if (verbose)
 			printf(_("%s: started %s\n"), progname, CLEANERD_NAME);
--- man/mount.nilfs2.8.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ man/mount.nilfs2.8	2009-11-28 21:51:23.000000000 +0100
@@ -103,6 +103,12 @@
 remount the file system read-only, or panic and halt the system.)  The
 default is continue.
 .TP
+.BR pp=\fP\fIprotection-period\fP
+Specify the \fIprotection-period\fP for the cleaner daemon (in
+seconds). nilfs_cleanerd never deletes recent checkpoints whose
+elapsed time from its creation is smaller than
+\fIprotection-period\fP.
+.TP
 .BR order=relaxed " / " order=strict
 Specify order semantics for file data.  Metadata is always written to
 follow the POSIX semantics about the order of filesystem operations.
_______________________________________________
users mailing list
users@nilfs.org
https://www.nilfs.org/mailman/listinfo/users

Re: pp (protection period) mount option

[Ryusuke Konishi]

Hi,
On Sat, 28 Nov 2009 22:16:31 +0100, David Smid <david-0RIiUExYzkzCfDggNXIi3w@public.gmane.org>
wrote:
> Thanks you for your valuable comments.
> 
> You are right, of course. No kernel code changes are needed.
> Here is my take #2.
> 
> David

Thank you, David.  I reviewed the patch in detail, and no worrisome
point found.

During tests, I noticed that remount doesn't work for this option:

  For instance, after doing

  # mount -t nilfs2 -o pp=3D600 /dev/sdb1 /test

  and

  # mount -t nilfs2 -o remount,pp=3D400 /dev/sdb1 /test

  then, a cleanerd with "-p 600" was still working.

I think usually this doesn't make trouble, but I'd like to hear your
opinion before merging.  If you want to revise the issue to keep
semantics of remount option, I will wait for you.

If you don't care, I will merge the patch as is.

Thanks,
Ryusuke Konishi

Re: pp (protection period) mount option

[Ryusuke Konishi]

On Wed, 02 Dec 2009 00:44:22 +0900 (JST), Ryusuke Konishi <ryusuke-sG5X7nlA6pw@public.gmane.org> wrote:
> Hi,
> On Sat, 28 Nov 2009 22:16:31 +0100, David Smid <david-0RIiUExYzkzCfDggNXIi3w@public.gmane.org>
> wrote:
> > Thanks you for your valuable comments.
> > 
> > You are right, of course. No kernel code changes are needed.
> > Here is my take #2.
> > 
> > David
> 
> Thank you, David.  I reviewed the patch in detail, and no worrisome
> point found.
> 
> During tests, I noticed that remount doesn't work for this option:
> 
>   For instance, after doing
> 
>   # mount -t nilfs2 -o pp=3D600 /dev/sdb1 /test
> 
>   and
> 
>   # mount -t nilfs2 -o remount,pp=3D400 /dev/sdb1 /test

Whoa, looks garbled..

>   # mount -t nilfs2 -o pp=600 /dev/sdb1 /test
>   # mount -t nilfs2 -o remount,pp=400 /dev/sdb1 /test

Ryusuke

Re: pp (protection period) mount option

[David Smid]

Ryusuke Konishi napsal(a):
> On Wed, 02 Dec 2009 00:44:22 +0900 (JST), Ryusuke Konishi <ryusuke-sG5X7nlA6pw@public.gmane.org> wrote:
>> Hi,
>> On Sat, 28 Nov 2009 22:16:31 +0100, David Smid <david-0RIiUExYzkzCfDggNXIi3w@public.gmane.org>
>> wrote:
>>> Thanks you for your valuable comments.
>>>
>>> You are right, of course. No kernel code changes are needed.
>>> Here is my take #2.
>>>
>>> David
>> Thank you, David.  I reviewed the patch in detail, and no worrisome
>> point found.
>>
>> During tests, I noticed that remount doesn't work for this option:
>>
>>   For instance, after doing
>>   # mount -t nilfs2 -o pp=600 /dev/sdb1 /test
>>   # mount -t nilfs2 -o remount,pp=400 /dev/sdb1 /test
> 
> Ryusuke

Good point, remount should definitely work because it is quite useful to change
pp on the fly.

This patch should work.

David



--- sbin/mount/mount.nilfs2.h.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/mount.nilfs2.h	2009-12-02 10:27:26.000000000 +0100
@@ -13,12 +13,13 @@
 #define NILFS2_FS_NAME		"nilfs2"
 #define CLEANERD_NAME		"nilfs_cleanerd"
 #define PIDOPT_NAME		"gcpid"
+#define PPOPT_NAME		"pp"

 #define CLEANERD_WAIT_RETRY_COUNT	3
 #define CLEANERD_WAIT_RETRY_INTERVAL	2  /* in seconds */


-extern int start_cleanerd(const char *, const char *, pid_t *);
+extern int start_cleanerd(const char *, const char *, unsigned long, pid_t *);
 extern int stop_cleanerd(const char *, pid_t);
 extern int check_cleanerd(const char *, pid_t);

--- sbin/mount/cleaner_ctl.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/cleaner_ctl.c	2009-12-02 18:16:16.000000000 +0100
@@ -45,6 +45,7 @@

 const char cleanerd[] = "/sbin/" CLEANERD_NAME;
 const char cleanerd_nofork_opt[] = "-n";
+const char cleanerd_protperiod_opt[] = "-p";

 extern char *progname;

@@ -54,12 +55,14 @@
 	return (kill(pid, 0) == 0);
 }

-int start_cleanerd(const char *device, const char *mntdir, pid_t *ppid)
+int start_cleanerd(const char *device, const char *mntdir,
+		   unsigned long protperiod, pid_t *ppid)
 {
-	const char *dargs[5];
+	const char *dargs[7];
 	struct stat statbuf;
 	int i = 0;
 	int res;
+	char buf[256];

 	if (stat(cleanerd, &statbuf) != 0) {
 		error(_("Warning: %s not found"), CLEANERD_NAME);
@@ -80,6 +83,11 @@
 		}
 		dargs[i++] = cleanerd;
 		dargs[i++] = cleanerd_nofork_opt;
+		if (protperiod != ULONG_MAX) {
+			dargs[i++] = cleanerd_protperiod_opt;
+			snprintf(buf, sizeof(buf), "%lu", protperiod);
+			dargs[i++] = buf;
+		}
 		dargs[i++] = device;
 		dargs[i++] = mntdir;
 		dargs[i] = NULL;
--- sbin/mount/umount.nilfs2.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/umount.nilfs2.c	2009-12-02 18:15:37.000000000 +0100
@@ -69,6 +69,9 @@
 const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
 typedef int gcpid_opt_t;

+const char pp_opt_fmt[] = PPOPT_NAME "=%lu";
+typedef unsigned long pp_opt_t;
+
 struct umount_options {
 	int flags;
 	int force;
@@ -317,6 +320,7 @@
 	int res, alive = 0;
 	const char *loopdev;
 	pid_t pid;
+	pp_opt_t prot_period;

 	if (streq (node, "/") || streq (node, "root"))
 		nomtab++;
@@ -349,7 +353,11 @@
 				      progname, spec);
 			}
 		} else if (alive && !check_cleanerd(spec, pid)) {
-			if (start_cleanerd(spec, node, &pid) == 0) {
+			if (find_opt(mc->m.mnt_opts, pp_opt_fmt, &prot_period)
+			    < 0)
+				prot_period = ULONG_MAX;
+
+			if (start_cleanerd(spec, node, prot_period, &pid) == 0) {
 				if (verbose)
 					printf(_("%s: restarted %s(pid=%d)\n"),
 					       progname, CLEANERD_NAME,
--- sbin/mount/mount.nilfs2.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ sbin/mount/mount.nilfs2.c	2009-12-02 18:14:50.000000000 +0100
@@ -71,6 +71,9 @@
 const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
 typedef int gcpid_opt_t;

+const char pp_opt_fmt[] = PPOPT_NAME "=%lu";
+typedef unsigned long pp_opt_t;
+
 struct mount_options {
 	char *fstype;
 	char *opts;
@@ -225,16 +228,43 @@
 	*opts = newopts;
 }

-static char *fix_extra_opts_string(const char *extra_opts, pid_t gcpid)
+static void update_pp_opt(char **opts, pp_opt_t protection_period)
+{
+	char buf[256], *newopts;
+	pp_opt_t oldpp;
+
+	*buf = 0;
+	if (protection_period != ULONG_MAX) {
+		snprintf(buf, sizeof(buf), pp_opt_fmt, protection_period);
+	}
+	newopts = change_opt(*opts, pp_opt_fmt, &oldpp, buf);
+	my_free(*opts);
+	*opts = newopts;
+}
+
+static char *fix_extra_opts_string(const char *extra_opts, pid_t gcpid,
+				   pp_opt_t protection_period)
 {
 	char buf[256];
 	gcpid_opt_t id;
+	pp_opt_t pp;
+	char *tmpres;
+	char *res;

 	buf[0] = '\0';
 	if (gcpid)
 		snprintf(buf, sizeof(buf), gcpid_opt_fmt, (int)gcpid);
 	/* The gcpid option will be removed if gcpid == 0 */
-	return change_opt(extra_opts, gcpid_opt_fmt, &id, buf);
+	tmpres = change_opt(extra_opts, gcpid_opt_fmt, &id, buf);
+	
+	buf[0] = '\0';
+	if (protection_period != ULONG_MAX)
+		snprintf(buf, sizeof(buf), pp_opt_fmt, protection_period);
+	/* The pp option will be removed if pp == ULONG_MAX */
+	res = change_opt(tmpres, pp_opt_fmt, &pp, buf);
+
+	my_free(tmpres);
+	return res;
 }

 /*
@@ -312,6 +342,7 @@
 	pid_t gcpid;
 	int type;
 	int mounted;
+	pp_opt_t protperiod;
 };

 static int check_mtab(void)
@@ -348,6 +379,7 @@
 {
 	struct mntentchn *mc;
 	gcpid_opt_t pid;
+	pp_opt_t prot_period;
 	int res = -1;

 	if (!(mo->flags & MS_REMOUNT) && mounted(NULL, mi->mntdir)) {
@@ -359,6 +391,9 @@
 	mi->gcpid = 0;
 	mi->optstr = NULL;
 	mi->mounted = mounted(mi->device, mi->mntdir);
+	mi->protperiod = ULONG_MAX;
+	if (find_opt(mo->extra_opts, pp_opt_fmt, &prot_period) >= 0)
+		mi->protperiod = prot_period;

 	if (mo->flags & MS_BIND)
 		return 0;
@@ -375,15 +410,13 @@
 		goto failed;
 	case MS_RDONLY: /* ro-mount (a rw-mount exists) */
 		break;
-	case MS_REMOUNT: /* rw->rw remount */
-		if (check_remount_dir(mc, mi->mntdir) < 0)
-			goto failed;
-		if (find_opt(mc->m.mnt_opts, gcpid_opt_fmt, &pid) >= 0)
-			mi->gcpid = pid;
-		mi->optstr = xstrdup(mc->m.mnt_opts); /* previous opts */
-		mi->type = RW2RW_REMOUNT;
-		break;
-	case MS_RDONLY | MS_REMOUNT:  /* rw->ro remount */
+	case MS_REMOUNT | MS_RDONLY: /* rw->ro remount */
+		mi->type = RW2RO_REMOUNT;
+		mi->protperiod = ULONG_MAX;
+		/* fallthrough */
+	case MS_REMOUNT:
+		if (!(mo->flags & MS_RDONLY))
+			mi->type = RW2RW_REMOUNT; /* rw->rw remount */			
 		if (check_remount_dir(mc, mi->mntdir) < 0)
 			goto failed;
 		pid = 0;
@@ -395,7 +428,6 @@
 		}
 		mi->gcpid = pid;
 		mi->optstr = xstrdup(mc->m.mnt_opts); /* previous opts */
-		mi->type = RW2RO_REMOUNT;
 		break;
 	}

@@ -408,9 +440,13 @@
 do_mount_one(struct nilfs_mount_info *mi, const struct mount_options *mo)
 {
 	int res, errsv;
+	char *exopts;
+	pp_opt_t prot_period;
+
+	exopts = change_opt(mo->extra_opts, pp_opt_fmt, &prot_period, "");

 	res = mount(mi->device, mi->mntdir, fstype, mo->flags & ~MS_NOSYS,
-		    mo->extra_opts);
+		    exopts);
 	if (!res)
 		goto out;

@@ -425,15 +461,19 @@
 		      progname, mi->device, mi->mntdir, strerror(errsv));
 		break;
 	}
-	if (mi->type != RW2RO_REMOUNT)
+	if (mi->type != RW2RO_REMOUNT && mi->type != RW2RW_REMOUNT)
 		goto out;
+	/* Cleaner daemon was stopped and it needs to run */
+	/* because filesystem is still mounted */
 	if (check_mtab()) {
 		/* Restarting cleaner daemon */
-		if (start_cleanerd(mi->device, mi->mntdir, &mi->gcpid) == 0) {
+		if (start_cleanerd(mi->device, mi->mntdir, mi->protperiod,
+				   &mi->gcpid) == 0) {
 			if (verbose)
 				printf(_("%s: restarted %s\n"),
 				       progname, CLEANERD_NAME);
 			update_gcpid_opt(&mi->optstr, mi->gcpid);
+			update_pp_opt(&mi->optstr, mi->protperiod);
 			update_mtab_entry(mi->device, mi->mntdir, fstype,
 					  mi->optstr, 0, 0, !mi->mounted);
 		} else {
@@ -443,31 +483,39 @@
 	} else
 		printf(_("%s not restarted\n"), CLEANERD_NAME);
  out:
+	my_free(exopts);
 	return res;
 }

 static void update_mount_state(struct nilfs_mount_info *mi,
 			       const struct mount_options *mo)
 {
-	pid_t pid = (mi->type == RW2RW_REMOUNT) ? mi->gcpid : 0;
+	pid_t pid = fake ? mi->gcpid : 0;
 	char *exopts;
 	int rungc;

+	if (mo->flags & MS_RDONLY)
+		mi->protperiod = ULONG_MAX;
+
 	rungc = !(mo->flags & MS_RDONLY) && !(mo->flags & MS_BIND) &&
-		(mi->type != RW2RW_REMOUNT || mi->gcpid == 0);
+		(pid == 0);
 	if (!check_mtab()) {
 		if (rungc)
 			printf(_("%s not started\n"), CLEANERD_NAME);
 		return;
 	}
 	if (rungc) {
-		if (start_cleanerd(mi->device, mi->mntdir, &pid) < 0)
+		if (start_cleanerd(mi->device, mi->mntdir, mi->protperiod,
+				   &pid) < 0)
 			error(_("%s aborted"), CLEANERD_NAME);
 		else if (verbose)
 			printf(_("%s: started %s\n"), progname, CLEANERD_NAME);
-	}
+	} else
+		if (!fake)
+			pid = 0;
+	
 	my_free(mi->optstr);
-	exopts = fix_extra_opts_string(mo->extra_opts, pid);
+	exopts = fix_extra_opts_string(mo->extra_opts, pid, mi->protperiod);
 	mi->optstr = fix_opts_string(((mo->flags & ~MS_NOMTAB) | MS_NETDEV),
 				     exopts, NULL);
 		
--- man/mount.nilfs2.8.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
+++ man/mount.nilfs2.8	2009-12-02 10:27:26.000000000 +0100
@@ -103,6 +103,12 @@
 remount the file system read-only, or panic and halt the system.)  The
 default is continue.
 .TP
+.BR pp=\fP\fIprotection-period\fP
+Specify the \fIprotection-period\fP for the cleaner daemon (in
+seconds). nilfs_cleanerd never deletes recent checkpoints whose
+elapsed time from its creation is smaller than
+\fIprotection-period\fP.
+.TP
 .BR order=relaxed " / " order=strict
 Specify order semantics for file data.  Metadata is always written to
 follow the POSIX semantics about the order of filesystem operations.

Re: pp (protection period) mount option

[Ryusuke Konishi]

Hi,
On Wed, 02 Dec 2009 18:20:10 +0100, David Smid <david-0RIiUExYzkzCfDggNXIi3w@public.gmane.org> wrote:
> Ryusuke Konishi napsal(a):
> > On Wed, 02 Dec 2009 00:44:22 +0900 (JST), Ryusuke Konishi <ryusuke-sG5X7nlA6pw@public.gmane.org> wrote:
> >> Hi,
> >> On Sat, 28 Nov 2009 22:16:31 +0100, David Smid <david-0RIiUExYzkzCfDggNXIi3w@public.gmane.org>
> >> wrote:
> >>> Thanks you for your valuable comments.
> >>>
> >>> You are right, of course. No kernel code changes are needed.
> >>> Here is my take #2.
> >>>
> >>> David
> >> Thank you, David.  I reviewed the patch in detail, and no worrisome
> >> point found.
> >>
> >> During tests, I noticed that remount doesn't work for this option:
> >>
> >>   For instance, after doing
> >>   # mount -t nilfs2 -o pp=600 /dev/sdb1 /test
> >>   # mount -t nilfs2 -o remount,pp=400 /dev/sdb1 /test
> > 
> > Ryusuke
> 
> Good point, remount should definitely work because it is quite useful to change
> pp on the fly.
> 
> This patch should work.
> 
> David

Thank you for revising the patch.  I reviewed the code anew.

I will comment inline.

> --- sbin/mount/mount.nilfs2.h.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
> +++ sbin/mount/mount.nilfs2.h	2009-12-02 10:27:26.000000000 +0100
> @@ -13,12 +13,13 @@
>  #define NILFS2_FS_NAME		"nilfs2"
>  #define CLEANERD_NAME		"nilfs_cleanerd"
>  #define PIDOPT_NAME		"gcpid"
> +#define PPOPT_NAME		"pp"
> 
>  #define CLEANERD_WAIT_RETRY_COUNT	3
>  #define CLEANERD_WAIT_RETRY_INTERVAL	2  /* in seconds */
> 
> 
> -extern int start_cleanerd(const char *, const char *, pid_t *);
> +extern int start_cleanerd(const char *, const char *, unsigned long, pid_t *);
>  extern int stop_cleanerd(const char *, pid_t);
>  extern int check_cleanerd(const char *, pid_t);
> 
> --- sbin/mount/cleaner_ctl.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
> +++ sbin/mount/cleaner_ctl.c	2009-12-02 18:16:16.000000000 +0100
> @@ -45,6 +45,7 @@
> 
>  const char cleanerd[] = "/sbin/" CLEANERD_NAME;
>  const char cleanerd_nofork_opt[] = "-n";
> +const char cleanerd_protperiod_opt[] = "-p";
> 
>  extern char *progname;
> 
> @@ -54,12 +55,14 @@
>  	return (kill(pid, 0) == 0);
>  }
> 
> -int start_cleanerd(const char *device, const char *mntdir, pid_t *ppid)
> +int start_cleanerd(const char *device, const char *mntdir,
> +		   unsigned long protperiod, pid_t *ppid)
>  {
> -	const char *dargs[5];
> +	const char *dargs[7];
>  	struct stat statbuf;
>  	int i = 0;
>  	int res;
> +	char buf[256];
> 
>  	if (stat(cleanerd, &statbuf) != 0) {
>  		error(_("Warning: %s not found"), CLEANERD_NAME);
> @@ -80,6 +83,11 @@
>  		}
>  		dargs[i++] = cleanerd;
>  		dargs[i++] = cleanerd_nofork_opt;
> +		if (protperiod != ULONG_MAX) {
> +			dargs[i++] = cleanerd_protperiod_opt;
> +			snprintf(buf, sizeof(buf), "%lu", protperiod);
> +			dargs[i++] = buf;
> +		}
>  		dargs[i++] = device;
>  		dargs[i++] = mntdir;
>  		dargs[i] = NULL;
> --- sbin/mount/umount.nilfs2.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
> +++ sbin/mount/umount.nilfs2.c	2009-12-02 18:15:37.000000000 +0100
> @@ -69,6 +69,9 @@
>  const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
>  typedef int gcpid_opt_t;
> 
> +const char pp_opt_fmt[] = PPOPT_NAME "=%lu";
> +typedef unsigned long pp_opt_t;
> +
>  struct umount_options {
>  	int flags;
>  	int force;
> @@ -317,6 +320,7 @@
>  	int res, alive = 0;
>  	const char *loopdev;
>  	pid_t pid;
> +	pp_opt_t prot_period;
> 
>  	if (streq (node, "/") || streq (node, "root"))
>  		nomtab++;
> @@ -349,7 +353,11 @@
>  				      progname, spec);
>  			}
>  		} else if (alive && !check_cleanerd(spec, pid)) {
> -			if (start_cleanerd(spec, node, &pid) == 0) {
> +			if (find_opt(mc->m.mnt_opts, pp_opt_fmt, &prot_period)
> +			    < 0)
> +				prot_period = ULONG_MAX;
> +
> +			if (start_cleanerd(spec, node, prot_period, &pid) == 0) {
>  				if (verbose)
>  					printf(_("%s: restarted %s(pid=%d)\n"),
>  					       progname, CLEANERD_NAME,
> --- sbin/mount/mount.nilfs2.c.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
> +++ sbin/mount/mount.nilfs2.c	2009-12-02 18:14:50.000000000 +0100
> @@ -71,6 +71,9 @@
>  const char gcpid_opt_fmt[] = PIDOPT_NAME "=%d";
>  typedef int gcpid_opt_t;
> 
> +const char pp_opt_fmt[] = PPOPT_NAME "=%lu";
> +typedef unsigned long pp_opt_t;
> +
>  struct mount_options {
>  	char *fstype;
>  	char *opts;
> @@ -225,16 +228,43 @@
>  	*opts = newopts;
>  }
> 
> -static char *fix_extra_opts_string(const char *extra_opts, pid_t gcpid)
> +static void update_pp_opt(char **opts, pp_opt_t protection_period)
> +{
> +	char buf[256], *newopts;
> +	pp_opt_t oldpp;
> +
> +	*buf = 0;
> +	if (protection_period != ULONG_MAX) {
> +		snprintf(buf, sizeof(buf), pp_opt_fmt, protection_period);
> +	}

In case of the kernel code, these braces (i.e '{' and '}') are removed
because it's a single statement.  (for your information.  This is not
important.)

> +	newopts = change_opt(*opts, pp_opt_fmt, &oldpp, buf);
> +	my_free(*opts);
> +	*opts = newopts;
> +}
> +
> +static char *fix_extra_opts_string(const char *extra_opts, pid_t gcpid,
> +				   pp_opt_t protection_period)
>  {
>  	char buf[256];
>  	gcpid_opt_t id;
> +	pp_opt_t pp;
> +	char *tmpres;
> +	char *res;
> 
>  	buf[0] = '\0';
>  	if (gcpid)
>  		snprintf(buf, sizeof(buf), gcpid_opt_fmt, (int)gcpid);
>  	/* The gcpid option will be removed if gcpid == 0 */
> -	return change_opt(extra_opts, gcpid_opt_fmt, &id, buf);
> +	tmpres = change_opt(extra_opts, gcpid_opt_fmt, &id, buf);
> +	
> +	buf[0] = '\0';
> +	if (protection_period != ULONG_MAX)
> +		snprintf(buf, sizeof(buf), pp_opt_fmt, protection_period);
> +	/* The pp option will be removed if pp == ULONG_MAX */
> +	res = change_opt(tmpres, pp_opt_fmt, &pp, buf);
> +
> +	my_free(tmpres);
> +	return res;
>  }
> 
>  /*
> @@ -312,6 +342,7 @@
>  	pid_t gcpid;
>  	int type;
>  	int mounted;
> +	pp_opt_t protperiod;
>  };
> 
>  static int check_mtab(void)
> @@ -348,6 +379,7 @@
>  {
>  	struct mntentchn *mc;
>  	gcpid_opt_t pid;
> +	pp_opt_t prot_period;
>  	int res = -1;
> 
>  	if (!(mo->flags & MS_REMOUNT) && mounted(NULL, mi->mntdir)) {
> @@ -359,6 +391,9 @@
>  	mi->gcpid = 0;
>  	mi->optstr = NULL;
>  	mi->mounted = mounted(mi->device, mi->mntdir);
> +	mi->protperiod = ULONG_MAX;

> +	if (find_opt(mo->extra_opts, pp_opt_fmt, &prot_period) >= 0)
> +		mi->protperiod = prot_period;

These two lines should be removed because prepare_mount() is the
function to find an existing mount and extract its options from mtab.
mo->extra_opts has new mount options.

Please see the code I add below.
 
>  	if (mo->flags & MS_BIND)
>  		return 0;
> @@ -375,15 +410,13 @@
>  		goto failed;
>  	case MS_RDONLY: /* ro-mount (a rw-mount exists) */
>  		break;
> -	case MS_REMOUNT: /* rw->rw remount */
> -		if (check_remount_dir(mc, mi->mntdir) < 0)
> -			goto failed;
> -		if (find_opt(mc->m.mnt_opts, gcpid_opt_fmt, &pid) >= 0)
> -			mi->gcpid = pid;
> -		mi->optstr = xstrdup(mc->m.mnt_opts); /* previous opts */
> -		mi->type = RW2RW_REMOUNT;
> -		break;
> -	case MS_RDONLY | MS_REMOUNT:  /* rw->ro remount */
> +	case MS_REMOUNT | MS_RDONLY: /* rw->ro remount */
> +		mi->type = RW2RO_REMOUNT;
> +		mi->protperiod = ULONG_MAX;
> +		/* fallthrough */
> +	case MS_REMOUNT:
> +		if (!(mo->flags & MS_RDONLY))
> +			mi->type = RW2RW_REMOUNT; /* rw->rw remount */			
Trailing space chars. should be removed.
>  		if (check_remount_dir(mc, mi->mntdir) < 0)
>  			goto failed;

Here, can get the existing pp value:

		prot_period = ULONG_MAX;
		if (find_opt(mc->m.mnt_opts, pp_opt_fmt, &prot_period) >= 0)
			mi->protperiod = prot_period;

This previous value is used in do_mount_one(), for example, when
remount fails and it restarts cleanerd.

>  		pid = 0;
> @@ -395,7 +428,6 @@
>  		}
>  		mi->gcpid = pid;
>  		mi->optstr = xstrdup(mc->m.mnt_opts); /* previous opts */
> -		mi->type = RW2RO_REMOUNT;
>  		break;
>  	}
> 
> @@ -408,9 +440,13 @@
>  do_mount_one(struct nilfs_mount_info *mi, const struct mount_options *mo)
>  {
>  	int res, errsv;
> +	char *exopts;
> +	pp_opt_t prot_period;
> +
> +	exopts = change_opt(mo->extra_opts, pp_opt_fmt, &prot_period, "");
> 
>  	res = mount(mi->device, mi->mntdir, fstype, mo->flags & ~MS_NOSYS,
> -		    mo->extra_opts);
> +		    exopts);
>  	if (!res)
>  		goto out;
> 
> @@ -425,15 +461,19 @@
>  		      progname, mi->device, mi->mntdir, strerror(errsv));
>  		break;
>  	}
> -	if (mi->type != RW2RO_REMOUNT)
> +	if (mi->type != RW2RO_REMOUNT && mi->type != RW2RW_REMOUNT)
>  		goto out;
> +	/* Cleaner daemon was stopped and it needs to run */
> +	/* because filesystem is still mounted */
>  	if (check_mtab()) {
>  		/* Restarting cleaner daemon */
> -		if (start_cleanerd(mi->device, mi->mntdir, &mi->gcpid) == 0) {
> +		if (start_cleanerd(mi->device, mi->mntdir, mi->protperiod,
> +				   &mi->gcpid) == 0) {
>  			if (verbose)
>  				printf(_("%s: restarted %s\n"),
>  				       progname, CLEANERD_NAME);
>  			update_gcpid_opt(&mi->optstr, mi->gcpid);
> +			update_pp_opt(&mi->optstr, mi->protperiod);

This update_pp_opt() is redundant because this is an error path and
the pp option should keep the previous value.  On the other hand,
update_gcpid_opt() is necessary since GC pid is always changed after
the restart.

>  			update_mtab_entry(mi->device, mi->mntdir, fstype,
>  					  mi->optstr, 0, 0, !mi->mounted);
>  		} else {
> @@ -443,31 +483,39 @@
>  	} else
>  		printf(_("%s not restarted\n"), CLEANERD_NAME);
>   out:
> +	my_free(exopts);
>  	return res;
>  }
> 
>  static void update_mount_state(struct nilfs_mount_info *mi,
>  			       const struct mount_options *mo)
>  {
> -	pid_t pid = (mi->type == RW2RW_REMOUNT) ? mi->gcpid : 0;
> +	pid_t pid = fake ? mi->gcpid : 0;

Right.  The old gcpid will break except for the fake mode.

>  	char *exopts;
>  	int rungc;
> 

> +	if (mo->flags & MS_RDONLY)
> +		mi->protperiod = ULONG_MAX;
> +

New pp option can be parsed here.

	if ((mo->flags & MS_RDONLY) || (mo->flags & MS_BIND) ||
	    find_opt(mo->extra_opts, pp_opt_fmt, &prot_period) < 0)
		mi->protperiod = ULONG_MAX;
	else
		mi->protperiod = prot_period;

Note that we don't invoke cleanerd for a bind mount.

>  	rungc = !(mo->flags & MS_RDONLY) && !(mo->flags & MS_BIND) &&
> -		(mi->type != RW2RW_REMOUNT || mi->gcpid == 0);
> +		(pid == 0);
>  	if (!check_mtab()) {
>  		if (rungc)
>  			printf(_("%s not started\n"), CLEANERD_NAME);
>  		return;
>  	}
>  	if (rungc) {
> -		if (start_cleanerd(mi->device, mi->mntdir, &pid) < 0)
> +		if (start_cleanerd(mi->device, mi->mntdir, mi->protperiod,
> +				   &pid) < 0)
>  			error(_("%s aborted"), CLEANERD_NAME);
>  		else if (verbose)
>  			printf(_("%s: started %s\n"), progname, CLEANERD_NAME);
> -	}
> +	} else
> +		if (!fake)
> +			pid = 0;
> +	
>  	my_free(mi->optstr);
> -	exopts = fix_extra_opts_string(mo->extra_opts, pid);
> +	exopts = fix_extra_opts_string(mo->extra_opts, pid, mi->protperiod);
>  	mi->optstr = fix_opts_string(((mo->flags & ~MS_NOMTAB) | MS_NETDEV),
>  				     exopts, NULL);
>  		
> --- man/mount.nilfs2.8.mount_pp_opt	2009-07-19 16:53:00.000000000 +0200
> +++ man/mount.nilfs2.8	2009-12-02 10:27:26.000000000 +0100
> @@ -103,6 +103,12 @@
>  remount the file system read-only, or panic and halt the system.)  The
>  default is continue.
>  .TP
> +.BR pp=\fP\fIprotection-period\fP
> +Specify the \fIprotection-period\fP for the cleaner daemon (in
> +seconds). nilfs_cleanerd never deletes recent checkpoints whose
> +elapsed time from its creation is smaller than
> +\fIprotection-period\fP.
> +.TP
>  .BR order=relaxed " / " order=strict
>  Specify order semantics for file data.  Metadata is always written to
>  follow the POSIX semantics about the order of filesystem operations.


Thanks,
Ryusuke Konishi