Re: [PATCH] locks: provide a file lease method enabling cluster-coherent leases

[Trond Myklebust <trond.myklebust@fys.uio.no>]

On Thu, 2007-05-31 at 17:40 -0400, J. Bruce Fields wrote:
> From: J. Bruce Fields <bfields@fieldses.org>
> 
> Currently leases are only kept locally, so there's no way for a distributed
> filesystem to enforce them against multiple clients.  We're particularly
> interested in the case of nfsd exporting a cluster filesystem, in which
> case nfsd needs cluster-coherent leases in order to implement delegations
> correctly.
> 
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
>  fs/locks.c         |    5 ++++-
>  include/linux/fs.h |    4 ++++
>  2 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 3f366e1..40a7f39 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -1444,7 +1444,10 @@ int setlease(struct file *filp, long arg, struct file_lock **lease)
>  		return error;
>  
>  	lock_kernel();
> -	error = __setlease(filp, arg, lease);
> +	if (filp->f_op && filp->f_op->set_lease)
> +		error = filp->f_op->set_lease(filp, arg, lease);
> +        else
> +		error = __setlease(filp, arg, lease);
>  	unlock_kernel();
>  
>  	return error;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 7cf0c54..09aefb4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1112,6 +1112,7 @@ struct file_operations {
>  	int (*flock) (struct file *, int, struct file_lock *);
>  	ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, loff_t *, size_t, unsigned int);
>  	ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int);
> +	int (*set_lease)(struct file *, long, struct file_lock **);
>  };
>  
>  struct inode_operations {
> @@ -1137,6 +1138,7 @@ struct inode_operations {
>  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
>  	int (*removexattr) (struct dentry *, const char *);
>  	void (*truncate_range)(struct inode *, loff_t, loff_t);
> +	int (*break_lease)(struct inode *, unsigned int);

Splitting the lease into a file_operation part and an inode_operation
part looks really ugly. It also means that you're calling twice down
into the filesystem for every call to may_open() (once for
vfs_permission() and once for break_lease()) and 3 times in
do_sys_truncate().

Would it perhaps make sense to package up the call to vfs_permission()
and break_lease() as a single 'may_open()' inode operation that could be
called by may_open(), do_sys_truncate() and nfsd?

Trond


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs