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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux