[linux-cifs-client] Re: [RFC] patch to add support for leases to cifs

[J. Bruce Fields]

What guarantees that no other client can modify the file as long as the
lease is held?

It's not enough to break the lease as soon as you notice the file
changes--the holder of the lease has up to 45 seconds (configurable with
/proc/sys/fs/lease-break-time) to return the lease before the kernel
forcibly revokes it.

--b.

On Wed, Oct 22, 2008 at 12:33:54AM -0500, Steve French wrote:
> > Why not include it inlined in the mail body?
> 
> Inlined as requested.
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index c6aad77..aaa6f37 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -618,6 +618,26 @@ static loff_t cifs_llseek(struct file *file,
> loff_t offset, int origin)
>  	return generic_file_llseek_unlocked(file, offset, origin);
>  }
> 
> +#ifdef CONFIG_CIFS_EXPERIMENTAL
> +static int cifs_setlease(struct file *file, long arg, struct file_lock **lease)
> +{
> +	/* note that this is called by vfs setlease with the BKL held
> +	   although I doubt that BKL is needed here in cifs */
> +
> +	if (!(S_ISREG(inode->i_mode))
> +		return -EINVAL;
> +
> +	/* check if file is oplocked */
> +	if (((arg == F_RDLCK) &&
> +		(CIFS_I(file->f_path.dentry->d_inode)->clientCanCacheRead)) ||
> +	    ((arg == F_WRLCK) &&
> +		(CIFS_I(file->f_path.dentry->d_inode)->clientCanCacheAll)))
> +		return generic_setlease(file, arg, lease);
> +	else
> +		return -EAGAIN;
> +}
> +#endif
> +
>  struct file_system_type cifs_fs_type = {
>  	.owner = THIS_MODULE,
>  	.name = "cifs",
> @@ -696,6 +716,7 @@ const struct file_operations cifs_file_ops = {
> 
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>  	.dir_notify = cifs_dir_notify,
> +	.setlease = cifs_setlease,
>  #endif /* CONFIG_CIFS_EXPERIMENTAL */
>  };
> 
> @@ -716,6 +737,7 @@ const struct file_operations cifs_file_direct_ops = {
>  	.llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>  	.dir_notify = cifs_dir_notify,
> +	.setlease = cifs_setlease,
>  #endif /* CONFIG_CIFS_EXPERIMENTAL */
>  };
>  const struct file_operations cifs_file_nobrl_ops = {
> @@ -736,6 +758,7 @@ const struct file_operations cifs_file_nobrl_ops = {
> 
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>  	.dir_notify = cifs_dir_notify,
> +	.setlease = cifs_setlease,
>  #endif /* CONFIG_CIFS_EXPERIMENTAL */
>  };
> 
> @@ -755,6 +778,7 @@ const struct file_operations cifs_file_direct_nobrl_ops = {
>  	.llseek = cifs_llseek,
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>  	.dir_notify = cifs_dir_notify,
> +	.setlease = cifs_setlease,
>  #endif /* CONFIG_CIFS_EXPERIMENTAL */
>  };
> 
> @@ -946,6 +970,12 @@ static int cifs_oplock_thread(void *dummyarg)
>  				the call */
>  			/* mutex_lock(&inode->i_mutex);*/
>  			if (S_ISREG(inode->i_mode)) {
> +#ifdef CONFIG_CIFS_EXPERIMENTAL
> +				if (CIFS_I(inode)->clientCanCacheAll == 0)
> +					break_lease(inode, FMODE_READ);
> +				else if (CIFS_I(inode)->clientCanCacheRead == 0)
> +					break_lease(inode, FMODE_WRITE);
> +#endif
>  				rc = filemap_fdatawrite(inode->i_mapping);
>  				if (CIFS_I(inode)->clientCanCacheRead == 0) {
>  					waitrc = filemap_fdatawait(
> 
> 
> 
> > On Wed, Oct 22, 2008 at 6:03 AM, Steve French <smfrench at gmail.com> wrote:
> >> fcntl(F_SETLEASE) currently is not exported by cifs (nor by local file
> >> systems) so cifs grants leases based on how other local processes have
> >> opened the file not by whether the file is cacheable (oplocked).  This
> >> adds the check to make sure that the file is cacheable on the client
> >> before checking whether we can grant the lease locally
> >> (generic_setlease).
> 
> 
> 
> 
> 
> -- 
> Thanks,
> 
> Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html