Skip to contentSkip to conversation

pp (protection period) mount option

PATCH David Smid 2 participants 7 messages Nov 27, 2009 – Dec 3, 2009 6 messages not in the archive NILFS2 filesystem

lore
Share & tools
lore

David Smid

author
#0 lore raw

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

super.c renamed from super.c.mount_p_opt +7 -1
@@ -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);
mount/mount.nilfs2.h renamed from mount/mount.nilfs2.h.mount_p_opt +1 -0
@@ -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 */
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_p_opt +25 -3
@@ -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);
mount/umount.nilfs2.c renamed from mount/umount.nilfs2.c.mount_p_opt +15 -1
@@ -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,
mount/cleaner_ctl.c renamed from mount/cleaner_ctl.c.mount_p_opt +6 -1
@@ -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;
mount/mount.nilfs2.h renamed from mount/mount.nilfs2.h.mount_p_opt +1 -1
@@ -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);

_______________________________________________
users mailing list
[email protected]
https://www.nilfs.org/mailman/listinfo/users

#2 lore raw
In reply to a message not in the archive

Hi!

Quoted 14 lines · David Smid

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.

Quoted 6 lines

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

author
#4 lore raw
In reply to a message not in the archive

2009/11/27 Ryusuke Konishi <[email protected]>:

Quoted 35 lines

Hi!

Quoted 14 lines · David Smid

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.

Quoted 6 lines

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.

Quoted 9 lines

(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

mount/cleaner_ctl.c renamed from mount/cleaner_ctl.c.mount_pp_opt +2 -1
@@ -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;

mount/mount.nilfs2.h renamed from mount/mount.nilfs2.h.mount_pp_opt +2 -1
@@ -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);
mount/umount.nilfs2.c renamed from mount/umount.nilfs2.c.mount_pp_opt +8 -1
@@ -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,
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +29 -3
@@ -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);
mount.nilfs2.8 renamed from mount.nilfs2.8.mount_pp_opt +6 -0
@@ -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
[email protected]
https://www.nilfs.org/mailman/listinfo/users

#6 lore raw
In reply to a message not in the archive

Hi,

Quoted 6 lines · David Smid

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

#8 lore raw
In reply to a message not in the archive
Quoted 15 lines · Ryusuke Konishi

Hi,

Quoted 6 lines · David Smid

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:

Quoted 7 lines

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

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

Ryusuke

author
#10 lore raw
In reply to a message not in the archive

Ryusuke Konishi napsal(a):

Quoted 13 lines · Ryusuke Konishi
Quoted 12 lines

Hi,

Quoted 6 lines · David Smid

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.

Quoted 7 lines
Quoted 5 lines

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

mount/mount.nilfs2.h renamed from mount/mount.nilfs2.h.mount_pp_opt +2 -1
@@ -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);
mount/cleaner_ctl.c renamed from mount/cleaner_ctl.c.mount_pp_opt +10 -2
@@ -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;
mount/umount.nilfs2.c renamed from mount/umount.nilfs2.c.mount_pp_opt +9 -1
@@ -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,
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +68 -20

185 lines not on this page · raw message

mount.nilfs2.8 renamed from mount.nilfs2.8.mount_pp_opt +6 -0
@@ -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.
#12 lore raw
In reply to a message not in the archive

Hi,

Quoted 20 lines · David Smid

Ryusuke Konishi napsal(a):

Quoted 19 lines · Ryusuke Konishi
Quoted 17 lines

Hi,

Quoted 6 lines · David Smid

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
Quoted 8 lines
Quoted 1 line

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.

Quoted 105 lines
mount/mount.nilfs2.h renamed from mount/mount.nilfs2.h.mount_pp_opt +2 -1
@@ -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);
mount/cleaner_ctl.c renamed from mount/cleaner_ctl.c.mount_pp_opt +10 -2
@@ -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;
mount/umount.nilfs2.c renamed from mount/umount.nilfs2.c.mount_pp_opt +9 -1
@@ -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,
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +3 -0
@@ -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;
 }

mount/mount.nilfs2.c line 239

Quoted 10 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +9 -1
-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.)

Quoted 49 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +22 -1
+	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)) {

mount/mount.nilfs2.c line 397

Quoted 7 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +3 -0
@@ -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.

Quoted 12 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +0 -6
 	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 */

mount/mount.nilfs2.c line 419

Quoted 10 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +7 -3
-		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.

mount/mount.nilfs2.c line 421

Quoted 2 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +0 -0
 		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.

Quoted 33 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +8 -3
 		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 */

mount/mount.nilfs2.c line 476

Quoted 10 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +3 -1
 	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.

Quoted 6 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +0 -0
 			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);

mount/mount.nilfs2.c line 493

Quoted 10 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +2 -1
  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.

mount/mount.nilfs2.c line 499

Quoted 7 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +3 -0
 	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.

Quoted 31 lines
mount/mount.nilfs2.c renamed from mount/mount.nilfs2.c.mount_pp_opt +8 -4
 	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);
mount.nilfs2.8 renamed from mount.nilfs2.8.mount_pp_opt +0 -0
@@ -103,6 +103,12 @@
 remount the file system read-only, or panic and halt the system.)  The
 default is continue.

mount.nilfs2.8 line 114

Quoted 10 lines
mount.nilfs2.8 renamed from mount.nilfs2.8.mount_pp_opt +6 -0
 .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