[PATCH 00/17][cr][v4]: C/R file owner, locks, leases

[PATCH 00/17][cr][v4]: C/R file owner, locks, leases

[Sukadev Bhattiprolu]

Checkpoint/restart file owner, file-locks and file-lease information.

See Documentation/checkpoint/file-locks added by patch 17/17 for design
and file-system consistency requirements.

Changelog[v4]:

	- [Oren Laadan, Steven Whitehouse]: Document design and fs consistency
	  requirements.
	- [Oren Laadan]: For consistency with other such objects, replace
	  the "marker lock" checkpoint with a checkpoint of a count of the
	  file-locks before the first file-lock of each file.

Changelog[v3]:
	- Broke-up C/R of file-leases patches into smaller patches and included
	  them in this set.
	- Addressed comments from Jamie Lokier, Oren Laadan with help from
	  John Stultz on the computation of time offsets.


Sukadev Bhattiprolu (17):
  Add uid, euid params to f_modown()
  Add uid, euid params to __f_setown()
  Checkpoint file-owner information
  Restore file_owner info
  Move file_lock macros into linux/fs.h
  Checkpoint file-locks
  Define flock_set()
  Define flock64_set()
  Restore file-locks
  Initialize ->fl_break_time to 0
  Add ->fl_type_prev field.
  Add ->fl_break_notified field.
  Add jiffies_begin field to ckpt_ctx
  Checkpoint file-leases
  Define do_setlease()
  Restore file-leases
  Document design of C/R of file-locks and leases

 Documentation/checkpoint/file-locks |  126 ++++++++++
 drivers/char/tty_io.c               |    3 +-
 drivers/net/tun.c                   |    3 +-
 fs/checkpoint.c                     |  468 ++++++++++++++++++++++++++++++++---
 fs/fcntl.c                          |   19 +-
 fs/locks.c                          |  207 ++++++++++++----
 fs/notify/dnotify/dnotify.c         |    3 +-
 include/linux/checkpoint_hdr.h      |   25 ++
 include/linux/checkpoint_types.h    |    1 +
 include/linux/fs.h                  |   17 ++-
 kernel/checkpoint/sys.c             |    1 +
 11 files changed, 779 insertions(+), 94 deletions(-)
 create mode 100644 Documentation/checkpoint/file-locks

NOTE: Most of the "added lines" in fs/locks.c are comments about C/R :-)

[PATCH 01/17][cr][v4]: Add uid, euid params to f_modown()

[Sukadev Bhattiprolu]

Checkpoint/restart of file-owner.

Add uid, euid parameters to f_modown(). These parameters will be needed
when restarting an application (and hence restoring the file information),
from a checkpoint image.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/fcntl.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 2079af0..aeab1f4 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -197,7 +197,7 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
 }
 
 static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
-                     int force)
+                     uid_t uid, uid_t euid, int force)
 {
 	write_lock_irq(&filp->f_owner.lock);
 	if (force || !filp->f_owner.pid) {
@@ -206,9 +206,8 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
 		filp->f_owner.pid_type = type;
 
 		if (pid) {
-			const struct cred *cred = current_cred();
-			filp->f_owner.uid = cred->uid;
-			filp->f_owner.euid = cred->euid;
+			filp->f_owner.uid = uid;
+			filp->f_owner.euid = euid;
 		}
 	}
 	write_unlock_irq(&filp->f_owner.lock);
@@ -223,7 +222,7 @@ int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
 	if (err)
 		return err;
 
-	f_modown(filp, pid, type, force);
+	f_modown(filp, pid, type, current_uid(), current_euid(), force);
 	return 0;
 }
 EXPORT_SYMBOL(__f_setown);
@@ -249,7 +248,7 @@ EXPORT_SYMBOL(f_setown);
 
 void f_delown(struct file *filp)
 {
-	f_modown(filp, NULL, PIDTYPE_PID, 1);
+	f_modown(filp, NULL, PIDTYPE_PID, current_uid(), current_euid(), 1);
 }
 
 pid_t f_getown(struct file *filp)
-- 
1.6.0.4

[PATCH 02/17][cr][v4]: Add uid, euid params to __f_setown()

[Sukadev Bhattiprolu]

Instead of letting __f_setown() use the UID and EUID of the calling process,
pass them in as parameters.

This modified interface will be useful when checkpointing and restarting an
application that has a 'file owner' specified for an open file.  When
checkpointing the application, the UID and EUID of the process setting up
the owner are saved in the checkpoint image.

When the application is restarted, we use the UID and EUID values saved in
the checkpoint-image, rather than that of the calling process.

Changelog[v2]:
	- [Matthew Wilcox] Rather than a new __f_setown_uid() interface add
	  the uid parameters to __f_setown() itself.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 drivers/char/tty_io.c       |    3 ++-
 drivers/net/tun.c           |    3 ++-
 fs/fcntl.c                  |   10 ++++++----
 fs/locks.c                  |    3 ++-
 fs/notify/dnotify/dnotify.c |    3 ++-
 include/linux/fs.h          |    3 ++-
 6 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index 115c8e4..9c282b2 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1969,7 +1969,8 @@ static int tty_fasync(int fd, struct file *filp, int on)
 		}
 		get_pid(pid);
 		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
-		retval = __f_setown(filp, pid, type, 0);
+		retval = __f_setown(filp, pid, type, current_uid(),
+				current_euid(), 0);
 		put_pid(pid);
 		if (retval)
 			goto out;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 4326520..dcbc37d 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1400,7 +1400,8 @@ static int tun_chr_fasync(int fd, struct file *file, int on)
 		goto out;
 
 	if (on) {
-		ret = __f_setown(file, task_pid(current), PIDTYPE_PID, 0);
+		ret = __f_setown(file, task_pid(current), PIDTYPE_PID,
+				current_uid(), current_euid(), 0);
 		if (ret)
 			goto out;
 		tun->flags |= TUN_FASYNC;
diff --git a/fs/fcntl.c b/fs/fcntl.c
index aeab1f4..f44327d 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -214,7 +214,7 @@ static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
 }
 
 int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
-		int force)
+		uid_t uid, uid_t euid, int force)
 {
 	int err;
 
@@ -222,7 +222,7 @@ int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
 	if (err)
 		return err;
 
-	f_modown(filp, pid, type, current_uid(), current_euid(), force);
+	f_modown(filp, pid, type, uid, euid, force);
 	return 0;
 }
 EXPORT_SYMBOL(__f_setown);
@@ -240,7 +240,8 @@ int f_setown(struct file *filp, unsigned long arg, int force)
 	}
 	rcu_read_lock();
 	pid = find_vpid(who);
-	result = __f_setown(filp, pid, type, force);
+	result = __f_setown(filp, pid, type, current_uid(), current_euid(),
+			force);
 	rcu_read_unlock();
 	return result;
 }
@@ -296,7 +297,8 @@ static int f_setown_ex(struct file *filp, unsigned long arg)
 	if (owner.pid && !pid)
 		ret = -ESRCH;
 	else
-		ret = __f_setown(filp, pid, type, 1);
+		ret = __f_setown(filp, pid, type, current_uid(), 
+				current_euid(), 1);
 	rcu_read_unlock();
 
 	return ret;
diff --git a/fs/locks.c b/fs/locks.c
index 9cd859e..ca0c7e2 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1514,7 +1514,8 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 		goto out_unlock;
 	}
 
-	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
+	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, current_uid(),
+				current_euid(), 0);
 out_unlock:
 	unlock_kernel();
 	return error;
diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c
index 0a63bf6..3e025e5 100644
--- a/fs/notify/dnotify/dnotify.c
+++ b/fs/notify/dnotify/dnotify.c
@@ -409,7 +409,8 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
 		goto out;
 	}
 
-	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
+	error = __f_setown(filp, task_pid(current), PIDTYPE_PID, current_uid(),
+				current_euid(), 0);
 	if (error) {
 		/* if we added, we must shoot */
 		if (dnentry == new_dnentry)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ee725ff..b4a6fb0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1304,7 +1304,8 @@ extern void kill_fasync(struct fasync_struct **, int, int);
 /* only for net: no internal synchronization */
 extern void __kill_fasync(struct fasync_struct *, int, int);
 
-extern int __f_setown(struct file *filp, struct pid *, enum pid_type, int force);
+extern int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
+		uid_t uid, uid_t euid, int force);
 extern int f_setown(struct file *filp, unsigned long arg, int force);
 extern void f_delown(struct file *filp);
 extern pid_t f_getown(struct file *filp);
-- 
1.6.0.4

[PATCH 03/17][cr][v4]: Checkpoint file-owner information

[Sukadev Bhattiprolu]

Checkpoint the file->f_owner information for an open file. This
information will be used to restore the file-owner information
when the application is restarted from the checkpoint.

The file->f_owner information is "private" to each 'struct file' i.e.
fown_struct is not an external object shared with other file structures.
So the information can directly be added to the 'ckpt_hdr_file' object.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/checkpoint.c                |   27 +++++++++++----------------
 include/linux/checkpoint_hdr.h |    5 +++++
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/checkpoint.c b/fs/checkpoint.c
index 87d7c6e..ce1b4af 100644
--- a/fs/checkpoint.c
+++ b/fs/checkpoint.c
@@ -168,12 +168,19 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
 			   struct ckpt_hdr_file *h)
 {
 	struct cred *f_cred = (struct cred *) file->f_cred;
+	struct pid *pid = file->f_owner.pid;
 
 	h->f_flags = file->f_flags;
 	h->f_mode = file->f_mode;
 	h->f_pos = file->f_pos;
 	h->f_version = file->f_version;
 
+	h->f_owner_pid = pid_nr_ns(pid, ns_of_pid(pid));
+	h->f_owner_pid_type = file->f_owner.pid_type;
+	h->f_owner_uid = file->f_owner.uid;
+	h->f_owner_euid = file->f_owner.euid;
+	h->f_owner_signum = file->f_owner.signum;
+
 	h->f_credref = checkpoint_obj(ctx, f_cred, CKPT_OBJ_CRED);
 	if (h->f_credref < 0)
 		return h->f_credref;
@@ -184,10 +191,10 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
 		return h->f_secref;
 	}
 
-	ckpt_debug("file %s credref %d secref %d\n",
-		file->f_dentry->d_name.name, h->f_credref, h->f_secref);
-
-	/* FIX: need also file->f_owner, etc */
+	ckpt_debug("file %s credref %d secref %d, fowner-pid %d, type %d, "
+			"fowner-signum %d\n", file->f_dentry->d_name.name,
+			h->f_credref, h->f_secref, h->f_owner_pid,
+			h->f_owner_pid_type, h->f_owner_signum);
 
 	return 0;
 }
@@ -267,7 +274,6 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
 	struct fdtable *fdt;
 	int objref, ret;
 	int coe = 0;	/* avoid gcc warning */
-	pid_t pid;
 
 	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE_DESC);
 	if (!h)
@@ -302,17 +308,6 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
 	}
 
 	/*
-	 * TODO: Implement c/r of fowner and f_sigio.  Should be
-	 * trivial, but for now we just refuse its checkpoint
-	 */
-	pid = f_getown(file);
-	if (pid) {
-		ret = -EBUSY;
-		ckpt_err(ctx, ret, "%(T)fd %d has an owner (%d)\n", fd);
-		goto out;
-	}
-
-	/*
 	 * if seen first time, this will add 'file' to the objhash, keep
 	 * a reference to it, dump its state while at it.
 	 */
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 9e8d518..0381019 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -575,6 +575,11 @@ struct ckpt_hdr_file {
 	__u64 f_pos;
 	__u64 f_version;
 	__s32 f_secref;
+	__s32 f_owner_pid;
+	__u32 f_owner_pid_type;
+	__u32 f_owner_uid;
+	__u32 f_owner_euid;
+	__s32 f_owner_signum;
 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_file_generic {
-- 
1.6.0.4

Re: [PATCH 03/17][cr][v4]: Checkpoint file-owner information

[Oren Laadan]

Hi Suka,

I was just about to pull this series, and merge every pair of
checkpoint-restore patches into a single patch. But then I saw
the problem described below, and I expect there will be v5. So
please for v5 and on, merge the checkpoint and restore parts
of a patch - it makes reviewing easier and reduced the number
of individual patches.

On 08/16/2010 03:43 PM, Sukadev Bhattiprolu wrote:
> Checkpoint the file->f_owner information for an open file. This
> information will be used to restore the file-owner information
> when the application is restarted from the checkpoint.
> 
> The file->f_owner information is "private" to each 'struct file' i.e.
> fown_struct is not an external object shared with other file structures.
> So the information can directly be added to the 'ckpt_hdr_file' object.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  fs/checkpoint.c                |   27 +++++++++++----------------
>  include/linux/checkpoint_hdr.h |    5 +++++
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/checkpoint.c b/fs/checkpoint.c
> index 87d7c6e..ce1b4af 100644
> --- a/fs/checkpoint.c
> +++ b/fs/checkpoint.c
> @@ -168,12 +168,19 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
>  			   struct ckpt_hdr_file *h)
>  {
>  	struct cred *f_cred = (struct cred *) file->f_cred;
> +	struct pid *pid = file->f_owner.pid;
>  
>  	h->f_flags = file->f_flags;
>  	h->f_mode = file->f_mode;
>  	h->f_pos = file->f_pos;
>  	h->f_version = file->f_version;
>  
> +	h->f_owner_pid = pid_nr_ns(pid, ns_of_pid(pid));

You take the pid as seen in the namespace in which ths @pid was
allocated (ns_of_pid). I'm not sure this is what we want .. is it ?

I would assume that the way to go is to save the pid as seen from
the _top_ pid-ns of the containers/subtree - so use ctx->root_task's
pid-ns as a reference.

Moreover, when restarting (now fast-forward to next patch where you
do the restore) - the restore occurs in the context of the restarting
task. However, in that context the pid may at all be invalid.

Perhaps the proper way to tackle this is to make 'struct pid *'
citizens of the objhash - that is, treat them as shared objects,
and refer to them through the tag.

Also, the pid may be invalid even if we originally only had one
single pid-ns, in the case that the original task carrying that pid
had already died by the time we checkpoint.

In this case, perhaps we can just ignore that pid (because there
will be no process to receive a signal, anyway). The alternative
is to create dummy 'struct pid *' on restart as necessary.

Once addressed, we will also need some test cases in the suite to
show that it works correctly ...

Thoughts ?

> +	h->f_owner_pid_type = file->f_owner.pid_type;
> +	h->f_owner_uid = file->f_owner.uid;
> +	h->f_owner_euid = file->f_owner.euid;

We will also need something like the above for user-ns, when the
user-ns is eventually conceived - maybe leave a "todo" comment ?

Oren.

> +	h->f_owner_signum = file->f_owner.signum;
> +
>  	h->f_credref = checkpoint_obj(ctx, f_cred, CKPT_OBJ_CRED);
>  	if (h->f_credref < 0)
>  		return h->f_credref;
> @@ -184,10 +191,10 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
>  		return h->f_secref;
>  	}
>  
> -	ckpt_debug("file %s credref %d secref %d\n",
> -		file->f_dentry->d_name.name, h->f_credref, h->f_secref);
> -
> -	/* FIX: need also file->f_owner, etc */
> +	ckpt_debug("file %s credref %d secref %d, fowner-pid %d, type %d, "
> +			"fowner-signum %d\n", file->f_dentry->d_name.name,
> +			h->f_credref, h->f_secref, h->f_owner_pid,
> +			h->f_owner_pid_type, h->f_owner_signum);
>  
>  	return 0;
>  }
> @@ -267,7 +274,6 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
>  	struct fdtable *fdt;
>  	int objref, ret;
>  	int coe = 0;	/* avoid gcc warning */
> -	pid_t pid;
>  
>  	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE_DESC);
>  	if (!h)
> @@ -302,17 +308,6 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
>  	}
>  
>  	/*
> -	 * TODO: Implement c/r of fowner and f_sigio.  Should be
> -	 * trivial, but for now we just refuse its checkpoint
> -	 */
> -	pid = f_getown(file);
> -	if (pid) {
> -		ret = -EBUSY;
> -		ckpt_err(ctx, ret, "%(T)fd %d has an owner (%d)\n", fd);
> -		goto out;
> -	}
> -
> -	/*
>  	 * if seen first time, this will add 'file' to the objhash, keep
>  	 * a reference to it, dump its state while at it.
>  	 */
> diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
> index 9e8d518..0381019 100644
> --- a/include/linux/checkpoint_hdr.h
> +++ b/include/linux/checkpoint_hdr.h
> @@ -575,6 +575,11 @@ struct ckpt_hdr_file {
>  	__u64 f_pos;
>  	__u64 f_version;
>  	__s32 f_secref;
> +	__s32 f_owner_pid;
> +	__u32 f_owner_pid_type;
> +	__u32 f_owner_uid;
> +	__u32 f_owner_euid;
> +	__s32 f_owner_signum;
>  } __attribute__((aligned(8)));
>  
>  struct ckpt_hdr_file_generic {

[PATCH 04/17][cr][v4]: Restore file_owner info

[Sukadev Bhattiprolu]

Restore the file-owner information for each 'struct file'.  This is
essentially is like a new fcntl(F_SETOWN) and fcntl(F_SETSIG) calls,
except that the pid, uid, euid and signum values are read from the
checkpoint image.

Changelog[v4]:
	- [Oren Laadan]: Sanitize the pid-type field read from checkpoint image.
Changelog[v3]:
	- [Oren Laadan]: Ensure find_vpid() found a valid pid before
	  making it the file owner.
Changelog[v2]:
	- [Matt Helsley, Serge Hallyn]: Don't trust uids in checkpoint image.
	  (added CAP_KILL check)
	- Check that signal number read from the checkpoint image is valid.
	  (not sure it is required, since its an incomplete check for tampering)

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/checkpoint.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/fs/checkpoint.c b/fs/checkpoint.c
index ce1b4af..be9d39a 100644
--- a/fs/checkpoint.c
+++ b/fs/checkpoint.c
@@ -618,6 +618,80 @@ static int attach_file(struct file *file)
 	return fd;
 }
 
+static int restore_file_owner(struct ckpt_ctx *ctx, struct ckpt_hdr_file *h,
+		struct file *file)
+{
+	int ret;
+	struct pid *pid;
+	uid_t uid, euid;
+
+	uid = h->f_owner_uid;
+	euid = h->f_owner_euid;
+
+	ckpt_debug("restore_file_owner(): uid %u, euid %u, pid %d, type %d\n",
+			uid, euid, h->f_owner_pid, h->f_owner_pid_type);
+	/*
+	 * We can't trust the uids in the checkpoint image and normally need
+	 * CAP_KILL. But if the uids match our ids, should be fine since we
+	 * have access to the file.
+	 *
+	 * TODO: Move this check to __f_setown() ?
+	 */
+	ret = -EACCES;
+	if (!capable(CAP_KILL) &&
+			(uid != current_uid() || euid != current_euid())) {
+		ckpt_err(ctx, ret, "image uids [%d, %d] don't match current "
+				"process uids [%d, %d] and no CAP_KILL\n",
+				uid, euid, current_uid(), current_euid());
+		return ret;
+	}
+
+	ret = -EINVAL;
+	if (!valid_signal(h->f_owner_signum)) {
+		ckpt_err(ctx, ret, "Invalid signum %d\n", h->f_owner_signum);
+		return ret;
+	}
+	file->f_owner.signum = h->f_owner_signum;
+
+	switch(h->f_owner_pid_type) {
+		case PIDTYPE_PID:
+		case PIDTYPE_PGID:
+		case PIDTYPE_SID:
+			break;
+		default:
+			ckpt_err(ctx, ret, "Invalid pid-type %d\n",
+						h->f_owner_pid_type);
+			return ret;
+			
+	}
+
+	rcu_read_lock();
+
+	/*
+	 * If file had a non-NULL owner and we can't find the owner after
+	 * restart, return error.
+	 */
+	pid = find_vpid(h->f_owner_pid);
+	if (h->f_owner_pid && !pid)
+		ret = -ESRCH;
+	else {
+		/*
+		 * TODO: Do we need 'force' to be 1 here or can it be 0 ?
+		 * 	 'force' is used to modify the owner, if one is
+		 * 	 already set. Can it be set when we restart an
+		 * 	 application ?
+		 */
+		ret = __f_setown(file, pid, h->f_owner_pid_type, uid, euid, 1);
+	}
+
+	rcu_read_unlock();
+
+	if (ret < 0)
+		ckpt_err(ctx, ret, "__fsetown_uid() failed\n");
+
+	return ret;
+}
+
 #define CKPT_SETFL_MASK  \
 	(O_APPEND | O_NONBLOCK | O_NDELAY | FASYNC | O_DIRECT | O_NOATIME)
 
@@ -651,6 +725,10 @@ int restore_file_common(struct ckpt_ctx *ctx, struct file *file,
 	if (ret < 0)
 		return ret;
 
+	ret = restore_file_owner(ctx, h, file);
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * Normally f_mode is set by open, and modified only via
 	 * fcntl(), so its value now should match that at checkpoint.
-- 
1.6.0.4

Re: [PATCH 04/17][cr][v4]: Restore file_owner info

[Oren Laadan]

On 08/16/2010 03:43 PM, Sukadev Bhattiprolu wrote:
> Restore the file-owner information for each 'struct file'.  This is
> essentially is like a new fcntl(F_SETOWN) and fcntl(F_SETSIG) calls,
> except that the pid, uid, euid and signum values are read from the
> checkpoint image.
> 
> Changelog[v4]:
> 	- [Oren Laadan]: Sanitize the pid-type field read from checkpoint image.
> Changelog[v3]:
> 	- [Oren Laadan]: Ensure find_vpid() found a valid pid before
> 	  making it the file owner.
> Changelog[v2]:
> 	- [Matt Helsley, Serge Hallyn]: Don't trust uids in checkpoint image.
> 	  (added CAP_KILL check)
> 	- Check that signal number read from the checkpoint image is valid.
> 	  (not sure it is required, since its an incomplete check for tampering)
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  fs/checkpoint.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 78 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/checkpoint.c b/fs/checkpoint.c
> index ce1b4af..be9d39a 100644
> --- a/fs/checkpoint.c
> +++ b/fs/checkpoint.c
> @@ -618,6 +618,80 @@ static int attach_file(struct file *file)
>  	return fd;
>  }
>  
> +static int restore_file_owner(struct ckpt_ctx *ctx, struct ckpt_hdr_file *h,
> +		struct file *file)
> +{
> +	int ret;
> +	struct pid *pid;
> +	uid_t uid, euid;
> +
> +	uid = h->f_owner_uid;
> +	euid = h->f_owner_euid;
> +
> +	ckpt_debug("restore_file_owner(): uid %u, euid %u, pid %d, type %d\n",
> +			uid, euid, h->f_owner_pid, h->f_owner_pid_type);
> +	/*
> +	 * We can't trust the uids in the checkpoint image and normally need
> +	 * CAP_KILL. But if the uids match our ids, should be fine since we
> +	 * have access to the file.
> +	 *
> +	 * TODO: Move this check to __f_setown() ?
> +	 */
> +	ret = -EACCES;
> +	if (!capable(CAP_KILL) &&
> +			(uid != current_uid() || euid != current_euid())) {
> +		ckpt_err(ctx, ret, "image uids [%d, %d] don't match current "
> +				"process uids [%d, %d] and no CAP_KILL\n",
> +				uid, euid, current_uid(), current_euid());
> +		return ret;
> +	}
> +
> +	ret = -EINVAL;
> +	if (!valid_signal(h->f_owner_signum)) {
> +		ckpt_err(ctx, ret, "Invalid signum %d\n", h->f_owner_signum);
> +		return ret;
> +	}
> +	file->f_owner.signum = h->f_owner_signum;
> +
> +	switch(h->f_owner_pid_type) {
> +		case PIDTYPE_PID:
> +		case PIDTYPE_PGID:
> +		case PIDTYPE_SID:
> +			break;
> +		default:
> +			ckpt_err(ctx, ret, "Invalid pid-type %d\n",
> +						h->f_owner_pid_type);
> +			return ret;
> +			
> +	}
> +
> +	rcu_read_lock();
> +
> +	/*
> +	 * If file had a non-NULL owner and we can't find the owner after
> +	 * restart, return error.
> +	 */
> +	pid = find_vpid(h->f_owner_pid);
> +	if (h->f_owner_pid && !pid)
> +		ret = -ESRCH;
> +	else {
> +		/*
> +		 * TODO: Do we need 'force' to be 1 here or can it be 0 ?
> +		 * 	 'force' is used to modify the owner, if one is
> +		 * 	 already set. Can it be set when we restart an
> +		 * 	 application ?
> +		 */
> +		ret = __f_setown(file, pid, h->f_owner_pid_type, uid, euid, 1);

The owner should not be set by now. So I suggest removing the
comment and bailing if already set.

Oren.

> +	}
> +
> +	rcu_read_unlock();
> +
> +	if (ret < 0)
> +		ckpt_err(ctx, ret, "__fsetown_uid() failed\n");
> +
> +	return ret;
> +}
> +
>  #define CKPT_SETFL_MASK  \
>  	(O_APPEND | O_NONBLOCK | O_NDELAY | FASYNC | O_DIRECT | O_NOATIME)
>  
> @@ -651,6 +725,10 @@ int restore_file_common(struct ckpt_ctx *ctx, struct file *file,
>  	if (ret < 0)
>  		return ret;
>  
> +	ret = restore_file_owner(ctx, h, file);
> +	if (ret < 0)
> +		return ret;
> +
>  	/*
>  	 * Normally f_mode is set by open, and modified only via
>  	 * fcntl(), so its value now should match that at checkpoint.

[PATCH 05/17][cr][v4]: Move file_lock macros into linux/fs.h

[Sukadev Bhattiprolu]

Move IS_POSIX(), IS_FLOCK(), IS_LEASE() and 'for_each_lock()' into
include/linux/fs.h since these are also needed to checkpoint/restart
file-locks.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c         |    7 -------
 include/linux/fs.h |    7 +++++++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index ca0c7e2..741b8b7 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -130,16 +130,9 @@
 
 #include <asm/uaccess.h>
 
-#define IS_POSIX(fl)	(fl->fl_flags & FL_POSIX)
-#define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
-#define IS_LEASE(fl)	(fl->fl_flags & FL_LEASE)
-
 int leases_enable = 1;
 int lease_break_time = 45;
 
-#define for_each_lock(inode, lockp) \
-	for (lockp = &inode->i_flock; *lockp != NULL; lockp = &(*lockp)->fl_next)
-
 static LIST_HEAD(file_lock_list);
 static LIST_HEAD(blocked_list);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b4a6fb0..abd2267 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1088,6 +1088,13 @@ struct file_lock {
 	} fl_u;
 };
 
+#define IS_POSIX(fl)	(fl->fl_flags & FL_POSIX)
+#define IS_FLOCK(fl)	(fl->fl_flags & FL_FLOCK)
+#define IS_LEASE(fl)	(fl->fl_flags & FL_LEASE)
+
+#define for_each_lock(inode, lockp) \
+	for (lockp = &inode->i_flock; *lockp != NULL; lockp = &(*lockp)->fl_next)
+
 /* The following constant reflects the upper bound of the file/locking space */
 #ifndef OFFSET_MAX
 #define INT_LIMIT(x)	(~((x)1 << (sizeof(x)*8 - 1)))
-- 
1.6.0.4

[PATCH 06/17][cr][v4]: Checkpoint file-locks

[Sukadev Bhattiprolu]

While checkpointing each file-descriptor, find all the locks on the
file and save information about the lock in the checkpoint-image.
A follow-on patch will use this informaiton to restore the file-locks.

Changelog[v4]:
	[Oren Laadan]: For consistency with other such objects, replace
	the "marker lock" checkpoint with a checkpoint of a count of the
	file-locks before the first file-lock of each file.

Changelog[v3]:
	[Oren Laadan] Add a missing (loff_t) type cast and use a macro
		to set the marker/dummy file lock

Changelog[v2]:
	[Matt Helsley]: Use fixed sizes (__s64) instead of 'loff_t' in
		'struct ckpt_hdr_file_lock'.
	[Matt Helsley, Serge Hallyn]: Highlight new use of BKL (using
		lock_flocks() macros as suggested by Serge).
	[Matt Helsley]: Reorg code a bit to simplify error handling.
	[Matt Helsley]: Reorg code to initialize marker-lock (Pass a
		NULL lock to checkpoint_one_lock() to indicate marker).

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/checkpoint.c                |  142 ++++++++++++++++++++++++++++++++++++----
 include/linux/checkpoint_hdr.h |   17 +++++
 2 files changed, 146 insertions(+), 13 deletions(-)

diff --git a/fs/checkpoint.c b/fs/checkpoint.c
index be9d39a..47aa802 100644
--- a/fs/checkpoint.c
+++ b/fs/checkpoint.c
@@ -26,8 +26,19 @@
 #include <linux/checkpoint.h>
 #include <linux/eventpoll.h>
 #include <linux/eventfd.h>
+#include <linux/smp_lock.h>
 #include <net/sock.h>
 
+/*
+ * TODO: This code uses the BKL for consistency with other uses of
+ * 	 'for_each_lock()'. But since the BKL may be replaced with another
+ * 	 lock in the future, use lock_flocks() macros instead. lock_flocks()
+ * 	 are currently used in BKL-fix sand boxes and when those changes
+ * 	 are merged, the following macros can be removed
+ */
+#define lock_flocks()		lock_kernel()
+#define unlock_flocks()	unlock_kernel()
+
 /**************************************************************************
  * Checkpoint
  */
@@ -256,8 +267,120 @@ static int checkpoint_file(struct ckpt_ctx *ctx, void *ptr)
 	return ret;
 }
 
+static int checkpoint_one_file_lock(struct ckpt_ctx *ctx, 
+		struct file_lock *lock)
+{
+	int rc;
+	struct ckpt_hdr_file_lock *h;
+
+	if (!IS_POSIX(lock)) {
+		/* Hmm, we should have caught this while counting locks */
+		return -EBADF;
+	}
+
+	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE_LOCK);
+	if (!h)
+		return -ENOMEM;
+
+	h->fl_start = lock->fl_start;
+	h->fl_end = lock->fl_end;
+	h->fl_type = lock->fl_type;
+	h->fl_flags = lock->fl_flags;
+
+	rc = ckpt_write_obj(ctx, &h->h);
+
+	ckpt_hdr_put(ctx, h);
+
+	return rc;
+}
+
+static int checkpoint_file_lock_count(struct ckpt_ctx *ctx, int num_locks)
+{
+	int rc;
+	struct ckpt_hdr_file_lock_count *h;
+
+	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE_LOCK_COUNT);
+	if (!h)
+		return -ENOMEM;
+
+	h->nr_locks = num_locks;
+
+	rc = ckpt_write_obj(ctx, &h->h);
+
+	ckpt_hdr_put(ctx, h);
+
+	return rc;
+}
+
+int
+checkpoint_file_locks(struct ckpt_ctx *ctx, struct files_struct *files,
+		struct file *file)
+{
+	int n;
+	int rc;
+	struct inode *inode;
+	struct file_lock **lockpp;
+	struct file_lock *lockp;
+
+	lock_flocks();
+
+	/*
+	 * First count the number of file-locks on this file
+	 */
+	n = 0;
+	rc = -EBADF;
+	inode = file->f_path.dentry->d_inode;
+	for_each_lock(inode, lockpp) {
+		lockp = *lockpp;
+		if (lockp->fl_owner != files)
+			continue;
+
+		ckpt_debug("Lock [%lld, %lld, %d, 0x%x]\n", lockp->fl_start,
+				lockp->fl_end, lockp->fl_type, lockp->fl_flags);
+
+		if (lockp->fl_owner != files)
+			continue;
+
+		if (IS_POSIX(lockp))
+			n++;
+		else {
+			ckpt_err(ctx,  rc, "%(T), checkpoint of lock "
+					"[%lld, %lld, %d, 0x%x] failed\n",
+					lockp->fl_start, lockp->fl_end,
+					lockp->fl_type, lockp->fl_flags);
+			goto out;
+		}
+	}
+
+	/*
+	 * Checkpoint the count of file-locks
+	 */
+	rc = checkpoint_file_lock_count(ctx, n);
+	if (rc < 0) {
+		ckpt_err(ctx,  rc, "%(T), checkpoint file-lock count failed\n");
+		goto out;
+	}
+
+	/*
+	 * Make a second pass and checkpoint file-locks themselves.
+	 */
+	for_each_lock(inode, lockpp) {
+		lockp = *lockpp;
+		if (lockp->fl_owner != files)
+			continue;
+
+		rc = checkpoint_one_file_lock(ctx, lockp);
+		if (rc < 0)
+			goto out;
+	}
+
+out:
+	unlock_flocks();
+	return rc;
+}
+
 /**
- * ckpt_write_file_desc - dump the state of a given file descriptor
+ * checkpoint_file_desc - dump the state of a given file descriptor
  * @ctx: checkpoint context
  * @files: files_struct pointer
  * @fd: file descriptor
@@ -288,18 +411,6 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
 	}
 	rcu_read_unlock();
 
-	ret = find_locks_with_owner(file, files);
-	/*
-	 * find_locks_with_owner() returns an error when there
-	 * are no locks found, so we *want* it to return an error
-	 * code.  Its success means we have to fail the checkpoint.
-	 */
-	if (!ret) {
-		ret = -EBADF;
-		ckpt_err(ctx, ret, "%(T)fd %d has file lock or lease\n", fd);
-		goto out;
-	}
-
 	/* sanity check (although this shouldn't happen) */
 	ret = -EBADF;
 	if (!file) {
@@ -323,6 +434,11 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
 	h->fd_close_on_exec = coe;
 
 	ret = ckpt_write_obj(ctx, &h->h);
+	if (ret < 0)
+		goto out;
+
+	ret = checkpoint_file_locks(ctx, files, file);
+
 out:
 	ckpt_hdr_put(ctx, h);
 	if (file)
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 0381019..14b287f 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -144,6 +144,10 @@ enum {
 #define CKPT_HDR_TTY_LDISC CKPT_HDR_TTY_LDISC
 	CKPT_HDR_EPOLL_ITEMS,  /* must be after file-table */
 #define CKPT_HDR_EPOLL_ITEMS CKPT_HDR_EPOLL_ITEMS
+	CKPT_HDR_FILE_LOCK_COUNT,
+#define CKPT_HDR_FILE_LOCK_COUNT CKPT_HDR_FILE_LOCK_COUNT
+	CKPT_HDR_FILE_LOCK,
+#define CKPT_HDR_FILE_LOCK CKPT_HDR_FILE_LOCK
 
 	CKPT_HDR_MM = 401,
 #define CKPT_HDR_MM CKPT_HDR_MM
@@ -586,6 +590,19 @@ struct ckpt_hdr_file_generic {
 	struct ckpt_hdr_file common;
 } __attribute__((aligned(8)));
 
+struct ckpt_hdr_file_lock_count {
+       struct ckpt_hdr h;
+       __u32 nr_locks;
+};
+
+struct ckpt_hdr_file_lock {
+       struct ckpt_hdr h;
+       __s64 fl_start;
+       __s64 fl_end;
+       __u8 fl_type;
+       __u8 fl_flags;
+};
+
 struct ckpt_hdr_file_pipe {
 	struct ckpt_hdr_file common;
 	__s32 pipe_objref;
-- 
1.6.0.4

Re: [PATCH 06/17][cr][v4]: Checkpoint file-locks

[Oren Laadan]

On 08/16/2010 03:43 PM, Sukadev Bhattiprolu wrote:
> While checkpointing each file-descriptor, find all the locks on the
> file and save information about the lock in the checkpoint-image.
> A follow-on patch will use this informaiton to restore the file-locks.
> 
> Changelog[v4]:
> 	[Oren Laadan]: For consistency with other such objects, replace
> 	the "marker lock" checkpoint with a checkpoint of a count of the
> 	file-locks before the first file-lock of each file.
> 
> Changelog[v3]:
> 	[Oren Laadan] Add a missing (loff_t) type cast and use a macro
> 		to set the marker/dummy file lock
> 
> Changelog[v2]:
> 	[Matt Helsley]: Use fixed sizes (__s64) instead of 'loff_t' in
> 		'struct ckpt_hdr_file_lock'.
> 	[Matt Helsley, Serge Hallyn]: Highlight new use of BKL (using
> 		lock_flocks() macros as suggested by Serge).
> 	[Matt Helsley]: Reorg code a bit to simplify error handling.
> 	[Matt Helsley]: Reorg code to initialize marker-lock (Pass a
> 		NULL lock to checkpoint_one_lock() to indicate marker).
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  fs/checkpoint.c                |  142 ++++++++++++++++++++++++++++++++++++----
>  include/linux/checkpoint_hdr.h |   17 +++++
>  2 files changed, 146 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/checkpoint.c b/fs/checkpoint.c
> index be9d39a..47aa802 100644
> --- a/fs/checkpoint.c
> +++ b/fs/checkpoint.c
> @@ -26,8 +26,19 @@
>  #include <linux/checkpoint.h>
>  #include <linux/eventpoll.h>
>  #include <linux/eventfd.h>
> +#include <linux/smp_lock.h>
>  #include <net/sock.h>
>  
> +/*
> + * TODO: This code uses the BKL for consistency with other uses of
> + * 	 'for_each_lock()'. But since the BKL may be replaced with another
> + * 	 lock in the future, use lock_flocks() macros instead. lock_flocks()
> + * 	 are currently used in BKL-fix sand boxes and when those changes
> + * 	 are merged, the following macros can be removed
> + */
> +#define lock_flocks()		lock_kernel()
> +#define unlock_flocks()	unlock_kernel()
> +
>  /**************************************************************************
>   * Checkpoint
>   */
> @@ -256,8 +267,120 @@ static int checkpoint_file(struct ckpt_ctx *ctx, void *ptr)
>  	return ret;
>  }
>  
> +static int checkpoint_one_file_lock(struct ckpt_ctx *ctx, 
> +		struct file_lock *lock)
> +{
> +	int rc;
> +	struct ckpt_hdr_file_lock *h;
> +
> +	if (!IS_POSIX(lock)) {
> +		/* Hmm, we should have caught this while counting locks */
> +		return -EBADF;
> +	}
> +
> +	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE_LOCK);
> +	if (!h)
> +		return -ENOMEM;
> +
> +	h->fl_start = lock->fl_start;
> +	h->fl_end = lock->fl_end;
> +	h->fl_type = lock->fl_type;
> +	h->fl_flags = lock->fl_flags;
> +
> +	rc = ckpt_write_obj(ctx, &h->h);
> +
> +	ckpt_hdr_put(ctx, h);
> +
> +	return rc;
> +}
> +
> +static int checkpoint_file_lock_count(struct ckpt_ctx *ctx, int num_locks)
> +{
> +	int rc;
> +	struct ckpt_hdr_file_lock_count *h;
> +
> +	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE_LOCK_COUNT);
> +	if (!h)
> +		return -ENOMEM;
> +
> +	h->nr_locks = num_locks;
> +
> +	rc = ckpt_write_obj(ctx, &h->h);
> +
> +	ckpt_hdr_put(ctx, h);
> +
> +	return rc;
> +}
> +
> +int
> +checkpoint_file_locks(struct ckpt_ctx *ctx, struct files_struct *files,
> +		struct file *file)
> +{
> +	int n;
> +	int rc;
> +	struct inode *inode;
> +	struct file_lock **lockpp;
> +	struct file_lock *lockp;
> +
> +	lock_flocks();
> +
> +	/*
> +	 * First count the number of file-locks on this file
> +	 */
> +	n = 0;
> +	rc = -EBADF;
> +	inode = file->f_path.dentry->d_inode;
> +	for_each_lock(inode, lockpp) {
> +		lockp = *lockpp;
> +		if (lockp->fl_owner != files)
> +			continue;
> +
> +		ckpt_debug("Lock [%lld, %lld, %d, 0x%x]\n", lockp->fl_start,
> +				lockp->fl_end, lockp->fl_type, lockp->fl_flags);
> +
> +		if (lockp->fl_owner != files)
> +			continue;
> +
> +		if (IS_POSIX(lockp))
> +			n++;
> +		else {
> +			ckpt_err(ctx,  rc, "%(T), checkpoint of lock "
> +					"[%lld, %lld, %d, 0x%x] failed\n",
> +					lockp->fl_start, lockp->fl_end,
> +					lockp->fl_type, lockp->fl_flags);
> +			goto out;
> +		}
> +	}
> +
> +	/*
> +	 * Checkpoint the count of file-locks
> +	 */
> +	rc = checkpoint_file_lock_count(ctx, n);
> +	if (rc < 0) {
> +		ckpt_err(ctx,  rc, "%(T), checkpoint file-lock count failed\n");
> +		goto out;
> +	}
> +
> +	/*
> +	 * Make a second pass and checkpoint file-locks themselves.
> +	 */
> +	for_each_lock(inode, lockpp) {
> +		lockp = *lockpp;
> +		if (lockp->fl_owner != files)
> +			continue;
> +
> +		rc = checkpoint_one_file_lock(ctx, lockp);
> +		if (rc < 0)
> +			goto out;
> +	}
> +
> +out:
> +	unlock_flocks();
> +	return rc;
> +}
> +
>  /**
> - * ckpt_write_file_desc - dump the state of a given file descriptor
> + * checkpoint_file_desc - dump the state of a given file descriptor
>   * @ctx: checkpoint context
>   * @files: files_struct pointer
>   * @fd: file descriptor
> @@ -288,18 +411,6 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
>  	}
>  	rcu_read_unlock();
>  
> -	ret = find_locks_with_owner(file, files);
> -	/*
> -	 * find_locks_with_owner() returns an error when there
> -	 * are no locks found, so we *want* it to return an error
> -	 * code.  Its success means we have to fail the checkpoint.
> -	 */
> -	if (!ret) {
> -		ret = -EBADF;
> -		ckpt_err(ctx, ret, "%(T)fd %d has file lock or lease\n", fd);
> -		goto out;
> -	}
> -
>  	/* sanity check (although this shouldn't happen) */
>  	ret = -EBADF;
>  	if (!file) {
> @@ -323,6 +434,11 @@ static int checkpoint_file_desc(struct ckpt_ctx *ctx,
>  	h->fd_close_on_exec = coe;
>  
>  	ret = ckpt_write_obj(ctx, &h->h);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = checkpoint_file_locks(ctx, files, file);
> +
>  out:
>  	ckpt_hdr_put(ctx, h);
>  	if (file)
> diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
> index 0381019..14b287f 100644
> --- a/include/linux/checkpoint_hdr.h
> +++ b/include/linux/checkpoint_hdr.h
> @@ -144,6 +144,10 @@ enum {
>  #define CKPT_HDR_TTY_LDISC CKPT_HDR_TTY_LDISC
>  	CKPT_HDR_EPOLL_ITEMS,  /* must be after file-table */
>  #define CKPT_HDR_EPOLL_ITEMS CKPT_HDR_EPOLL_ITEMS
> +	CKPT_HDR_FILE_LOCK_COUNT,
> +#define CKPT_HDR_FILE_LOCK_COUNT CKPT_HDR_FILE_LOCK_COUNT
> +	CKPT_HDR_FILE_LOCK,
> +#define CKPT_HDR_FILE_LOCK CKPT_HDR_FILE_LOCK
>  
>  	CKPT_HDR_MM = 401,
>  #define CKPT_HDR_MM CKPT_HDR_MM
> @@ -586,6 +590,19 @@ struct ckpt_hdr_file_generic {
>  	struct ckpt_hdr_file common;
>  } __attribute__((aligned(8)));
>  
> +struct ckpt_hdr_file_lock_count {
> +       struct ckpt_hdr h;
> +       __u32 nr_locks;
> +};
> +
> +struct ckpt_hdr_file_lock {
> +       struct ckpt_hdr h;
> +       __s64 fl_start;
> +       __s64 fl_end;
> +       __u8 fl_type;
> +       __u8 fl_flags;
> +};
> +

You forgot to add "__attribute__((aligned(8)));".

[maybe picking.. but-] Giving a "whole" object per lock seems
wasteful; I was hoping for something similar to how we do the
'struct ckpt_hdr_sigpending', e.g.

struct ckpt_file_lock {
	__s64 fl_start;
	__s64 fl_end;
	__u8 fl_type;
	__u8 fl_flags;
} __attribute__((aligned(8)));

struct ckpt_hdr_file_locks {
	struct ckpt_hdr h;
	__u32 nr_locks;
	struct ckpt_file_lock locks[0];
} __attribute__((aligned(8)));

You could use ckpt_write_obj_type() with null pointer, and then
follow with the payload in chunks in the loop in checkpoint_file_locks()

Makes sense ?

Oren.

[PATCH 07/17][cr][v4]: Define flock_set()

[Sukadev Bhattiprolu]

Extract core functionality of fcntl_setlk() into a separate function,
flock_set(). flock_set() can be also used when restarting a checkpointed
application and restoring its file-locks.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c         |   44 +++++++++++++++++++++++++++-----------------
 include/linux/fs.h |    1 +
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 741b8b7..cb83741 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1759,14 +1759,10 @@ static int do_lock_file_wait(struct file *filp, unsigned int cmd,
 	return error;
 }
 
-/* Apply the lock described by l to an open file descriptor.
- * This implements both the F_SETLK and F_SETLKW commands of fcntl().
- */
-int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
-		struct flock __user *l)
+int flock_set(unsigned int fd, struct file *filp, unsigned int cmd,
+		struct flock *flock)
 {
 	struct file_lock *file_lock = locks_alloc_lock();
-	struct flock flock;
 	struct inode *inode;
 	struct file *f;
 	int error;
@@ -1774,13 +1770,6 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
 	if (file_lock == NULL)
 		return -ENOLCK;
 
-	/*
-	 * This might block, so we do it before checking the inode.
-	 */
-	error = -EFAULT;
-	if (copy_from_user(&flock, l, sizeof(flock)))
-		goto out;
-
 	inode = filp->f_path.dentry->d_inode;
 
 	/* Don't allow mandatory locks on files that may be memory mapped
@@ -1792,7 +1781,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
 	}
 
 again:
-	error = flock_to_posix_lock(filp, file_lock, &flock);
+	error = flock_to_posix_lock(filp, file_lock, flock);
 	if (error)
 		goto out;
 	if (cmd == F_SETLKW) {
@@ -1800,7 +1789,7 @@ again:
 	}
 	
 	error = -EBADF;
-	switch (flock.l_type) {
+	switch (flock->l_type) {
 	case F_RDLCK:
 		if (!(filp->f_mode & FMODE_READ))
 			goto out;
@@ -1830,8 +1819,8 @@ again:
 	spin_lock(&current->files->file_lock);
 	f = fcheck(fd);
 	spin_unlock(&current->files->file_lock);
-	if (!error && f != filp && flock.l_type != F_UNLCK) {
-		flock.l_type = F_UNLCK;
+	if (!error && f != filp && flock->l_type != F_UNLCK) {
+		flock->l_type = F_UNLCK;
 		goto again;
 	}
 
@@ -1840,6 +1829,27 @@ out:
 	return error;
 }
 
+/* Apply the lock described by l to an open file descriptor.
+ * This implements both the F_SETLK and F_SETLKW commands of fcntl().
+ */
+int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
+		struct flock __user *l)
+{
+	int error;
+	struct flock flock;
+	
+	/*
+	 * This might block, so we do it before checking the inode
+	 * in flock_set().
+	 */
+	error = -EFAULT;
+	if (copy_from_user(&flock, l, sizeof(flock)))
+		return error;
+
+	return flock_set(fd, filp, cmd, &flock);
+}
+
+
 #if BITS_PER_LONG == 32
 /* Report the first existing lock that would conflict with l.
  * This implements the F_GETLK command of fcntl().
diff --git a/include/linux/fs.h b/include/linux/fs.h
index abd2267..fd66f37 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1112,6 +1112,7 @@ extern void send_sigio(struct fown_struct *fown, int fd, int band);
 extern int fcntl_getlk(struct file *, struct flock __user *);
 extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
 			struct flock __user *);
+extern int flock_set(unsigned int, struct file *, unsigned int, struct flock *);
 
 #if BITS_PER_LONG == 32
 extern int fcntl_getlk64(struct file *, struct flock64 __user *);
-- 
1.6.0.4

[PATCH 08/17][cr][v4]: Define flock64_set()

[Sukadev Bhattiprolu]

Extract core functionality of fcntl_setlk64() into a separate function,
flock64_set(). flock64_set() can be also used when restarting a checkpointed
application and restoring its file-locks.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c         |   38 ++++++++++++++++++++++++--------------
 include/linux/fs.h |    2 ++
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index cb83741..c62ab7f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1890,11 +1890,10 @@ out:
 /* Apply the lock described by l to an open file descriptor.
  * This implements both the F_SETLK and F_SETLKW commands of fcntl().
  */
-int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd,
-		struct flock64 __user *l)
+int flock64_set(unsigned int fd, struct file *filp, unsigned int cmd,
+		struct flock64 *flock)
 {
 	struct file_lock *file_lock = locks_alloc_lock();
-	struct flock64 flock;
 	struct inode *inode;
 	struct file *f;
 	int error;
@@ -1902,13 +1901,6 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd,
 	if (file_lock == NULL)
 		return -ENOLCK;
 
-	/*
-	 * This might block, so we do it before checking the inode.
-	 */
-	error = -EFAULT;
-	if (copy_from_user(&flock, l, sizeof(flock)))
-		goto out;
-
 	inode = filp->f_path.dentry->d_inode;
 
 	/* Don't allow mandatory locks on files that may be memory mapped
@@ -1920,7 +1912,7 @@ int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd,
 	}
 
 again:
-	error = flock64_to_posix_lock(filp, file_lock, &flock);
+	error = flock64_to_posix_lock(filp, file_lock, flock);
 	if (error)
 		goto out;
 	if (cmd == F_SETLKW64) {
@@ -1928,7 +1920,7 @@ again:
 	}
 	
 	error = -EBADF;
-	switch (flock.l_type) {
+	switch (flock->l_type) {
 	case F_RDLCK:
 		if (!(filp->f_mode & FMODE_READ))
 			goto out;
@@ -1953,8 +1945,8 @@ again:
 	spin_lock(&current->files->file_lock);
 	f = fcheck(fd);
 	spin_unlock(&current->files->file_lock);
-	if (!error && f != filp && flock.l_type != F_UNLCK) {
-		flock.l_type = F_UNLCK;
+	if (!error && f != filp && flock->l_type != F_UNLCK) {
+		flock->l_type = F_UNLCK;
 		goto again;
 	}
 
@@ -1962,6 +1954,24 @@ out:
 	locks_free_lock(file_lock);
 	return error;
 }
+
+/* Apply the lock described by l to an open file descriptor.
+ * This implements both the F_SETLK and F_SETLKW commands of fcntl().
+ */
+int fcntl_setlk64(unsigned int fd, struct file *filp, unsigned int cmd,
+		struct flock64 __user *l)
+{
+	struct flock64 flock;
+
+	/*
+	 * This might block, so we do it before checking the inode in
+	 * flock64_set().
+	 */
+	if (copy_from_user(&flock, l, sizeof(flock)))
+		return -EFAULT;
+
+	return flock64_set(fd, filp, cmd, &flock);
+}
 #endif /* BITS_PER_LONG == 32 */
 
 /*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index fd66f37..49d4eeb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1118,6 +1118,8 @@ extern int flock_set(unsigned int, struct file *, unsigned int, struct flock *);
 extern int fcntl_getlk64(struct file *, struct flock64 __user *);
 extern int fcntl_setlk64(unsigned int, struct file *, unsigned int,
 			struct flock64 __user *);
+extern int flock64_set(unsigned int, struct file *, unsigned int,
+			struct flock64 *);
 #endif
 
 extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
-- 
1.6.0.4

[PATCH 09/17][cr][v4]: Restore file-locks

[Sukadev Bhattiprolu]

Restore POSIX file-locks of an application from its checkpoint image.

Read the saved file-locks from the checkpoint image and for each POSIX
lock, call flock_set() to set the lock on the file.

As pointed out by Matt Helsley, no special handling is necessary for a
process P2 in the checkpointed container that is blocked on a lock, L1
held by another process P1. Processes in the restarted container begin
execution only after all processes have restored. If the blocked process
P2 is restored first, it will prepare to return an -ERESTARTSYS from the
fcntl() system call, but wait for P1 to be restored. When P1 is restored,
it will re-acquire the lock L1 before P1 and P2 begin actual execution.
This ensures that even if P2 is scheduled to run before P1, P2 will go
back to waiting for the lock L1.

Changelog[v4]:
	- [Oren Laadan]: For consistency with other such objects, replace
	  checkpoint of the "marker lock" with a checkpoint of a count of
	  file-locks on the file, before the checkpoint of the first lock.
	- Added a missing ckpt_hdr_put()
	- Minor reorg of code.

Changelog[v3]:
	- [Oren Laadan]: Use a macro that can be shared with user-space
	  to set/test marker file-lock.

Changelog[v2]:
	- Add support for C/R of F_SETLK64/F_GETLK64

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/checkpoint.c |  176 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 174 insertions(+), 2 deletions(-)

diff --git a/fs/checkpoint.c b/fs/checkpoint.c
index 47aa802..540f831 100644
--- a/fs/checkpoint.c
+++ b/fs/checkpoint.c
@@ -981,8 +981,176 @@ static void *restore_file(struct ckpt_ctx *ctx)
 	return (void *)file;
 }
 
+#if BITS_PER_LONG == 32
+
+/*
+ * NOTE: Even if we checkpointed a lock that was set with 'struct flock'
+ * 	 restore the lock using 'struct flock64'. Note that both these lock
+ * 	 types are first converted to a posix_file_lock before processing so
+ * 	 converting to 'struct flock64' is (hopefully) not a problem. 
+ * 	 NFS for instance uses IS_SETLK() instead of cmd == F_SETLK.
+ *
+ * 	 TODO:  Are there filesystems that implement F_SETLK but not F_SETLK64 ?
+ * 	 	If there are, restore_one_posix_lock() will fail.
+ */
+static int
+ckpt_hdr_file_lock_to_flock64(struct ckpt_hdr_file_lock *h, struct flock64 *fl)
+{
+	/*
+	 * We checkpoint the 'raw' fl_type which in case of leases includes
+	 * the F_INPROGRESS flag. But for posix-locks, the fl_type should
+	 * be simple.
+	 */
+	switch(h->fl_type) {
+		case F_RDLCK:
+		case F_WRLCK:
+		case F_UNLCK:
+			break;
+		default:
+			ckpt_debug("Bad posix lock type 0x%x ?\n", h->fl_type);
+			return -EINVAL;
+	}
+
+	memset(fl, 0, sizeof(*fl));
+	fl->l_type = h->fl_type;
+	fl->l_start = h->fl_start;
+	fl->l_len = h->fl_end == OFFSET_MAX ? 0 : h->fl_end - h->fl_start + 1;
+	fl->l_whence = SEEK_SET;
+
+	/* TODO: Init ->l_sysid, l_pid fields */
+	ckpt_debug("Restoring filelock [%lld, %lld, %d]\n", fl->l_start,
+			fl->l_len, fl->l_type);
+
+	return 0;
+}
+
+static int restore_one_posix_lock(struct ckpt_ctx *ctx, struct file *file,
+		int fd, struct ckpt_hdr_file_lock *h)
+{
+	struct flock64 fl;
+	int ret;
+
+	ret = ckpt_hdr_file_lock_to_flock64(h, &fl);
+	if (ret < 0) {
+		ckpt_err(ctx, ret, "%(T) Unexpected flock\n");
+		return ret;
+	}
+
+	/*
+	 * Use F_SETLK because we should not have to wait for the lock. If
+	 * another process holds the lock, it indicates that filesystem-state
+	 * is not consistent with what it was at checkpoint. In which case we
+	 * better fail.
+	 */
+	ret = flock64_set(fd, file, F_SETLK64, &fl);
+	if (ret)
+		ckpt_err(ctx, ret, "flock64_set(): %d\n", (int)h->fl_type);
+
+	return ret;
+}
+
+#else
+
+static int
+ckpt_hdr_file_lock_to_flock(struct ckpt_hdr_file_lock *h, struct flock *fl)
+{
+	/*
+	 * We checkpoint the 'raw' fl_type which in case of leases includes
+	 * the F_INPROGRESS flag. But for posix-locks, the fl_type should
+	 * be simple.
+	 */
+	switch(h->fl_type) {
+		case F_RDLCK:
+		case F_WRLCK:
+		case F_UNLCK:
+			break;
+		default:
+			ckpt_debug("Bad posix lock type 0x%x ?\n", h->fl_type);
+			return -EINVAL;
+	}
+
+	memset(fl, 0, sizeof(*fl));
+
+	fl->l_type = h->fl_type;
+	fl->l_start = h->fl_start;
+	fl->l_len = fl->fl_end == OFFSET_MAX ? 0 : h->fl_end - h->fl_start + 1;
+	fl->l_whence = SEEK_SET;
+
+	ckpt_debug("Restoring filelock [%lld, %lld, %d]\n", fl->l_start,
+			fl->l_len, fl->l_type);
+
+	/* TODO: Init ->l_sysid, l_pid fields */
+
+	return 0;
+}
+
+static int restore_one_posix_lock(struct ckpt_ctx *ctx, struct file *file,
+		int fd, struct ckpt_hdr_file_lock *h)
+{
+	struct flock fl;
+	int ret;
+
+	ret = ckpt_hdr_file_lock_to_flock(h, &fl);
+	if (ret < 0) {
+		ckpt_err(ctx, ret, "%(T) Unexpected flock\n");
+		break;
+	}
+
+	/*
+	 * Use F_SETLK because we should not have to wait for the lock. If
+	 * another process holds the lock, it indicates that filesystem-state
+	 * is not consistent with what it was at checkpoint. In which case we
+	 * better fail.
+	 */
+	ret = flock_set(fd, file, F_SETLK, &fl);
+	if (ret)
+		ckpt_err(ctx, ret, "flock_set(): %d\n", (int)h->fl_type);
+
+	return ret;
+}
+#endif
+
+static int restore_file_locks(struct ckpt_ctx *ctx, struct file *file, int fd)
+{
+	int i, ret;
+	struct ckpt_hdr_file_lock *h;
+	struct ckpt_hdr_file_lock_count *hfc;
+
+	hfc = ckpt_read_obj_type(ctx, sizeof(*hfc), CKPT_HDR_FILE_LOCK_COUNT);
+	if (IS_ERR(hfc))
+		return PTR_ERR(hfc);
+
+	ret = 0;
+	for (i = 0; i < hfc->nr_locks; i++) {
+
+		h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_FILE_LOCK);
+		if (IS_ERR(h)) {
+			ret = PTR_ERR(h);
+			goto out;
+		}
+
+		ckpt_debug("Lock [%lld, %lld, %d, 0x%x]\n", h->fl_start,
+				h->fl_end, (int)h->fl_type, h->fl_flags);
+
+		ret = -EBADF;
+		if (h->fl_flags & FL_POSIX)
+			ret = restore_one_posix_lock(ctx, file, fd, h); 
+
+		ckpt_hdr_put(ctx, h);
+
+		if (ret < 0) {
+			ckpt_err(ctx, ret, "%(T)\n");
+			goto out;
+		}
+
+	}
+out:
+	ckpt_hdr_put(ctx, hfc);
+	return ret;
+}
+
 /**
- * ckpt_read_file_desc - restore the state of a given file descriptor
+ * restore_file_desc - restore the state of a given file descriptor
  * @ctx: checkpoint context
  *
  * Restores the state of a file descriptor; looks up the objref (in the
@@ -1028,7 +1196,11 @@ static int restore_file_desc(struct ckpt_ctx *ctx)
 	}
 
 	set_close_on_exec(h->fd_descriptor, h->fd_close_on_exec);
-	ret = 0;
+
+	ret = restore_file_locks(ctx, file, h->fd_descriptor);
+	if (ret < 0)
+		ckpt_err(ctx, ret, "Error on fd %d\n", h->fd_descriptor);
+
  out:
 	ckpt_hdr_put(ctx, h);
 	return ret;
-- 
1.6.0.4

[PATCH 10/17][cr][v4]: Initialize ->fl_break_time to 0

[Sukadev Bhattiprolu]

Explicitly initialize ->fl_break_time to 0 in locks_init_lock() and
__locks_copy_lock(). ->fl_break_time will be later used during
checkpoint/restart of an applicaton that has a file-lease.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index c62ab7f..0bd5af7 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -185,6 +185,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_flags = 0;
 	fl->fl_type = 0;
 	fl->fl_start = fl->fl_end = 0;
+	fl->fl_break_time = 0UL;
 	fl->fl_ops = NULL;
 	fl->fl_lmops = NULL;
 }
@@ -228,6 +229,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 	new->fl_type = fl->fl_type;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
+	new->fl_break_time = 0UL;
 	new->fl_ops = NULL;
 	new->fl_lmops = NULL;
 }
-- 
1.6.0.4

[PATCH 11/17][cr][v4]: Add ->fl_type_prev field.

[Sukadev Bhattiprolu]

In preparation for checkpoint/restart of file leases, add ->fl_type_prev
field to 'struct file_lock'. This field is needed to correctly restore
file leases in case of recursive checkpoint/restart of an in-progress
lease.

This ->fl_type_prev is only initialized in this patch. It will actually
be used when restoring file leases after a checkpoint.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c         |   10 ++++++++++
 include/linux/fs.h |    1 +
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 0bd5af7..9a00876 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -184,6 +184,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_file = NULL;
 	fl->fl_flags = 0;
 	fl->fl_type = 0;
+	fl->fl_type_prev = 0;
 	fl->fl_start = fl->fl_end = 0;
 	fl->fl_break_time = 0UL;
 	fl->fl_ops = NULL;
@@ -227,6 +228,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 	new->fl_file = NULL;
 	new->fl_flags = fl->fl_flags;
 	new->fl_type = fl->fl_type;
+	new->fl_type_prev = fl->fl_type_prev;
 	new->fl_start = fl->fl_start;
 	new->fl_end = fl->fl_end;
 	new->fl_break_time = 0UL;
@@ -293,6 +295,13 @@ static int assign_type(struct file_lock *fl, int type)
 	case F_WRLCK:
 	case F_UNLCK:
 		fl->fl_type = type;
+		/*
+		 * Clear ->fl_type_prev since this is a new lease type.
+		 * break_lease() will use this cleared state to know
+		 * if it must save the lease-type in case of checkpoint/
+		 * restart.
+		 */
+		fl->fl_type_prev = 0;
 		break;
 	default:
 		return -EINVAL;
@@ -1222,6 +1231,7 @@ int __break_lease(struct inode *inode, unsigned int mode)
 
 	for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) {
 		if (fl->fl_type != future) {
+			fl->fl_type_prev = fl->fl_type;
 			fl->fl_type = future;
 			fl->fl_break_time = break_time;
 			/* lease must have lmops break callback */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 49d4eeb..299cc09 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1066,6 +1066,7 @@ struct file_lock {
 	fl_owner_t fl_owner;
 	unsigned char fl_flags;
 	unsigned char fl_type;
+	unsigned char fl_type_prev;
 	unsigned int fl_pid;
 	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
-- 
1.6.0.4

Re: [PATCH 11/17][cr][v4]: Add ->fl_type_prev field.

[Oren Laadan]

On 08/16/2010 03:43 PM, Sukadev Bhattiprolu wrote:
> In preparation for checkpoint/restart of file leases, add ->fl_type_prev
> field to 'struct file_lock'. This field is needed to correctly restore
> file leases in case of recursive checkpoint/restart of an in-progress
> lease.

Nit: since it isn't obvious why fl_type_prev field exists,
maybe add this comment also inside fs.h  (so that future
readers won't need to look it up in the git history) ?

Oren.

> 
> This ->fl_type_prev is only initialized in this patch. It will actually
> be used when restoring file leases after a checkpoint.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  fs/locks.c         |   10 ++++++++++
>  include/linux/fs.h |    1 +
>  2 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 0bd5af7..9a00876 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -184,6 +184,7 @@ void locks_init_lock(struct file_lock *fl)
>  	fl->fl_file = NULL;
>  	fl->fl_flags = 0;
>  	fl->fl_type = 0;
> +	fl->fl_type_prev = 0;
>  	fl->fl_start = fl->fl_end = 0;
>  	fl->fl_break_time = 0UL;
>  	fl->fl_ops = NULL;
> @@ -227,6 +228,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
>  	new->fl_file = NULL;
>  	new->fl_flags = fl->fl_flags;
>  	new->fl_type = fl->fl_type;
> +	new->fl_type_prev = fl->fl_type_prev;
>  	new->fl_start = fl->fl_start;
>  	new->fl_end = fl->fl_end;
>  	new->fl_break_time = 0UL;
> @@ -293,6 +295,13 @@ static int assign_type(struct file_lock *fl, int type)
>  	case F_WRLCK:
>  	case F_UNLCK:
>  		fl->fl_type = type;
> +		/*
> +		 * Clear ->fl_type_prev since this is a new lease type.
> +		 * break_lease() will use this cleared state to know
> +		 * if it must save the lease-type in case of checkpoint/
> +		 * restart.
> +		 */
> +		fl->fl_type_prev = 0;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -1222,6 +1231,7 @@ int __break_lease(struct inode *inode, unsigned int mode)
>  
>  	for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) {
>  		if (fl->fl_type != future) {
> +			fl->fl_type_prev = fl->fl_type;
>  			fl->fl_type = future;
>  			fl->fl_break_time = break_time;
>  			/* lease must have lmops break callback */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 49d4eeb..299cc09 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1066,6 +1066,7 @@ struct file_lock {
>  	fl_owner_t fl_owner;
>  	unsigned char fl_flags;
>  	unsigned char fl_type;
> +	unsigned char fl_type_prev;
>  	unsigned int fl_pid;
>  	struct pid *fl_nspid;
>  	wait_queue_head_t fl_wait;

[PATCH 12/17][cr][v4]: Add ->fl_break_notified field.

[Sukadev Bhattiprolu]

The file_lock->fl_break_notified field will be used when checkpointing and
restarting a file-lease. It is needed if the application was checkpointed
during the lease-break-protocol, to "remember" if the application was notified
of the lease-break.

This patch just initializes the ->fl_break_notified field. It will be used
in a follow-on patch, when checkpoint/restart of file-leases are implemented.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c         |    7 +++++++
 include/linux/fs.h |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 9a00876..b5eb4c0 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -185,6 +185,7 @@ void locks_init_lock(struct file_lock *fl)
 	fl->fl_flags = 0;
 	fl->fl_type = 0;
 	fl->fl_type_prev = 0;
+	fl->fl_break_notified = 0;
 	fl->fl_start = fl->fl_end = 0;
 	fl->fl_break_time = 0UL;
 	fl->fl_ops = NULL;
@@ -230,6 +231,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
 	new->fl_type = fl->fl_type;
 	new->fl_type_prev = fl->fl_type_prev;
 	new->fl_start = fl->fl_start;
+	new->fl_break_notified = fl->fl_break_notified;
 	new->fl_end = fl->fl_end;
 	new->fl_break_time = 0UL;
 	new->fl_ops = NULL;
@@ -458,6 +460,7 @@ static int lease_init(struct file *filp, int type, struct file_lock *fl)
 
 	fl->fl_file = filp;
 	fl->fl_flags = FL_LEASE;
+	fl->fl_break_notified = 0;
 	fl->fl_start = 0;
 	fl->fl_end = OFFSET_MAX;
 	fl->fl_ops = NULL;
@@ -1141,6 +1144,8 @@ int lease_modify(struct file_lock **before, int arg)
 	struct file_lock *fl = *before;
 	int error = assign_type(fl, arg);
 
+	fl->fl_break_notified = 0;
+
 	if (error)
 		return error;
 	locks_wake_up_blocks(fl);
@@ -1234,8 +1239,10 @@ int __break_lease(struct inode *inode, unsigned int mode)
 			fl->fl_type_prev = fl->fl_type;
 			fl->fl_type = future;
 			fl->fl_break_time = break_time;
+
 			/* lease must have lmops break callback */
 			fl->fl_lmops->fl_break(fl);
+			fl->fl_break_notified = 1;
 		}
 	}
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 299cc09..c21f002 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1067,6 +1067,7 @@ struct file_lock {
 	unsigned char fl_flags;
 	unsigned char fl_type;
 	unsigned char fl_type_prev;
+	unsigned char fl_break_notified;
 	unsigned int fl_pid;
 	struct pid *fl_nspid;
 	wait_queue_head_t fl_wait;
-- 
1.6.0.4

Re: [PATCH 12/17][cr][v4]: Add ->fl_break_notified field.

[Oren Laadan]

Same nit as for previous patch :)

Oren.

On 08/16/2010 03:43 PM, Sukadev Bhattiprolu wrote:
> The file_lock->fl_break_notified field will be used when checkpointing and
> restarting a file-lease. It is needed if the application was checkpointed
> during the lease-break-protocol, to "remember" if the application was notified
> of the lease-break.
> 
> This patch just initializes the ->fl_break_notified field. It will be used
> in a follow-on patch, when checkpoint/restart of file-leases are implemented.
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> ---
>  fs/locks.c         |    7 +++++++
>  include/linux/fs.h |    1 +
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 9a00876..b5eb4c0 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -185,6 +185,7 @@ void locks_init_lock(struct file_lock *fl)
>  	fl->fl_flags = 0;
>  	fl->fl_type = 0;
>  	fl->fl_type_prev = 0;
> +	fl->fl_break_notified = 0;
>  	fl->fl_start = fl->fl_end = 0;
>  	fl->fl_break_time = 0UL;
>  	fl->fl_ops = NULL;
> @@ -230,6 +231,7 @@ void __locks_copy_lock(struct file_lock *new, const struct file_lock *fl)
>  	new->fl_type = fl->fl_type;
>  	new->fl_type_prev = fl->fl_type_prev;
>  	new->fl_start = fl->fl_start;
> +	new->fl_break_notified = fl->fl_break_notified;
>  	new->fl_end = fl->fl_end;
>  	new->fl_break_time = 0UL;
>  	new->fl_ops = NULL;
> @@ -458,6 +460,7 @@ static int lease_init(struct file *filp, int type, struct file_lock *fl)
>  
>  	fl->fl_file = filp;
>  	fl->fl_flags = FL_LEASE;
> +	fl->fl_break_notified = 0;
>  	fl->fl_start = 0;
>  	fl->fl_end = OFFSET_MAX;
>  	fl->fl_ops = NULL;
> @@ -1141,6 +1144,8 @@ int lease_modify(struct file_lock **before, int arg)
>  	struct file_lock *fl = *before;
>  	int error = assign_type(fl, arg);
>  
> +	fl->fl_break_notified = 0;
> +
>  	if (error)
>  		return error;
>  	locks_wake_up_blocks(fl);
> @@ -1234,8 +1239,10 @@ int __break_lease(struct inode *inode, unsigned int mode)
>  			fl->fl_type_prev = fl->fl_type;
>  			fl->fl_type = future;
>  			fl->fl_break_time = break_time;
> +
>  			/* lease must have lmops break callback */
>  			fl->fl_lmops->fl_break(fl);
> +			fl->fl_break_notified = 1;
>  		}
>  	}
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 299cc09..c21f002 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1067,6 +1067,7 @@ struct file_lock {
>  	unsigned char fl_flags;
>  	unsigned char fl_type;
>  	unsigned char fl_type_prev;
> +	unsigned char fl_break_notified;
>  	unsigned int fl_pid;
>  	struct pid *fl_nspid;
>  	wait_queue_head_t fl_wait;

[PATCH 13/17][cr][v4]: Add jiffies_begin field to ckpt_ctx

[Sukadev Bhattiprolu]

'jiffies_begin' is needed to compute relative time-offsets after restart.
We cannot use ->ktime_begin for these computations since, as pointed out
by John Stultz, converting jiffies to CLOCK_MONOTONIC can result in
incorrect values.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 include/linux/checkpoint_types.h |    1 +
 kernel/checkpoint/sys.c          |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/checkpoint_types.h b/include/linux/checkpoint_types.h
index 3ffe9bd..aba9380 100644
--- a/include/linux/checkpoint_types.h
+++ b/include/linux/checkpoint_types.h
@@ -33,6 +33,7 @@ struct ckpt_ctx {
 	int crid;		/* unique checkpoint id */
 
 	ktime_t ktime_begin;	/* checkpoint start time */
+	unsigned long jiffies_begin;
 
 	int root_init;				/* [container] root init ? */
 	pid_t root_pid;				/* [container] root pid */
diff --git a/kernel/checkpoint/sys.c b/kernel/checkpoint/sys.c
index b761ec0..0beeee5 100644
--- a/kernel/checkpoint/sys.c
+++ b/kernel/checkpoint/sys.c
@@ -278,6 +278,7 @@ static struct ckpt_ctx *ckpt_ctx_alloc(int fd, unsigned long uflags,
 	ctx->uflags = uflags;
 	ctx->kflags = kflags;
 	ctx->ktime_begin = ktime_get();
+	ctx->jiffies_begin = jiffies;
 	ctx->coord_pidns = get_pid_ns(current->nsproxy->pid_ns);
 
 	atomic_set(&ctx->refcount, 0);
-- 
1.6.0.4

[PATCH 14/17][cr][v4]: Checkpoint file-leases

[Sukadev Bhattiprolu]

Build upon the C/R of file-locks to checkpoint file-leases. This patch
simply checkpoints the file-lease information. A follow-on patch
will use this checkpoint information to restorethe file leases.

C/R of file leases depends on whether the lease is currently being broken
(i.e F_INPROGRESS is set) or not.  If the file-lease is not being broken,
checkpoint of file-lease is identical to checkpoint of file-locks.

But if the lease is being broken, we checkpoint additional information,
such as:
        - the previous lease type,
        - the time remaining in the lease, and
        - whether we already notified the lease-holder about the lease-break

See follow-on patch ("Restore file-leases") for more details on how this
information is used while restoring file leases and for other semantics
relating to file-leases.

Changelog[v4]:
	- [Oren Laadan]: Minor changes due to the fact that we now checkpoint
	  the count of file-locks rather than a "marker-lock".
Changelog[v3]:
	- [Oren Laadan, John Stultz]: Use ->jiffies_begin to compute
	  remaining lease
        - Broke-up the draft-patch from previous version into smaller
          patches and addressed comments from Oren Laadan.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/checkpoint.c                |   29 +++++++++++++++++++++++++----
 include/linux/checkpoint_hdr.h |    3 +++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/fs/checkpoint.c b/fs/checkpoint.c
index 540f831..8a4cd23 100644
--- a/fs/checkpoint.c
+++ b/fs/checkpoint.c
@@ -267,13 +267,24 @@ static int checkpoint_file(struct ckpt_ctx *ctx, void *ptr)
 	return ret;
 }
 
+static int get_rem_lease(struct ckpt_ctx *ctx, struct file_lock *lock)
+{
+	int rem_lease;
+
+	rem_lease = 0;
+	if ((lock->fl_type & F_INPROGRESS))
+		rem_lease = (lock->fl_break_time - ctx->jiffies_begin) / HZ;
+
+	return rem_lease;
+}
+
 static int checkpoint_one_file_lock(struct ckpt_ctx *ctx, 
 		struct file_lock *lock)
 {
 	int rc;
 	struct ckpt_hdr_file_lock *h;
 
-	if (!IS_POSIX(lock)) {
+	if (!IS_POSIX(lock) && !IS_LEASE(lock)) {
 		/* Hmm, we should have caught this while counting locks */
 		return -EBADF;
 	}
@@ -284,8 +295,14 @@ static int checkpoint_one_file_lock(struct ckpt_ctx *ctx,
 
 	h->fl_start = lock->fl_start;
 	h->fl_end = lock->fl_end;
+	/* checkpoint F_INPROGRESS also, if set */
+	h->fl_type_prev = lock->fl_type_prev;
 	h->fl_type = lock->fl_type;
 	h->fl_flags = lock->fl_flags;
+	if (IS_LEASE(lock)) {
+		h->fl_break_notified = lock->fl_break_notified;
+		h->fl_rem_lease = get_rem_lease(ctx, lock);
+	}
 
 	rc = ckpt_write_obj(ctx, &h->h);
 
@@ -335,13 +352,17 @@ checkpoint_file_locks(struct ckpt_ctx *ctx, struct files_struct *files,
 		if (lockp->fl_owner != files)
 			continue;
 
-		ckpt_debug("Lock [%lld, %lld, %d, 0x%x]\n", lockp->fl_start,
-				lockp->fl_end, lockp->fl_type, lockp->fl_flags);
+		ckpt_debug("Lock [%lld, %lld, %d, 0x%x], type_prev %d, "
+				"break-notified %d, rem-lease %d\n",
+				lockp->fl_start, lockp->fl_end, lockp->fl_type,
+				lockp->fl_flags, lockp->fl_type_prev,
+				lockp->fl_break_notified,
+				get_rem_lease(ctx, lockp));
 
 		if (lockp->fl_owner != files)
 			continue;
 
-		if (IS_POSIX(lockp))
+		if (IS_POSIX(lockp) || IS_LEASE(lockp))
 			n++;
 		else {
 			ckpt_err(ctx,  rc, "%(T), checkpoint of lock "
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 14b287f..2a54a89 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -600,7 +600,10 @@ struct ckpt_hdr_file_lock {
        __s64 fl_start;
        __s64 fl_end;
        __u8 fl_type;
+       __u8 fl_type_prev;
        __u8 fl_flags;
+       __u8 fl_break_notified;
+       __s32 fl_rem_lease;
 };
 
 struct ckpt_hdr_file_pipe {
-- 
1.6.0.4

[PATCH 15/17][cr][v4]: Define do_setlease()

[Sukadev Bhattiprolu]

Move the core functionality of fcntl_setlease() into a new function,
do_setlease(). do_setlease() is, same as fcntl_setlease(), except for
two new parameters, 'notified' and 'break_time'.

do_setlease() is used, in a follow-on patch when restoring file leases.
These new parameters are needed if the lease was being broken when the
process was checkpointed.

The parameter 'notified' is used to notify the lease-holder exactly once,
even if the lease was checkpointed during a lease-break and then restarted.

The parameter, 'break_time' specifies the new break-time for the lease.
(i.e if the process had 10-seconds remaining on the lease when it was
checkpointed, the 'break_time' is used give the process 10 seconds on
the lease after the process is restarted).

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/locks.c         |   31 ++++++++++++++++++++-----------
 include/linux/fs.h |    2 ++
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index b5eb4c0..6e84d90 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1490,17 +1490,8 @@ int vfs_setlease(struct file *filp, long arg, struct file_lock **lease)
 }
 EXPORT_SYMBOL_GPL(vfs_setlease);
 
-/**
- *	fcntl_setlease	-	sets a lease on an open file
- *	@fd: open file descriptor
- *	@filp: file pointer
- *	@arg: type of lease to obtain
- *
- *	Call this fcntl to establish a lease on the file.
- *	Note that you also need to call %F_SETSIG to
- *	receive a signal when the lease is broken.
- */
-int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
+int do_setlease(unsigned int fd, struct file *filp, long arg, 
+			unsigned long break_time, int notified)
 {
 	struct file_lock fl, *flp = &fl;
 	struct inode *inode = filp->f_path.dentry->d_inode;
@@ -1511,6 +1502,9 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
 	if (error)
 		return error;
 
+	fl.fl_break_notified = notified;
+	fl.fl_break_time = break_time;
+
 	lock_kernel();
 
 	error = vfs_setlease(filp, arg, &flp);
@@ -1534,6 +1528,21 @@ out_unlock:
 }
 
 /**
+ *	fcntl_setlease	-	sets a lease on an open file
+ *	@fd: open file descriptor
+ *	@filp: file pointer
+ *	@arg: type of lease to obtain
+ *
+ *	Call this fcntl to establish a lease on the file.
+ *	Note that you also need to call %F_SETSIG to
+ *	receive a signal when the lease is broken.
+ */
+int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
+{
+	return do_setlease(fd, filp, arg, 0UL, 0);
+}
+
+/**
  * flock_lock_file_wait - Apply a FLOCK-style lock to a file
  * @filp: The file to apply the lock to
  * @fl: The lock to be applied
diff --git a/include/linux/fs.h b/include/linux/fs.h
index c21f002..5056477 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1124,6 +1124,8 @@ extern int flock64_set(unsigned int, struct file *, unsigned int,
 			struct flock64 *);
 #endif
 
+extern int do_setlease(unsigned int fd, struct file *filp, long arg,
+			unsigned long break_time, int notified);
 extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
 extern int fcntl_getlease(struct file *filp);
 
-- 
1.6.0.4

[PATCH 16/17][cr][v4]: Restore file-leases

[Sukadev Bhattiprolu]

Restart an application with file-leases, from its checkpoint.

Restart of file-lease that is not being broken (i.e F_INPROGRESS is not set)
is almost identical to C/R of file-locks. i.e save the type of lease for the
file in the checkpoint image and when restarting, restore the lease by calling
do_setlease().

C/R of file-lease gets complicated (I think), if a process is checkpointed
when its lease was being revoked. i.e if P1 has a F_WRLCK lease on file F1
and P2 opens F1 for write, P2's open is blocked for lease_break_time (45 secs).
P1's lease is revoked (i.e set to F_UNLCK) and P1 is notified via a SIGIO to
flush any dirty data.

Basic design:

To restore a lease that is being broken, we temporarily re-assign the original
lease type (that we saved in ->fl_type_prev) to the lease-holder. i.e. in the
above example, give P1 a F_WRLCK lease). When the lease-breaker (P2) is
restarted after checkpoint, its open() system fails with -ERESTARTSYS and it
will retry the open(). This open() will re-initiate the lease-break protocol
(i.e P2 will go back to waiting and P1 will be notified).

Some observations about this approach:

1. We must use ->fl_type_prev because, when the lease is being broken,
  ->fl_type is already set to F_UNLCK and would not result in a
  lease-break protocol when P2 is restarted.

2. When the lease-break is initiated and we signal the lease-holder, we set
   the ->fl_break_notified field. When restarting the lease and repeating
   the lease-break protocol, we check the ->fl_break_notified field and
   signal the lease-holder only if did not signal before the checkpoint.

3. If P1 was was checkpointed 40 seconds into the lease_break_time,(i.e.
   it had 5 seconds remaining in the lease), we would ideally want to ensure
   that after restart, P1 gets 5 or at least 5 seconds to finish cleaning up
   the lease.

   But the actual time that P1 gets after the application is restarted
   depends on many factors (number of processes in the application
   process tree, load on system at the time of restart etc).

   Jamie Lokier had suggested that we favor the lease-holder (P1) during
   restart, even if it meant giving the lease-holder the entire lease-break
   interval (45 seconds) again after the restart. Oren Laadan suggested
   that rather than make that a kernel policy, we let the user choose a
   policy based on the application's behavior.

   The current patchset computes and checkpoints the remaining-lease and
   uses this value to restore the lease. i.e the kernel simply uses the
   "remaining-lease" value stored in the checkpoint image. Userspace tools
   can be developed to alter the remaining-lease value in the checkpoint
   image to either favor the lease-holder or the lease-breaker or to add
   a fixed delta.

4. The above design of C/R of file-leases assumes that both lease-holder
   and lease-breaker are restarted. If only the lease-holder is
   restarted, the kernel will re-assign the original lease (F_WRLCK in
   the example) to lease-holder. If no lease-breaker comes along, the
   kernel will leave the lease assigned to lease-holder.

   This should not be a problem because, as far as the lease-holder is
   concerned the lease was revoked and it will/should reacquire the
   lease.

Changelog[v4]:
	- [Oren Laadan] Minor changes to reflect that we now checkpoint the
	  count of file-locks rather than a marker-lock

Changelog[v3]:

	- Broke-up patchset into smaller patches and addressed comments
	  from Oren Laadan, Jamie Lokier.

Changelog[v2]:
	- comments from Matt Helsley, Serge Hallyn...

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 fs/checkpoint.c |   32 +++++++++++++++++++++---
 fs/locks.c      |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 96 insertions(+), 7 deletions(-)

diff --git a/fs/checkpoint.c b/fs/checkpoint.c
index 8a4cd23..5577e99 100644
--- a/fs/checkpoint.c
+++ b/fs/checkpoint.c
@@ -1150,20 +1150,44 @@ static int restore_file_locks(struct ckpt_ctx *ctx, struct file *file, int fd)
 			goto out;
 		}
 
-		ckpt_debug("Lock [%lld, %lld, %d, 0x%x]\n", h->fl_start,
-				h->fl_end, (int)h->fl_type, h->fl_flags);
+		ckpt_debug("Lock [%lld, %lld, %d, 0x%x], fl_type_prev %d, "
+				"fl_break_notified %d\n", h->fl_start,
+				h->fl_end, (int)h->fl_type, h->fl_flags,
+				h->fl_type_prev, h->fl_break_notified);
 
 		ret = -EBADF;
 		if (h->fl_flags & FL_POSIX)
 			ret = restore_one_posix_lock(ctx, file, fd, h); 
 
-		ckpt_hdr_put(ctx, h);
+		else if (h->fl_flags & FL_LEASE) {
+			int type;
+			unsigned long bt;
+
+			type = h->fl_type;
+			if (h->fl_type & F_INPROGRESS)
+				type = h->fl_type_prev;
+
+			/*
+			 * Use ->fl_rem_lease to compute new break time in
+			 * jiffies relative to ->jiffies_begin.
+			 */
+			bt = ctx->jiffies_begin + (h->fl_rem_lease * HZ);
+
+			ckpt_debug("current-jiffies %lu, jiffies_begin %lu, "
+					"break-time-jiffies %lu\n", jiffies,
+					ctx->jiffies_begin, bt);
 
+			ret = do_setlease(fd, file, type, bt,
+						h->fl_break_notified);
+			if (ret)
+				ckpt_err(ctx, ret, "do_setlease(): %d\n", type);
+		}
+
+		ckpt_hdr_put(ctx, h);
 		if (ret < 0) {
 			ckpt_err(ctx, ret, "%(T)\n");
 			goto out;
 		}
-
 	}
 out:
 	ckpt_hdr_put(ctx, hfc);
diff --git a/fs/locks.c b/fs/locks.c
index 6e84d90..b0640b9 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1183,6 +1183,16 @@ static void time_out_leases(struct inode *inode)
  *	some kind of lock (maybe a lease) on this file.  Leases are broken on
  *	a call to open() or truncate().  This function can sleep unless you
  *	specified %O_NONBLOCK to your open().
+ *
+ *	Checkpoint/restart: Suppose 10 seconds remain in a lease when the
+ *		lease-holder is checkpointed. When the lease-holder is
+ *		restarted, it should probably only get 10-seconds of the
+ *		lease, but we favor the lease-holder and allow it to to
+ *		have entire lease-break-time again. This can of course
+ *		cause the lease-breaker(s) to starve if the application is
+ *		repeatedly checkpointed during the lease-break protocol,
+ *		but that is hopefully not a common occurence.
+ *
  */
 int __break_lease(struct inode *inode, unsigned int mode)
 {
@@ -1236,12 +1246,38 @@ int __break_lease(struct inode *inode, unsigned int mode)
 
 	for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) {
 		if (fl->fl_type != future) {
-			fl->fl_type_prev = fl->fl_type;
+
+			/*
+			 * If ->fl_type_prev is already set, we could be in a
+			 * recursive checkpoint/restart. i.e. we were
+			 * checkpointed once when our lease was being broken.
+			 * We were then restarted from the checkpoint and
+			 * immediately checkpointed again before the restored
+			 * lease expired. In this case, we want to restore
+			 * the lease to the original type. So don't overwrite
+			 * ->fl_type_prev if its already set.
+			 */
+			if (!fl->fl_type_prev)
+				fl->fl_type_prev = fl->fl_type;
+
 			fl->fl_type = future;
-			fl->fl_break_time = break_time;
+			/*
+			 * If this lease was restored from a checkpoint (i.e.
+			 * ->fl_break_time is set), don't change the remaining
+			 *  time on the lease.
+			 */
+			if (!fl->fl_break_time)
+				fl->fl_break_time = break_time;
+
+			/*
+			 * Similarly, if we already notified the lease-holder
+			 * before the checkpoint, i.e. ->fl_break_notified is
+			 * set, don't notify again.
+			 */
 
 			/* lease must have lmops break callback */
-			fl->fl_lmops->fl_break(fl);
+			if (!fl->fl_break_notified)
+				fl->fl_lmops->fl_break(fl);
 			fl->fl_break_notified = 1;
 		}
 	}
@@ -1490,6 +1526,35 @@ int vfs_setlease(struct file *filp, long arg, struct file_lock **lease)
 }
 EXPORT_SYMBOL_GPL(vfs_setlease);
 
+/*
+ * do_setlease():
+ *
+ *
+ * Checkpoint/restart notes:
+ *
+ * 	If this lease was from a checkpoint, we assume that the lease-breaker
+ * 	was also checkpointed, and we re-assign the lease to the lease holder.
+ *
+ * 	When the lease-breaker is restarted, it will retry the open() and thus
+ * 	initiate the lease-break protocol again. But to avoid confusing the
+ * 	application, we follow the entire lease-break protocol except that we
+ * 	signal the application only if it was not signalled before.
+ *
+ * 	Note that these semantics assume that the lease-breaker will come along
+ * 	and reclaim the lease. If that does not happen, (either because the
+ * 	lease-breaker was not checkpointed or is not being restarted at the
+ * 	same time as the lease-holder) the kernel will leave the lease assigned
+ * 	to the lease-holder, even though the application assumes it no longer
+ * 	has the lease.
+ *
+ * 	But this discrepancy will not cause a problem for the lease-holder,
+ * 	since the lease-holder "knows" it does not have the lease and will
+ * 	have to reacquire the lease.
+ *
+ * 	This semantics has a down-side to any new lease-breaker in that they
+ * 	will be blocked for the lease_break_time, even if the lease-holder no
+ * 	longer has the lease.
+ */
 int do_setlease(unsigned int fd, struct file *filp, long arg, 
 			unsigned long break_time, int notified)
 {
-- 
1.6.0.4

[PATCH 17/17][cr][v4]: Document design of C/R of file-locks and leases

[Sukadev Bhattiprolu]

Summarize the file-system consistency requirements and the design of
the C/R of file-locks and leases.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 Documentation/checkpoint/file-locks |  126 +++++++++++++++++++++++++++++++++++
 1 files changed, 126 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/checkpoint/file-locks

diff --git a/Documentation/checkpoint/file-locks b/Documentation/checkpoint/file-locks
new file mode 100644
index 0000000..e562990
--- /dev/null
+++ b/Documentation/checkpoint/file-locks
@@ -0,0 +1,126 @@
+
+Filesystem consistency across C/R.
+==================================
+
+To checkpoint/restart a process that is using any filesystem resource, the
+kernel assumes that the file system state at the time of restart is consistent
+with its state at the time of checkpoint. In general, this consistency can be
+achieved by:
+
+	a. running the application inside a container (to ensure no process
+	   outside the container modifies the filesystem/IPC or other states)
+
+	b. freezing the application before checkpoint
+	c. taking a snapshot of the file system while application is frozen
+	d. checkpointing the application while it is frozen
+
+	e. restoring the file system state to its snapshot
+	f. restart the application inside a container
+
+i.e the kernel assumes that file system state is consistent but it does/can
+NOT verify that it is. The administrator must provide this consistency taking
+into account the file system type including whether it is local or remote,
+and the tools available in the file system (snapshot tools in btrfs or rsync
+etc).
+
+For distributed applications operating on distributed filesystems, it is
+expected that an external mechanism will coordinate the freeze/checkpoint/
+snapshot/restart across the nodes. IOW, the current semantics in the kernel
+provide for C/R on a single node.
+
+Checkpoint/restart of file-locks.
+================================
+
+To checkpoint file-locks in an application, we start with each file-descriptor
+and count the number of file-locks on that file-descriptor. We save this count
+in the checkpoint image, and then information about each file-lock on the
+file-descriptor.
+
+When restarting the application from the checkpoint, we read the file-lock
+count for each file-descriptor and then read the information about each
+file-lock. For each file-lock, we call flock_set() to set a new file-lock.
+
+No special handling is necessary for a process P2 in the checkpointed container
+that is blocked on a file-lock, L1 held by another process P1. Processes in the
+restarted container begin execution only after all processes have restored.
+If the blocked process P2 is restored first, it will prepare to return an
+-ERESTARTSYS from the fcntl() system call, but wait for P1 to be restored.
+When P1 is restored, it will re-acquire the file-lock L1 before P1 and P2 begin
+actual execution.
+
+This ensures that even if P2 is scheduled to run before P1, P2 will go
+back to waiting for the file-lock L1.
+
+Checkpoint/restart of file leases
+==================================
+
+C/R of file-leases depends on whether the lease is currently being broken
+(i.e F_INPROGRESS is set).  If the file-lease is not being broken, checkpoint/
+restart of file-lease is identical to checkpoint of file-locks (i.e save
+the type of the lease for the file in the checkpoint image. When restarting,
+restore the lease by calling do_setlease().
+
+C/R of file-lease gets complicated, if a process is checkpointed when its lease
+was being revoked. i.e if P1 has a F_WRLCK lease on file F1 and P2 opens F1 for
+write, P2's open is blocked for lease_break_time (45 secs).  P1's lease is
+revoked (i.e set to F_UNLCK) and P1 is notified via a SIGIO to flush any dirty
+data.
+
+Basic design:
+
+To address "in-progress" leases, we checkpoint additional information about
+the lease:
+
+        - the previous lease type (file_lock->fl_type_prev)
+        - the time remaining in the lease (->fl_rem_lease), and
+        - whether we already notified the lease-holder about the lease-break
+	  (->fl_break_notified)
+
+To restore an "in-progrss" lease that, we temporarily re-assign the original
+lease type (that we saved in ->fl_type_prev) to the lease-holder. i.e. in the
+above example, give P1 a F_WRLCK lease). When the lease-breaker (P2) is
+restarted after checkpoint, its open() system fails with -ERESTARTSYS and it
+will retry the open(). This open() will re-initiate the lease-break protocol
+(i.e P2 will go back to waiting and P1 will be notified).
+
+Some observations about this approach:
+
+1. We must use ->fl_type_prev because, when the lease is being broken,
+  ->fl_type is already set to F_UNLCK and would not result in a
+  lease-break protocol when P2 is restarted.
+
+2. When the lease-break is initiated and we signal the lease-holder, we set
+   the ->fl_break_notified field. When restarting the lease and repeating
+   the lease-break protocol, we check the ->fl_break_notified field and
+   signal the lease-holder only if did not signal before the checkpoint.
+
+3. If P1 was was checkpointed 40 seconds into the lease_break_time,(i.e.
+   it had 5 seconds remaining in the lease), we would ideally want to ensure
+   that after restart, P1 gets 5 or at least 5 seconds to finish cleaning up
+   the lease.
+
+   But the actual time that P1 gets after the application is restarted depends
+   on many factors (number of processes in the application process tree, load
+   on system at the time of restart etc).
+
+   Jamie Lokier had suggested that we favor the lease-holder (P1) during
+   restart, even if it meant giving the lease-holder the entire lease-break
+   interval (45 seconds) again after the restart. Oren Laadan suggested
+   that rather than make that a kernel policy, we let the user choose a
+   policy based on the application's behavior.
+
+   The current design computes and checkpoints the remaining-lease and
+   uses this value to restore the lease. i.e the kernel simply uses the
+   "remaining-lease" value stored in the checkpoint image. Userspace tools
+   can be developed to alter the remaining-lease value in the checkpoint
+   image to either favor the lease-holder or the lease-breaker or to add
+   a fixed delta.
+
+4. The current design of C/R of file-leases assumes that both lease-holder
+   and lease-breaker are restarted. If only the lease-holder is restarted,
+   the kernel will re-assign the original lease (F_WRLCK in the example) to
+   lease-holder. If no lease-breaker comes along, the kernel will leave the
+   lease assigned to lease-holder.
+
+   This should not be a problem because, as far as the lease-holder is
+   concerned the lease was revoked and it will/should reacquire the lease.
-- 
1.6.0.4