[PATCH] Convert Samba to OFD locking - version 2

Jeremy Allison jra at samba.org
Sat May 21 00:07:25 UTC 2016


On Fri, May 20, 2016 at 07:01:18AM -0400, Jeff Layton wrote:
> On Thu, 2016-05-19 at 14:12 -0700, Jeremy Allison wrote:
> > OK, here is version 2 with Uri and Simo's changes
> > incorporated. Compile-time checks now run the test
> > code rather than just compiling, and there's a
> > tuneable "smbd:force process locks" that will force
> > the old-style process specific locks. Advice to
> > use it gets printed in the log if fcntl returns
> > EINVAL.
> > 
> > Please review and push if happy !
> > 
> > Thanks,
> > 
> > Jeremy.
> 
> Looks reasonable to me, though again, I don't know the userland samba
> code that well. So, fwiw...
> 
>     Acked-by: Jeff Layton <jlayton at samba.org>
> 
> ...also attached is an untested patch for cifsfs, which I was thinking
> would do the right thing wrt lockowners. I'll test it out as soon as I
> can put a test rig together for it. In the meantime, thoughts?

Looks exactly right to me. With that and my patches
OFD-locks should work over the wire to Samba with
cifsfs and SMB1. You'll have to wait until my
autobuild lands in master first though (2 unrelated
and random different failures so far, grrr. I keep trying).

SMB2 uses the persistent part of the handle as the
lock context so once we've gotten SMB2+ unix extensions
onboard this should just work out of the box over
SMB3.

> From c976fce960bad85bb97ad62b7d3efc8fe04aafad Mon Sep 17 00:00:00 2001
> From: Jeff Layton <jeff.layton at primarydata.com>
> Date: Fri, 20 May 2016 06:18:24 -0400
> Subject: [PATCH] cifs: stuff the fl_owner into "pid" field in the lock request
> 
> Right now, we send a "pid" across the wire. What we really want to send
> though is a hashed fl_owner_t. That should allow OFD locks to work
> seamlessly in cifs with POSIX extensions.
> 
> Signed-off-by: Jeff Layton <jeff.layton at primarydata.com>
> ---
>  fs/cifs/cifsfs.c   |  3 +++
>  fs/cifs/cifsglob.h |  1 +
>  fs/cifs/file.c     | 14 +++++++++++---
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 08fa36e5b2bc..1ef5b8aee878 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -87,6 +87,7 @@ extern mempool_t *cifs_req_poolp;
>  extern mempool_t *cifs_mid_poolp;
>  
>  struct workqueue_struct	*cifsiod_wq;
> +__u32 cifs_lock_secret;
>  
>  /*
>   * Bumps refcount for cifs super block.
> @@ -1271,6 +1272,8 @@ init_cifs(void)
>  	spin_lock_init(&cifs_file_list_lock);
>  	spin_lock_init(&GlobalMid_Lock);
>  
> +	get_random_bytes(&cifs_lock_secret, sizeof(cifs_lock_secret));
> +
>  	if (cifs_max_pending < 2) {
>  		cifs_max_pending = 2;
>  		cifs_dbg(FYI, "cifs_max_pending set to min of 2\n");
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index bba106cdc43c..8f1d8c1e72be 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -1619,6 +1619,7 @@ void cifs_oplock_break(struct work_struct *work);
>  
>  extern const struct slow_work_ops cifs_oplock_break_ops;
>  extern struct workqueue_struct *cifsiod_wq;
> +extern __u32 cifs_lock_secret;
>  
>  extern mempool_t *cifs_mid_poolp;
>  
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 9793ae0bcaa2..d4890b6dc22d 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -1112,6 +1112,12 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile)
>  	return rc;
>  }
>  
> +static __u32
> +hash_lockowner(fl_owner_t owner)
> +{
> +	return cifs_lock_secret ^ hash32_ptr((const void *)owner);
> +}
> +
>  struct lock_to_push {
>  	struct list_head llist;
>  	__u64 offset;
> @@ -1178,7 +1184,7 @@ cifs_push_posix_locks(struct cifsFileInfo *cfile)
>  		else
>  			type = CIFS_WRLCK;
>  		lck = list_entry(el, struct lock_to_push, llist);
> -		lck->pid = flock->fl_pid;
> +		lck->pid = hash_lockowner(flock->fl_owner);
>  		lck->netfid = cfile->fid.netfid;
>  		lck->length = length;
>  		lck->type = type;
> @@ -1305,7 +1311,8 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
>  			posix_lock_type = CIFS_RDLCK;
>  		else
>  			posix_lock_type = CIFS_WRLCK;
> -		rc = CIFSSMBPosixLock(xid, tcon, netfid, current->tgid,
> +		rc = CIFSSMBPosixLock(xid, tcon, netfid,
> +				      hash_lockowner(flock->fl_owner),
>  				      flock->fl_start, length, flock,
>  				      posix_lock_type, wait_flag);
>  		return rc;
> @@ -1505,7 +1512,8 @@ cifs_setlk(struct file *file, struct file_lock *flock, __u32 type,
>  			posix_lock_type = CIFS_UNLCK;
>  
>  		rc = CIFSSMBPosixLock(xid, tcon, cfile->fid.netfid,
> -				      current->tgid, flock->fl_start, length,
> +				      hash_lockowner(flock->fl_owner),
> +				      flock->fl_start, length,
>  				      NULL, posix_lock_type, wait_flag);
>  		goto out;
>  	}
> -- 
> 2.5.5
> 




More information about the samba-technical mailing list