[PATCH v2 0/6] Prepare byte-range locking code for future SMB2 usage

[PATCH v2 0/6] Prepare byte-range locking code for future SMB2 usage

[Pavel Shilovsky]

This is one of the preparation step that makes the byte-range locking code more
protocol independent.

Pavel Shilovsky (6):
  CIFS: Move locks to file structure
  CIFS: Fix VFS locks usage
  CIFS: Convert lock type to 32 bit variable
  CIFS: Separate protocol specific lock type handling
  CIFS: Separate protocol specific part from getlk
  CIFS: Separate protocol specific part from setlk

 fs/cifs/cifsfs.c   |    1 -
 fs/cifs/cifsglob.h |    5 +-
 fs/cifs/file.c     |  250 +++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 182 insertions(+), 74 deletions(-)

[PATCH v2 1/6] CIFS: Move locks to file structure

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsfs.c   |    1 -
 fs/cifs/cifsglob.h |    3 +-
 fs/cifs/file.c     |   98 ++++++++++++++++++++++++++++++++--------------------
 3 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index f266161..2df55bc 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -944,7 +944,6 @@ cifs_init_once(void *inode)
 	struct cifsInodeInfo *cifsi = inode;
 
 	inode_init_once(&cifsi->vfs_inode);
-	INIT_LIST_HEAD(&cifsi->llist);
 	mutex_init(&cifsi->lock_mutex);
 }
 
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index d5ccd46..4e095ae 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -548,7 +548,6 @@ struct cifsLockInfo {
 	__u64 length;
 	__u32 pid;
 	__u8 type;
-	__u16 netfid;
 };
 
 /*
@@ -573,6 +572,7 @@ struct cifs_search_info {
 struct cifsFileInfo {
 	struct list_head tlist;	/* pointer to next fid owned by tcon */
 	struct list_head flist;	/* next fid (file instance) for this inode */
+	struct list_head llist;	/* brlocks held by this fid */
 	unsigned int uid;	/* allows finding which FileInfo structure */
 	__u32 pid;		/* process id who opened file */
 	__u16 netfid;		/* file id from remote */
@@ -613,7 +613,6 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
  */
 
 struct cifsInodeInfo {
-	struct list_head llist;		/* brlocks for this inode */
 	bool can_cache_brlcks;
 	struct mutex lock_mutex;	/* protect two fields above */
 	/* BB add in lists for dirty pages i.e. write caching info for oplock */
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 4aa6080..17dae68 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -264,6 +264,7 @@ cifs_new_fileinfo(__u16 fileHandle, struct file *file,
 	pCifsFile->tlink = cifs_get_tlink(tlink);
 	mutex_init(&pCifsFile->fh_mutex);
 	INIT_WORK(&pCifsFile->oplock_break, cifs_oplock_break);
+	INIT_LIST_HEAD(&pCifsFile->llist);
 
 	spin_lock(&cifs_file_list_lock);
 	list_add(&pCifsFile->tlist, &(tlink_tcon(tlink)->openFileList));
@@ -334,9 +335,7 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
 	 * is closed anyway.
 	 */
 	mutex_lock(&cifsi->lock_mutex);
-	list_for_each_entry_safe(li, tmp, &cifsi->llist, llist) {
-		if (li->netfid != cifs_file->netfid)
-			continue;
+	list_for_each_entry_safe(li, tmp, &cifs_file->llist, llist) {
 		list_del(&li->llist);
 		cifs_del_lock_waiters(li);
 		kfree(li);
@@ -645,7 +644,7 @@ int cifs_closedir(struct inode *inode, struct file *file)
 }
 
 static struct cifsLockInfo *
-cifs_lock_init(__u64 offset, __u64 length, __u8 type, __u16 netfid)
+cifs_lock_init(__u64 offset, __u64 length, __u8 type)
 {
 	struct cifsLockInfo *lock =
 		kmalloc(sizeof(struct cifsLockInfo), GFP_KERNEL);
@@ -654,7 +653,6 @@ cifs_lock_init(__u64 offset, __u64 length, __u8 type, __u16 netfid)
 	lock->offset = offset;
 	lock->length = length;
 	lock->type = type;
-	lock->netfid = netfid;
 	lock->pid = current->tgid;
 	INIT_LIST_HEAD(&lock->blist);
 	init_waitqueue_head(&lock->block_q);
@@ -672,19 +670,19 @@ cifs_del_lock_waiters(struct cifsLockInfo *lock)
 }
 
 static bool
-__cifs_find_lock_conflict(struct cifsInodeInfo *cinode, __u64 offset,
-			__u64 length, __u8 type, __u16 netfid,
-			struct cifsLockInfo **conf_lock)
+__cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
+			      __u64 length, __u8 type, __u16 netfid,
+			      struct cifsLockInfo **conf_lock)
 {
 	struct cifsLockInfo *li, *tmp;
 
-	list_for_each_entry_safe(li, tmp, &cinode->llist, llist) {
+	list_for_each_entry_safe(li, tmp, &cfile->llist, llist) {
 		if (offset + length <= li->offset ||
 		    offset >= li->offset + li->length)
 			continue;
 		else if ((type & LOCKING_ANDX_SHARED_LOCK) &&
-			 ((netfid == li->netfid && current->tgid == li->pid) ||
-			  type == li->type))
+			 ((netfid == cfile->netfid && current->tgid == li->pid)
+			 || type == li->type))
 			continue;
 		else {
 			*conf_lock = li;
@@ -695,11 +693,36 @@ __cifs_find_lock_conflict(struct cifsInodeInfo *cinode, __u64 offset,
 }
 
 static bool
+__cifs_find_lock_conflict(struct cifsInodeInfo *cinode, __u64 offset,
+			  __u64 length, __u8 type, __u16 netfid,
+			  struct cifsLockInfo **conf_lock)
+{
+	bool rc;
+	struct cifsFileInfo *fid, *tmp;
+
+	spin_lock(&cifs_file_list_lock);
+	list_for_each_entry_safe(fid, tmp, &cinode->openFileList, flist) {
+		cifsFileInfo_get(fid);
+		spin_unlock(&cifs_file_list_lock);
+		rc = __cifs_find_fid_lock_conflict(fid, offset, length, type,
+						   netfid, conf_lock);
+		cifsFileInfo_put(fid);
+		if (rc)
+			return rc;
+		spin_lock(&cifs_file_list_lock);
+	}
+	spin_unlock(&cifs_file_list_lock);
+
+	return false;
+}
+
+static bool
 cifs_find_lock_conflict(struct cifsInodeInfo *cinode, struct cifsLockInfo *lock,
-			struct cifsLockInfo **conf_lock)
+			__u16 netfid, struct cifsLockInfo **conf_lock)
 {
+
 	return __cifs_find_lock_conflict(cinode, lock->offset, lock->length,
-					 lock->type, lock->netfid, conf_lock);
+					 lock->type, netfid, conf_lock);
 }
 
 /*
@@ -710,17 +733,18 @@ cifs_find_lock_conflict(struct cifsInodeInfo *cinode, struct cifsLockInfo *lock,
  * the server or 1 otherwise.
  */
 static int
-cifs_lock_test(struct cifsInodeInfo *cinode, __u64 offset, __u64 length,
-	       __u8 type, __u16 netfid, struct file_lock *flock)
+cifs_lock_test(struct cifsFileInfo *cfile, __u64 offset, __u64 length,
+	       __u8 type, struct file_lock *flock)
 {
 	int rc = 0;
 	struct cifsLockInfo *conf_lock;
+	struct cifsInodeInfo *cinode = CIFS_I(cfile->dentry->d_inode);
 	bool exist;
 
 	mutex_lock(&cinode->lock_mutex);
 
-	exist = __cifs_find_lock_conflict(cinode, offset, length, type, netfid,
-					  &conf_lock);
+	exist = __cifs_find_lock_conflict(cinode, offset, length, type,
+					  cfile->netfid, &conf_lock);
 	if (exist) {
 		flock->fl_start = conf_lock->offset;
 		flock->fl_end = conf_lock->offset + conf_lock->length - 1;
@@ -739,10 +763,11 @@ cifs_lock_test(struct cifsInodeInfo *cinode, __u64 offset, __u64 length,
 }
 
 static void
-cifs_lock_add(struct cifsInodeInfo *cinode, struct cifsLockInfo *lock)
+cifs_lock_add(struct cifsFileInfo *cfile, struct cifsLockInfo *lock)
 {
+	struct cifsInodeInfo *cinode = CIFS_I(cfile->dentry->d_inode);
 	mutex_lock(&cinode->lock_mutex);
-	list_add_tail(&lock->llist, &cinode->llist);
+	list_add_tail(&lock->llist, &cfile->llist);
 	mutex_unlock(&cinode->lock_mutex);
 }
 
@@ -753,10 +778,11 @@ cifs_lock_add(struct cifsInodeInfo *cinode, struct cifsLockInfo *lock)
  * 3) -EACCESS, if there is a lock that prevents us and wait is false.
  */
 static int
-cifs_lock_add_if(struct cifsInodeInfo *cinode, struct cifsLockInfo *lock,
+cifs_lock_add_if(struct cifsFileInfo *cfile, struct cifsLockInfo *lock,
 		 bool wait)
 {
 	struct cifsLockInfo *conf_lock;
+	struct cifsInodeInfo *cinode = CIFS_I(cfile->dentry->d_inode);
 	bool exist;
 	int rc = 0;
 
@@ -764,9 +790,10 @@ try_again:
 	exist = false;
 	mutex_lock(&cinode->lock_mutex);
 
-	exist = cifs_find_lock_conflict(cinode, lock, &conf_lock);
+	exist = cifs_find_lock_conflict(cinode, lock, cfile->netfid,
+					&conf_lock);
 	if (!exist && cinode->can_cache_brlcks) {
-		list_add_tail(&lock->llist, &cinode->llist);
+		list_add_tail(&lock->llist, &cfile->llist);
 		mutex_unlock(&cinode->lock_mutex);
 		return rc;
 	}
@@ -880,7 +907,7 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile)
 	for (i = 0; i < 2; i++) {
 		cur = buf;
 		num = 0;
-		list_for_each_entry_safe(li, tmp, &cinode->llist, llist) {
+		list_for_each_entry_safe(li, tmp, &cfile->llist, llist) {
 			if (li->type != types[i])
 				continue;
 			cur->Pid = cpu_to_le16(li->pid);
@@ -1095,7 +1122,6 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u8 type,
 	__u64 length = 1 + flock->fl_end - flock->fl_start;
 	struct cifsFileInfo *cfile = (struct cifsFileInfo *)file->private_data;
 	struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
-	struct cifsInodeInfo *cinode = CIFS_I(cfile->dentry->d_inode);
 	__u16 netfid = cfile->netfid;
 
 	if (posix_lck) {
@@ -1115,8 +1141,7 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u8 type,
 		return rc;
 	}
 
-	rc = cifs_lock_test(cinode, flock->fl_start, length, type, netfid,
-			    flock);
+	rc = cifs_lock_test(cfile, flock->fl_start, length, type, flock);
 	if (!rc)
 		return rc;
 
@@ -1203,15 +1228,13 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, int xid)
 	for (i = 0; i < 2; i++) {
 		cur = buf;
 		num = 0;
-		list_for_each_entry_safe(li, tmp, &cinode->llist, llist) {
+		list_for_each_entry_safe(li, tmp, &cfile->llist, llist) {
 			if (flock->fl_start > li->offset ||
 			    (flock->fl_start + length) <
 			    (li->offset + li->length))
 				continue;
 			if (current->tgid != li->pid)
 				continue;
-			if (cfile->netfid != li->netfid)
-				continue;
 			if (types[i] != li->type)
 				continue;
 			if (!cinode->can_cache_brlcks) {
@@ -1224,7 +1247,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, int xid)
 					cpu_to_le32((u32)(li->offset>>32));
 				/*
 				 * We need to save a lock here to let us add
-				 * it again to the inode list if the unlock
+				 * it again to the file's list if the unlock
 				 * range request fails on the server.
 				 */
 				list_move(&li->llist, &tmp_llist);
@@ -1238,10 +1261,10 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, int xid)
 						 * We failed on the unlock range
 						 * request - add all locks from
 						 * the tmp list to the head of
-						 * the inode list.
+						 * the file's list.
 						 */
 						cifs_move_llist(&tmp_llist,
-								&cinode->llist);
+								&cfile->llist);
 						rc = stored_rc;
 					} else
 						/*
@@ -1256,7 +1279,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, int xid)
 			} else {
 				/*
 				 * We can cache brlock requests - simply remove
-				 * a lock from the inode list.
+				 * a lock from the file's list.
 				 */
 				list_del(&li->llist);
 				cifs_del_lock_waiters(li);
@@ -1267,7 +1290,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, int xid)
 			stored_rc = cifs_lockv(xid, tcon, cfile->netfid,
 					       types[i], num, 0, buf);
 			if (stored_rc) {
-				cifs_move_llist(&tmp_llist, &cinode->llist);
+				cifs_move_llist(&tmp_llist, &cfile->llist);
 				rc = stored_rc;
 			} else
 				cifs_free_llist(&tmp_llist);
@@ -1287,7 +1310,6 @@ cifs_setlk(struct file *file,  struct file_lock *flock, __u8 type,
 	__u64 length = 1 + flock->fl_end - flock->fl_start;
 	struct cifsFileInfo *cfile = (struct cifsFileInfo *)file->private_data;
 	struct cifs_tcon *tcon = tlink_tcon(cfile->tlink);
-	struct cifsInodeInfo *cinode = CIFS_I(file->f_path.dentry->d_inode);
 	__u16 netfid = cfile->netfid;
 
 	if (posix_lck) {
@@ -1314,11 +1336,11 @@ cifs_setlk(struct file *file,  struct file_lock *flock, __u8 type,
 	if (lock) {
 		struct cifsLockInfo *lock;
 
-		lock = cifs_lock_init(flock->fl_start, length, type, netfid);
+		lock = cifs_lock_init(flock->fl_start, length, type);
 		if (!lock)
 			return -ENOMEM;
 
-		rc = cifs_lock_add_if(cinode, lock, wait_flag);
+		rc = cifs_lock_add_if(cfile, lock, wait_flag);
 		if (rc < 0)
 			kfree(lock);
 		if (rc <= 0)
@@ -1331,7 +1353,7 @@ cifs_setlk(struct file *file,  struct file_lock *flock, __u8 type,
 			goto out;
 		}
 
-		cifs_lock_add(cinode, lock);
+		cifs_lock_add(cfile, lock);
 	} else if (unlock)
 		rc = cifs_unlock_range(cfile, flock, xid);
 
-- 
1.7.1

[PATCH v2 2/6] CIFS: Fix VFS locks usage

[Pavel Shilovsky]

Surround VFS posix_lock_file_wait calls with CIFS lock_mutex
where it needs.

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/file.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 17dae68..eeb1ba5 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -669,6 +669,21 @@ cifs_del_lock_waiters(struct cifsLockInfo *lock)
 	}
 }
 
+/*
+ * Copied from fs/locks.c with small changes.
+ * Remove waiter from blocker's block list.
+ * When blocker ends up pointing to itself then the list is empty.
+ */
+static void
+cifs_locks_delete_block(struct file_lock *waiter)
+{
+	lock_flocks();
+	list_del_init(&waiter->fl_block);
+	list_del_init(&waiter->fl_link);
+	waiter->fl_next = NULL;
+	unlock_flocks();
+}
+
 static bool
 __cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
 			      __u64 length, __u8 type, __u16 netfid,
@@ -847,6 +862,39 @@ cifs_posix_lock_test(struct file *file, struct file_lock *flock)
 	return rc;
 }
 
+/* Called with locked lock_mutex, return with unlocked. */
+static int
+cifs_posix_lock_file_wait_locked(struct file *file, struct file_lock *flock)
+{
+	struct cifsInodeInfo *cinode = CIFS_I(file->f_path.dentry->d_inode);
+	int rc;
+
+	while (true) {
+		rc = posix_lock_file(file, flock, NULL);
+		mutex_unlock(&cinode->lock_mutex);
+		if (rc != FILE_LOCK_DEFERRED)
+			break;
+		rc = wait_event_interruptible(flock->fl_wait, !flock->fl_next);
+		if (!rc) {
+			mutex_lock(&cinode->lock_mutex);
+			continue;
+		}
+		cifs_locks_delete_block(flock);
+		break;
+	}
+	return rc;
+}
+
+static int
+cifs_posix_lock_file_wait(struct file *file, struct file_lock *flock)
+{
+	struct cifsInodeInfo *cinode = CIFS_I(file->f_path.dentry->d_inode);
+
+	mutex_unlock(&cinode->lock_mutex);
+	/* lock_mutex will be released by the function below */
+	return cifs_posix_lock_file_wait_locked(file, flock);
+}
+
 /*
  * Set the byte-range lock (posix style). Returns:
  * 1) 0, if we set the lock and don't need to request to the server;
@@ -867,9 +915,9 @@ cifs_posix_lock_set(struct file *file, struct file_lock *flock)
 		mutex_unlock(&cinode->lock_mutex);
 		return rc;
 	}
-	rc = posix_lock_file_wait(file, flock);
-	mutex_unlock(&cinode->lock_mutex);
-	return rc;
+
+	/* lock_mutex will be released by the function below */
+	return cifs_posix_lock_file_wait_locked(file, flock);
 }
 
 static int
@@ -1359,7 +1407,7 @@ cifs_setlk(struct file *file,  struct file_lock *flock, __u8 type,
 
 out:
 	if (flock->fl_flags & FL_POSIX)
-		posix_lock_file_wait(file, flock);
+		cifs_posix_lock_file_wait(file, flock);
 	return rc;
 }
 
-- 
1.7.1

[PATCH v2 3/6] CIFS: Convert lock type to 32 bit variable

[Pavel Shilovsky]

to handle SMB2 lock type field further.

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/cifsglob.h |    2 +-
 fs/cifs/file.c     |   13 +++++++------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 4e095ae..16b5b19 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -547,7 +547,7 @@ struct cifsLockInfo {
 	__u64 offset;
 	__u64 length;
 	__u32 pid;
-	__u8 type;
+	__u32 type;
 };
 
 /*
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index eeb1ba5..4e0fc0e 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -965,7 +965,8 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile)
 			cur->OffsetHigh = cpu_to_le32((u32)(li->offset>>32));
 			if (++num == max_num) {
 				stored_rc = cifs_lockv(xid, tcon, cfile->netfid,
-						       li->type, 0, num, buf);
+						       (__u8)li->type, 0, num,
+						       buf);
 				if (stored_rc)
 					rc = stored_rc;
 				cur = buf;
@@ -976,7 +977,7 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile)
 
 		if (num) {
 			stored_rc = cifs_lockv(xid, tcon, cfile->netfid,
-					       types[i], 0, num, buf);
+					       (__u8)types[i], 0, num, buf);
 			if (stored_rc)
 				rc = stored_rc;
 		}
@@ -1119,7 +1120,7 @@ cifs_push_locks(struct cifsFileInfo *cfile)
 }
 
 static void
-cifs_read_flock(struct file_lock *flock, __u8 *type, int *lock, int *unlock,
+cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock,
 		bool *wait_flag)
 {
 	if (flock->fl_flags & FL_POSIX)
@@ -1163,7 +1164,7 @@ cifs_read_flock(struct file_lock *flock, __u8 *type, int *lock, int *unlock,
 }
 
 static int
-cifs_getlk(struct file *file, struct file_lock *flock, __u8 type,
+cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
 	   bool wait_flag, bool posix_lck, int xid)
 {
 	int rc = 0;
@@ -1351,7 +1352,7 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock, int xid)
 }
 
 static int
-cifs_setlk(struct file *file,  struct file_lock *flock, __u8 type,
+cifs_setlk(struct file *file,  struct file_lock *flock, __u32 type,
 	   bool wait_flag, bool posix_lck, int lock, int unlock, int xid)
 {
 	int rc = 0;
@@ -1422,7 +1423,7 @@ int cifs_lock(struct file *file, int cmd, struct file_lock *flock)
 	struct cifsInodeInfo *cinode;
 	struct cifsFileInfo *cfile;
 	__u16 netfid;
-	__u8 type;
+	__u32 type;
 
 	rc = -EACCES;
 	xid = GetXid();
-- 
1.7.1

[PATCH v2 4/6] CIFS: Separate protocol specific lock type handling

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/file.c |   48 +++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 4e0fc0e..0f69c76 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -684,6 +684,30 @@ cifs_locks_delete_block(struct file_lock *waiter)
 	unlock_flocks();
 }
 
+static inline __u32
+large_lock_type(void)
+{
+	return LOCKING_ANDX_LARGE_FILES;
+}
+
+static inline __u32
+exclusive_lock_type(void)
+{
+	return 0;
+}
+
+static inline __u32
+shared_lock_type(void)
+{
+	return LOCKING_ANDX_SHARED_LOCK;
+}
+
+static inline __u32
+unlock_lock_type(void)
+{
+	return 0;
+}
+
 static bool
 __cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
 			      __u64 length, __u8 type, __u16 netfid,
@@ -695,7 +719,7 @@ __cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
 		if (offset + length <= li->offset ||
 		    offset >= li->offset + li->length)
 			continue;
-		else if ((type & LOCKING_ANDX_SHARED_LOCK) &&
+		else if ((type & shared_lock_type()) &&
 			 ((netfid == cfile->netfid && current->tgid == li->pid)
 			 || type == li->type))
 			continue;
@@ -764,7 +788,7 @@ cifs_lock_test(struct cifsFileInfo *cfile, __u64 offset, __u64 length,
 		flock->fl_start = conf_lock->offset;
 		flock->fl_end = conf_lock->offset + conf_lock->length - 1;
 		flock->fl_pid = conf_lock->pid;
-		if (conf_lock->type & LOCKING_ANDX_SHARED_LOCK)
+		if (conf_lock->type & shared_lock_type())
 			flock->fl_type = F_RDLCK;
 		else
 			flock->fl_type = F_WRLCK;
@@ -1140,24 +1164,27 @@ cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock,
 	    (~(FL_POSIX | FL_FLOCK | FL_SLEEP | FL_ACCESS | FL_LEASE)))
 		cFYI(1, "Unknown lock flags 0x%x", flock->fl_flags);
 
-	*type = LOCKING_ANDX_LARGE_FILES;
+	*type = large_lock_type();
 	if (flock->fl_type == F_WRLCK) {
 		cFYI(1, "F_WRLCK ");
+		*type |= exclusive_lock_type();
 		*lock = 1;
 	} else if (flock->fl_type == F_UNLCK) {
 		cFYI(1, "F_UNLCK");
+		*type |= unlock_lock_type();
 		*unlock = 1;
 		/* Check if unlock includes more than one lock range */
 	} else if (flock->fl_type == F_RDLCK) {
 		cFYI(1, "F_RDLCK");
-		*type |= LOCKING_ANDX_SHARED_LOCK;
+		*type |= shared_lock_type();
 		*lock = 1;
 	} else if (flock->fl_type == F_EXLCK) {
 		cFYI(1, "F_EXLCK");
+		*type |= exclusive_lock_type();
 		*lock = 1;
 	} else if (flock->fl_type == F_SHLCK) {
 		cFYI(1, "F_SHLCK");
-		*type |= LOCKING_ANDX_SHARED_LOCK;
+		*type |= shared_lock_type();
 		*lock = 1;
 	} else
 		cFYI(1, "Unknown type of lock");
@@ -1180,7 +1207,7 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
 		if (!rc)
 			return rc;
 
-		if (type & LOCKING_ANDX_SHARED_LOCK)
+		if (type & shared_lock_type())
 			posix_lock_type = CIFS_RDLCK;
 		else
 			posix_lock_type = CIFS_WRLCK;
@@ -1208,19 +1235,18 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
 		return 0;
 	}
 
-	if (type & LOCKING_ANDX_SHARED_LOCK) {
+	if (type & shared_lock_type()) {
 		flock->fl_type = F_WRLCK;
 		return 0;
 	}
 
 	rc = CIFSSMBLock(xid, tcon, netfid, current->tgid, length,
 			 flock->fl_start, 0, 1,
-			 type | LOCKING_ANDX_SHARED_LOCK, 0, 0);
+			 type | shared_lock_type(), 0, 0);
 	if (rc == 0) {
 		rc = CIFSSMBLock(xid, tcon, netfid, current->tgid,
 				 length, flock->fl_start, 1, 0,
-				 type | LOCKING_ANDX_SHARED_LOCK,
-				 0, 0);
+				 type | shared_lock_type(), 0, 0);
 		flock->fl_type = F_RDLCK;
 		if (rc != 0)
 			cERROR(1, "Error unlocking previously locked "
@@ -1368,7 +1394,7 @@ cifs_setlk(struct file *file,  struct file_lock *flock, __u32 type,
 		if (!rc || rc < 0)
 			return rc;
 
-		if (type & LOCKING_ANDX_SHARED_LOCK)
+		if (type & shared_lock_type())
 			posix_lock_type = CIFS_RDLCK;
 		else
 			posix_lock_type = CIFS_WRLCK;
-- 
1.7.1

[PATCH v2 5/6] CIFS: Separate protocol specific part from getlk

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/file.c |   61 ++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 0f69c76..86f112e 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -708,9 +708,15 @@ unlock_lock_type(void)
 	return 0;
 }
 
+static inline bool
+compare_fids(struct cifsFileInfo *ob1, struct cifsFileInfo *ob2)
+{
+	return ob1->netfid == ob2->netfid;
+}
+
 static bool
 __cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
-			      __u64 length, __u8 type, __u16 netfid,
+			      __u64 length, __u8 type, struct cifsFileInfo *cur,
 			      struct cifsLockInfo **conf_lock)
 {
 	struct cifsLockInfo *li, *tmp;
@@ -720,7 +726,7 @@ __cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
 		    offset >= li->offset + li->length)
 			continue;
 		else if ((type & shared_lock_type()) &&
-			 ((netfid == cfile->netfid && current->tgid == li->pid)
+			 ((compare_fids(cur, cfile) && current->tgid == li->pid)
 			 || type == li->type))
 			continue;
 		else {
@@ -732,19 +738,20 @@ __cifs_find_fid_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
 }
 
 static bool
-__cifs_find_lock_conflict(struct cifsInodeInfo *cinode, __u64 offset,
-			  __u64 length, __u8 type, __u16 netfid,
+__cifs_find_lock_conflict(struct cifsFileInfo *cfile, __u64 offset,
+			  __u64 length, __u8 type,
 			  struct cifsLockInfo **conf_lock)
 {
 	bool rc;
 	struct cifsFileInfo *fid, *tmp;
+	struct cifsInodeInfo *cinode = CIFS_I(cfile->dentry->d_inode);
 
 	spin_lock(&cifs_file_list_lock);
 	list_for_each_entry_safe(fid, tmp, &cinode->openFileList, flist) {
 		cifsFileInfo_get(fid);
 		spin_unlock(&cifs_file_list_lock);
 		rc = __cifs_find_fid_lock_conflict(fid, offset, length, type,
-						   netfid, conf_lock);
+						   cfile, conf_lock);
 		cifsFileInfo_put(fid);
 		if (rc)
 			return rc;
@@ -756,12 +763,12 @@ __cifs_find_lock_conflict(struct cifsInodeInfo *cinode, __u64 offset,
 }
 
 static bool
-cifs_find_lock_conflict(struct cifsInodeInfo *cinode, struct cifsLockInfo *lock,
-			__u16 netfid, struct cifsLockInfo **conf_lock)
+cifs_find_lock_conflict(struct cifsFileInfo *cfile, struct cifsLockInfo *lock,
+			struct cifsLockInfo **conf_lock)
 {
 
-	return __cifs_find_lock_conflict(cinode, lock->offset, lock->length,
-					 lock->type, netfid, conf_lock);
+	return __cifs_find_lock_conflict(cfile, lock->offset, lock->length,
+					 lock->type, conf_lock);
 }
 
 /*
@@ -782,8 +789,8 @@ cifs_lock_test(struct cifsFileInfo *cfile, __u64 offset, __u64 length,
 
 	mutex_lock(&cinode->lock_mutex);
 
-	exist = __cifs_find_lock_conflict(cinode, offset, length, type,
-					  cfile->netfid, &conf_lock);
+	exist = __cifs_find_lock_conflict(cfile, offset, length, type,
+					  &conf_lock);
 	if (exist) {
 		flock->fl_start = conf_lock->offset;
 		flock->fl_end = conf_lock->offset + conf_lock->length - 1;
@@ -829,8 +836,7 @@ try_again:
 	exist = false;
 	mutex_lock(&cinode->lock_mutex);
 
-	exist = cifs_find_lock_conflict(cinode, lock, cfile->netfid,
-					&conf_lock);
+	exist = cifs_find_lock_conflict(cfile, lock, &conf_lock);
 	if (!exist && cinode->can_cache_brlcks) {
 		list_add_tail(&lock->llist, &cfile->llist);
 		mutex_unlock(&cinode->lock_mutex);
@@ -1191,6 +1197,15 @@ cifs_read_flock(struct file_lock *flock, __u32 *type, int *lock, int *unlock,
 }
 
 static int
+cifs_mandatory_lock(int xid, struct cifsFileInfo *cfile, __u64 offset,
+		    __u64 length, __u32 type, int lock, int unlock, bool wait)
+{
+	return CIFSSMBLock(xid, tlink_tcon(cfile->tlink), cfile->netfid,
+			   current->tgid, length, offset, unlock, lock,
+			   (__u8)type, wait, 0);
+}
+
+static int
 cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
 	   bool wait_flag, bool posix_lck, int xid)
 {
@@ -1222,12 +1237,11 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
 		return rc;
 
 	/* BB we could chain these into one lock request BB */
-	rc = CIFSSMBLock(xid, tcon, netfid, current->tgid, length,
-			 flock->fl_start, 0, 1, type, 0, 0);
+	rc = cifs_mandatory_lock(xid, cfile, flock->fl_start, length, type,
+				 1, 0, false);
 	if (rc == 0) {
-		rc = CIFSSMBLock(xid, tcon, netfid, current->tgid,
-				 length, flock->fl_start, 1, 0,
-				 type, 0, 0);
+		rc = cifs_mandatory_lock(xid, cfile, flock->fl_start, length,
+					 type, 0, 1, false);
 		flock->fl_type = F_UNLCK;
 		if (rc != 0)
 			cERROR(1, "Error unlocking previously locked "
@@ -1240,13 +1254,12 @@ cifs_getlk(struct file *file, struct file_lock *flock, __u32 type,
 		return 0;
 	}
 
-	rc = CIFSSMBLock(xid, tcon, netfid, current->tgid, length,
-			 flock->fl_start, 0, 1,
-			 type | shared_lock_type(), 0, 0);
+	rc = cifs_mandatory_lock(xid, cfile, flock->fl_start, length,
+				 type | shared_lock_type(), 1, 0, false);
 	if (rc == 0) {
-		rc = CIFSSMBLock(xid, tcon, netfid, current->tgid,
-				 length, flock->fl_start, 1, 0,
-				 type | shared_lock_type(), 0, 0);
+		rc = cifs_mandatory_lock(xid, cfile, flock->fl_start, length,
+					 type | shared_lock_type(), 0, 1,
+					 false);
 		flock->fl_type = F_RDLCK;
 		if (rc != 0)
 			cERROR(1, "Error unlocking previously locked "
-- 
1.7.1

[PATCH v2 6/6] CIFS: Separate protocol specific part from setlk

[Pavel Shilovsky]

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/cifs/file.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 86f112e..1399151 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1434,8 +1434,8 @@ cifs_setlk(struct file *file,  struct file_lock *flock, __u32 type,
 		if (rc <= 0)
 			goto out;
 
-		rc = CIFSSMBLock(xid, tcon, netfid, current->tgid, length,
-				 flock->fl_start, 0, 1, type, wait_flag, 0);
+		rc = cifs_mandatory_lock(xid, cfile, flock->fl_start, length,
+					 type, 1, 0, wait_flag);
 		if (rc) {
 			kfree(lock);
 			goto out;
-- 
1.7.1

Re: [PATCH v2 0/6] Prepare byte-range locking code for future SMB2 usage

[Jeff Layton]

On Fri, 16 Mar 2012 18:17:46 +0300
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:

> This is one of the preparation step that makes the byte-range locking code more
> protocol independent.
> 

Ok, I sat down to review this set today, but found it very difficult to
do so. Most of these patches have little to no explanation or
justification at all. Terseness is not a virtue when it come to patch
descriptions:

> Pavel Shilovsky (6):
>   CIFS: Move locks to file structure
^^^
That's all that patch says. No explanation of *why* you're moving those
to the file structure -- whatever that is. Do you mean cifsFileInfo here?

>   CIFS: Fix VFS locks usage

This one says:

Surround VFS posix_lock_file_wait calls with CIFS lock_mutex
where it needs.

...so you're saying that the existing code is broken? Or is it only a
problem for SMB2 support? Does a fix need to go to stable?

>   CIFS: Convert lock type to 32 bit variable
>   CIFS: Separate protocol specific lock type handling
>   CIFS: Separate protocol specific part from getlk
>   CIFS: Separate protocol specific part from setlk
> 
>  fs/cifs/cifsfs.c   |    1 -
>  fs/cifs/cifsglob.h |    5 +-
>  fs/cifs/file.c     |  250 +++++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 182 insertions(+), 74 deletions(-)
> 

Given that it's not clear to me what the goal of this patchset is, not
to mention the lack of any description on these patches, I can't
reasonably review this.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v2 0/6] Prepare byte-range locking code for future SMB2 usage

[Pavel Shilovsky]

26 марта 2012 г. 22:48 пользователь Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> написал:
> On Fri, 16 Mar 2012 18:17:46 +0300
> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>
>> This is one of the preparation step that makes the byte-range locking code more
>> protocol independent.
>>
>
> Ok, I sat down to review this set today, but found it very difficult to
> do so. Most of these patches have little to no explanation or
> justification at all. Terseness is not a virtue when it come to patch
> descriptions:
>
>> Pavel Shilovsky (6):
>>   CIFS: Move locks to file structure
> ^^^
> That's all that patch says. No explanation of *why* you're moving those
> to the file structure -- whatever that is. Do you mean cifsFileInfo here?
>
>>   CIFS: Fix VFS locks usage
>
> This one says:
>
> Surround VFS posix_lock_file_wait calls with CIFS lock_mutex
> where it needs.
>
> ...so you're saying that the existing code is broken? Or is it only a
> problem for SMB2 support? Does a fix need to go to stable?
>
>>   CIFS: Convert lock type to 32 bit variable
>>   CIFS: Separate protocol specific lock type handling
>>   CIFS: Separate protocol specific part from getlk
>>   CIFS: Separate protocol specific part from setlk
>>
>>  fs/cifs/cifsfs.c   |    1 -
>>  fs/cifs/cifsglob.h |    5 +-
>>  fs/cifs/file.c     |  250 +++++++++++++++++++++++++++++++++++++---------------
>>  3 files changed, 182 insertions(+), 74 deletions(-)
>>
>
> Given that it's not clear to me what the goal of this patchset is, not
> to mention the lack of any description on these patches, I can't
> reasonably review this.
>

Sorry - forgot to add proper descriptions for these two patches - will
repost asap.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v2 0/6] Prepare byte-range locking code for future SMB2 usage

[Pavel Shilovsky]

27 марта 2012 г. 15:00 пользователь Pavel Shilovsky
<piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> написал:
> 26 марта 2012 г. 22:48 пользователь Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> написал:
>> On Fri, 16 Mar 2012 18:17:46 +0300
>> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>>
>>> This is one of the preparation step that makes the byte-range locking code more
>>> protocol independent.
>>>
>>
>> Ok, I sat down to review this set today, but found it very difficult to
>> do so. Most of these patches have little to no explanation or
>> justification at all. Terseness is not a virtue when it come to patch
>> descriptions:
>>
>>> Pavel Shilovsky (6):
>>>   CIFS: Move locks to file structure
>> ^^^
>> That's all that patch says. No explanation of *why* you're moving those
>> to the file structure -- whatever that is. Do you mean cifsFileInfo here?
>>
>>>   CIFS: Fix VFS locks usage
>>
>> This one says:
>>
>> Surround VFS posix_lock_file_wait calls with CIFS lock_mutex
>> where it needs.
>>
>> ...so you're saying that the existing code is broken? Or is it only a
>> problem for SMB2 support? Does a fix need to go to stable?
>>
>>>   CIFS: Convert lock type to 32 bit variable
>>>   CIFS: Separate protocol specific lock type handling
>>>   CIFS: Separate protocol specific part from getlk
>>>   CIFS: Separate protocol specific part from setlk
>>>
>>>  fs/cifs/cifsfs.c   |    1 -
>>>  fs/cifs/cifsglob.h |    5 +-
>>>  fs/cifs/file.c     |  250 +++++++++++++++++++++++++++++++++++++---------------
>>>  3 files changed, 182 insertions(+), 74 deletions(-)
>>>
>>
>> Given that it's not clear to me what the goal of this patchset is, not
>> to mention the lack of any description on these patches, I can't
>> reasonably review this.
>>
>
> Sorry - forgot to add proper descriptions for these two patches - will
> repost asap.
>
> --
> Best regards,
> Pavel Shilovsky.

rebased cifs-patches branch
(http://git.altlinux.org/people/piastry/public/?p=cifs-2.6.git;a=shortlog;h=refs/heads/cifs-patches)
and smb2-3.4 branch
(http://git.altlinux.org/people/piastry/public/?p=cifs-2.6.git;a=shortlog;h=refs/heads/smb2-3.4)
on top of the current Steve's master for an easier review.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v2 0/6] Prepare byte-range locking code for future SMB2 usage

[Pavel Shilovsky]

27 марта 2012 г. 22:00 пользователь Pavel Shilovsky
<piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> написал:
> 27 марта 2012 г. 15:00 пользователь Pavel Shilovsky
> <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> написал:
>> 26 марта 2012 г. 22:48 пользователь Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> написал:
>>> On Fri, 16 Mar 2012 18:17:46 +0300
>>> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>>>
>>>> This is one of the preparation step that makes the byte-range locking code more
>>>> protocol independent.
>>>>
>>>
>>> Ok, I sat down to review this set today, but found it very difficult to
>>> do so. Most of these patches have little to no explanation or
>>> justification at all. Terseness is not a virtue when it come to patch
>>> descriptions:
>>>
>>>> Pavel Shilovsky (6):
>>>>   CIFS: Move locks to file structure
>>> ^^^
>>> That's all that patch says. No explanation of *why* you're moving those
>>> to the file structure -- whatever that is. Do you mean cifsFileInfo here?
>>>
>>>>   CIFS: Fix VFS locks usage
>>>
>>> This one says:
>>>
>>> Surround VFS posix_lock_file_wait calls with CIFS lock_mutex
>>> where it needs.
>>>
>>> ...so you're saying that the existing code is broken? Or is it only a
>>> problem for SMB2 support? Does a fix need to go to stable?
>>>
>>>>   CIFS: Convert lock type to 32 bit variable
>>>>   CIFS: Separate protocol specific lock type handling
>>>>   CIFS: Separate protocol specific part from getlk
>>>>   CIFS: Separate protocol specific part from setlk
>>>>
>>>>  fs/cifs/cifsfs.c   |    1 -
>>>>  fs/cifs/cifsglob.h |    5 +-
>>>>  fs/cifs/file.c     |  250 +++++++++++++++++++++++++++++++++++++---------------
>>>>  3 files changed, 182 insertions(+), 74 deletions(-)
>>>>
>>>
>>> Given that it's not clear to me what the goal of this patchset is, not
>>> to mention the lack of any description on these patches, I can't
>>> reasonably review this.
>>>
>>
>> Sorry - forgot to add proper descriptions for these two patches - will
>> repost asap.
>>
>> --
>> Best regards,
>> Pavel Shilovsky.
>
> rebased cifs-patches branch
> (http://git.altlinux.org/people/piastry/public/?p=cifs-2.6.git;a=shortlog;h=refs/heads/cifs-patches)
> and smb2-3.4 branch
> (http://git.altlinux.org/people/piastry/public/?p=cifs-2.6.git;a=shortlog;h=refs/heads/smb2-3.4)
> on top of the current Steve's master for an easier review.

updated branches according to he new version of VFS lock usage fix.

-- 
Best regards,
Pavel Shilovsky.