[RFC] patch to add support for leases to cifs

[RFC] patch to add support for leases to cifs

[Steve French]

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

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cifs-setlease.patch --]
[-- Type: text/x-patch; name=cifs-setlease.patch, Size: 2432 bytes --]

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(

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

[Steve French]

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

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@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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Steve French]

Oplock is a guarantee that no one else is changing (or reading) a
file.  The notification that a file is now changing is different (that
is SMB change notify), Oplock is a guarantee that you are the only
writer (or reader/writer) of the file - so if the server wants to
revoke that (oplock break) - and with the patch we call break_lease
and don't return on the oplock break until break_lease returns,  and
__break_lease in fs/locks.c looks synchronous - break_lease async?

By the way, the server also gives up on the client if it does not
respond (but this is a long time - more than 20 seconds for most
servers) - presumably by flushing the dirty pages, the issues the
oplock break response.

Following is an updated patch to add support for mount option to
disable the check for oplock (since not all server support oplock):

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index c6aad77..76919c2 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -618,6 +618,37 @@ 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 */
+	struct inode *inode = file->f_path.dentry->d_inode;
+
+	if (!(S_ISREG(inode->i_mode)))
+		return -EINVAL;
+
+	/* check if file is oplocked */
+	if (((arg == F_RDLCK) &&
+		(CIFS_I(inode)->clientCanCacheRead)) ||
+	    ((arg == F_WRLCK) &&
+		(CIFS_I(inode)->clientCanCacheAll)))
+		return generic_setlease(file, arg, lease);
+	else if (CIFS_SB(inode->i_sb)->tcon->local_lease &&
+			!CIFS_I(inode)->clientCanCacheRead)
+		/* If the server claims to support oplock on this
+		   file, then we still need to check oplock even
+		   if the local_lease mount option is set, but there
+		   are servers which do not support oplock for which
+		   this mount option may be useful if the user
+		   knows that the file won't be changed on the server
+		   by anyone else */
+		return generic_setlease(file, arg, lease);
+	else
+		return -EAGAIN;
+}
+#endif
+
 struct file_system_type cifs_fs_type = {
 	.owner = THIS_MODULE,
 	.name = "cifs",
@@ -696,6 +727,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 +748,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 +769,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 +789,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 +981,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(
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 178f733..c791e5b 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -285,6 +285,7 @@ struct cifsTconInfo {
 	bool seal:1;      /* transport encryption for this mounted share */
 	bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
 				for this mount even if server would support */
+	bool local_lease:1; /* check leases (only) on local system not remote */
 	/* BB add field for back pointer to sb struct(s)? */
 };

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 1126f7a..17058c5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -90,7 +90,8 @@ struct smb_vol {
 	bool nocase:1;     /* request case insensitive filenames */
 	bool nobrl:1;      /* disable sending byte range locks to srv */
 	bool seal:1;       /* request transport encryption on share */
-	bool nodfs:1;
+	bool nodfs:1;      /* Do not request DFS, even if available */
+	bool local_lease:1; /* check leases only on local system, not remote */
 	unsigned int rsize;
 	unsigned int wsize;
 	unsigned int sockopt;
@@ -1264,6 +1265,8 @@ cifs_parse_mount_options(char *options, const
char *devname,
 			vol->no_psx_acl = 0;
 		} else if (strnicmp(data, "noacl", 5) == 0) {
 			vol->no_psx_acl = 1;
+		} else if (strnicmp(data, "locallease", 6) == 0) {
+			vol->local_lease = 1;
 		} else if (strnicmp(data, "sign", 4) == 0) {
 			vol->secFlg |= CIFSSEC_MUST_SIGN;
 		} else if (strnicmp(data, "seal", 4) == 0) {
@@ -2162,6 +2165,7 @@ cifs_mount(struct super_block *sb, struct
cifs_sb_info *cifs_sb,
 			   for the retry flag is used */
 			tcon->retry = volume_info.retry;
 			tcon->nocase = volume_info.nocase;
+			tcon->local_lease = volume_info.local_lease;
 			if (tcon->seal != volume_info.seal)
 				cERROR(1, ("transport encryption setting "
 					   "conflicts with existing tid"));

On Wed, Oct 22, 2008 at 3:19 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 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.



-- 
Thanks,

Steve

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

[J. Bruce Fields]

On Wed, Oct 22, 2008 at 03:31:06PM -0500, Steve French wrote:
> Oplock is a guarantee that no one else is changing (or reading) a
> file.  The notification that a file is now changing is different (that
> is SMB change notify), Oplock is a guarantee that you are the only
> writer (or reader/writer) of the file - so if the server wants to
> revoke that (oplock break) - and with the patch we call break_lease
> and don't return on the oplock break until break_lease returns,  and
> __break_lease in fs/locks.c looks synchronous - break_lease async?

Ah!  Yes, you're right.  nfsd always calls break_lease with
O_NONBLOCK....

Makes sense to me, then.

> By the way, the server also gives up on the client if it does not
> respond (but this is a long time - more than 20 seconds for most
> servers) - presumably by flushing the dirty pages, the issues the
> oplock break response.

So it's a little irritating that the server's timeout may not agree with
the local timeout set by that sysctl.  I suppose the lease timeout
should really be a characteristic of the filesystem rather than a global
thing.

--b.

> 
> Following is an updated patch to add support for mount option to
> disable the check for oplock (since not all server support oplock):
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index c6aad77..76919c2 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -618,6 +618,37 @@ 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 */
> +	struct inode *inode = file->f_path.dentry->d_inode;
> +
> +	if (!(S_ISREG(inode->i_mode)))
> +		return -EINVAL;
> +
> +	/* check if file is oplocked */
> +	if (((arg == F_RDLCK) &&
> +		(CIFS_I(inode)->clientCanCacheRead)) ||
> +	    ((arg == F_WRLCK) &&
> +		(CIFS_I(inode)->clientCanCacheAll)))
> +		return generic_setlease(file, arg, lease);
> +	else if (CIFS_SB(inode->i_sb)->tcon->local_lease &&
> +			!CIFS_I(inode)->clientCanCacheRead)
> +		/* If the server claims to support oplock on this
> +		   file, then we still need to check oplock even
> +		   if the local_lease mount option is set, but there
> +		   are servers which do not support oplock for which
> +		   this mount option may be useful if the user
> +		   knows that the file won't be changed on the server
> +		   by anyone else */
> +		return generic_setlease(file, arg, lease);
> +	else
> +		return -EAGAIN;
> +}
> +#endif
> +
>  struct file_system_type cifs_fs_type = {
>  	.owner = THIS_MODULE,
>  	.name = "cifs",
> @@ -696,6 +727,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 +748,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 +769,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 +789,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 +981,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(
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 178f733..c791e5b 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -285,6 +285,7 @@ struct cifsTconInfo {
>  	bool seal:1;      /* transport encryption for this mounted share */
>  	bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
>  				for this mount even if server would support */
> +	bool local_lease:1; /* check leases (only) on local system not remote */
>  	/* BB add field for back pointer to sb struct(s)? */
>  };
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1126f7a..17058c5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -90,7 +90,8 @@ struct smb_vol {
>  	bool nocase:1;     /* request case insensitive filenames */
>  	bool nobrl:1;      /* disable sending byte range locks to srv */
>  	bool seal:1;       /* request transport encryption on share */
> -	bool nodfs:1;
> +	bool nodfs:1;      /* Do not request DFS, even if available */
> +	bool local_lease:1; /* check leases only on local system, not remote */
>  	unsigned int rsize;
>  	unsigned int wsize;
>  	unsigned int sockopt;
> @@ -1264,6 +1265,8 @@ cifs_parse_mount_options(char *options, const
> char *devname,
>  			vol->no_psx_acl = 0;
>  		} else if (strnicmp(data, "noacl", 5) == 0) {
>  			vol->no_psx_acl = 1;
> +		} else if (strnicmp(data, "locallease", 6) == 0) {
> +			vol->local_lease = 1;
>  		} else if (strnicmp(data, "sign", 4) == 0) {
>  			vol->secFlg |= CIFSSEC_MUST_SIGN;
>  		} else if (strnicmp(data, "seal", 4) == 0) {
> @@ -2162,6 +2165,7 @@ cifs_mount(struct super_block *sb, struct
> cifs_sb_info *cifs_sb,
>  			   for the retry flag is used */
>  			tcon->retry = volume_info.retry;
>  			tcon->nocase = volume_info.nocase;
> +			tcon->local_lease = volume_info.local_lease;
>  			if (tcon->seal != volume_info.seal)
>  				cERROR(1, ("transport encryption setting "
>  					   "conflicts with existing tid"));
> 
> On Wed, Oct 22, 2008 at 3:19 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > 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.
> 
> 
> 
> -- 
> Thanks,
> 
> Steve

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

[J. Bruce Fields]

On Wed, Oct 22, 2008 at 05:47:46PM -0400, J. Bruce Fields wrote:
> On Wed, Oct 22, 2008 at 03:31:06PM -0500, Steve French wrote:
> > Oplock is a guarantee that no one else is changing (or reading) a
> > file.  The notification that a file is now changing is different (that
> > is SMB change notify), Oplock is a guarantee that you are the only
> > writer (or reader/writer) of the file - so if the server wants to
> > revoke that (oplock break) - and with the patch we call break_lease
> > and don't return on the oplock break until break_lease returns,  and
> > __break_lease in fs/locks.c looks synchronous - break_lease async?
> 
> Ah!  Yes, you're right.  nfsd always calls break_lease with
> O_NONBLOCK....
> 
> Makes sense to me, then.
> 
> > By the way, the server also gives up on the client if it does not
> > respond (but this is a long time - more than 20 seconds for most
> > servers) - presumably by flushing the dirty pages, the issues the
> > oplock break response.
> 
> So it's a little irritating that the server's timeout may not agree with
> the local timeout set by that sysctl.  I suppose the lease timeout
> should really be a characteristic of the filesystem rather than a global
> thing.

More annoying: if there's a local lease, and someone local breaks it,
then I think they're going to block in break_lease() up to the time
given by that sysctl, before they even call into the filesystem.

So a little better coordination with the filesystem is going to be
needed before we get lease breaking completely right.

--b.

> 
> --b.
> 
> > 
> > Following is an updated patch to add support for mount option to
> > disable the check for oplock (since not all server support oplock):
> > 
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index c6aad77..76919c2 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -618,6 +618,37 @@ 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 */
> > +	struct inode *inode = file->f_path.dentry->d_inode;
> > +
> > +	if (!(S_ISREG(inode->i_mode)))
> > +		return -EINVAL;
> > +
> > +	/* check if file is oplocked */
> > +	if (((arg == F_RDLCK) &&
> > +		(CIFS_I(inode)->clientCanCacheRead)) ||
> > +	    ((arg == F_WRLCK) &&
> > +		(CIFS_I(inode)->clientCanCacheAll)))
> > +		return generic_setlease(file, arg, lease);
> > +	else if (CIFS_SB(inode->i_sb)->tcon->local_lease &&
> > +			!CIFS_I(inode)->clientCanCacheRead)
> > +		/* If the server claims to support oplock on this
> > +		   file, then we still need to check oplock even
> > +		   if the local_lease mount option is set, but there
> > +		   are servers which do not support oplock for which
> > +		   this mount option may be useful if the user
> > +		   knows that the file won't be changed on the server
> > +		   by anyone else */
> > +		return generic_setlease(file, arg, lease);
> > +	else
> > +		return -EAGAIN;
> > +}
> > +#endif
> > +
> >  struct file_system_type cifs_fs_type = {
> >  	.owner = THIS_MODULE,
> >  	.name = "cifs",
> > @@ -696,6 +727,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 +748,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 +769,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 +789,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 +981,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(
> > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> > index 178f733..c791e5b 100644
> > --- a/fs/cifs/cifsglob.h
> > +++ b/fs/cifs/cifsglob.h
> > @@ -285,6 +285,7 @@ struct cifsTconInfo {
> >  	bool seal:1;      /* transport encryption for this mounted share */
> >  	bool unix_ext:1;  /* if false disable Linux extensions to CIFS protocol
> >  				for this mount even if server would support */
> > +	bool local_lease:1; /* check leases (only) on local system not remote */
> >  	/* BB add field for back pointer to sb struct(s)? */
> >  };
> > 
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 1126f7a..17058c5 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -90,7 +90,8 @@ struct smb_vol {
> >  	bool nocase:1;     /* request case insensitive filenames */
> >  	bool nobrl:1;      /* disable sending byte range locks to srv */
> >  	bool seal:1;       /* request transport encryption on share */
> > -	bool nodfs:1;
> > +	bool nodfs:1;      /* Do not request DFS, even if available */
> > +	bool local_lease:1; /* check leases only on local system, not remote */
> >  	unsigned int rsize;
> >  	unsigned int wsize;
> >  	unsigned int sockopt;
> > @@ -1264,6 +1265,8 @@ cifs_parse_mount_options(char *options, const
> > char *devname,
> >  			vol->no_psx_acl = 0;
> >  		} else if (strnicmp(data, "noacl", 5) == 0) {
> >  			vol->no_psx_acl = 1;
> > +		} else if (strnicmp(data, "locallease", 6) == 0) {
> > +			vol->local_lease = 1;
> >  		} else if (strnicmp(data, "sign", 4) == 0) {
> >  			vol->secFlg |= CIFSSEC_MUST_SIGN;
> >  		} else if (strnicmp(data, "seal", 4) == 0) {
> > @@ -2162,6 +2165,7 @@ cifs_mount(struct super_block *sb, struct
> > cifs_sb_info *cifs_sb,
> >  			   for the retry flag is used */
> >  			tcon->retry = volume_info.retry;
> >  			tcon->nocase = volume_info.nocase;
> > +			tcon->local_lease = volume_info.local_lease;
> >  			if (tcon->seal != volume_info.seal)
> >  				cERROR(1, ("transport encryption setting "
> >  					   "conflicts with existing tid"));
> > 
> > On Wed, Oct 22, 2008 at 3:19 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > > 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.
> > 
> > 
> > 
> > -- 
> > Thanks,
> > 
> > Steve
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html