* [PATCH v3 0/5] Prepare byte-range locking code for future SMB2 usage
@ 2012-03-27 11:38 Pavel Shilovsky
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-27 11:38 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
This is the another preparation step before SMB2 protocol support. It separates
protocol specific things from byte-range locking code.
Pavel Shilovsky (5):
CIFS: Move locks to cifsFileInfo structure
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 | 194 ++++++++++++++++++++++++++++++++++------------------
3 files changed, 130 insertions(+), 70 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/5] CIFS: Move locks to cifsFileInfo structure
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
@ 2012-03-27 11:38 ` Pavel Shilovsky
[not found] ` <1332848319-6483-2-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-27 11:38 ` [PATCH v3 2/5] CIFS: Convert lock type to 32 bit variable Pavel Shilovsky
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-27 11:38 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
CIFS brlock cache can be used by several file handles if we have a
write-caching lease on the file that is supported by SMB2 protocol.
Prepate the code to handle this situation correctly by sorting brlocks
by a fid to easily push them in portions when lease break comes.
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 eee522c..3ac61a7 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -939,7 +939,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 2bf04e9..cc54033 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);
@@ -687,19 +685,19 @@ cifs_locks_delete_block(struct file_lock *waiter)
}
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;
@@ -710,11 +708,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);
}
/*
@@ -725,17 +748,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;
@@ -754,10 +778,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);
}
@@ -768,10 +793,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;
@@ -779,9 +805,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;
}
@@ -928,7 +955,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);
@@ -1144,7 +1171,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) {
@@ -1164,8 +1190,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;
@@ -1252,15 +1277,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) {
@@ -1273,7 +1296,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);
@@ -1287,10 +1310,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
/*
@@ -1305,7 +1328,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);
@@ -1316,7 +1339,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);
@@ -1336,7 +1359,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) {
@@ -1363,11 +1385,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)
@@ -1380,7 +1402,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/5] CIFS: Convert lock type to 32 bit variable
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-27 11:38 ` [PATCH v3 1/5] CIFS: Move locks to cifsFileInfo structure Pavel Shilovsky
@ 2012-03-27 11:38 ` Pavel Shilovsky
[not found] ` <1332848319-6483-3-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-27 11:38 ` [PATCH v3 3/5] CIFS: Separate protocol specific lock type handling Pavel Shilovsky
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-27 11:38 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
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 cc54033..2e541f0 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;
}
@@ -1120,7 +1121,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)
@@ -1164,7 +1165,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;
@@ -1352,7 +1353,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;
@@ -1423,7 +1424,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/5] CIFS: Separate protocol specific lock type handling
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-27 11:38 ` [PATCH v3 1/5] CIFS: Move locks to cifsFileInfo structure Pavel Shilovsky
2012-03-27 11:38 ` [PATCH v3 2/5] CIFS: Convert lock type to 32 bit variable Pavel Shilovsky
@ 2012-03-27 11:38 ` Pavel Shilovsky
[not found] ` <1332848319-6483-4-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-27 11:38 ` [PATCH v3 4/5] CIFS: Separate protocol specific part from getlk Pavel Shilovsky
2012-03-27 11:38 ` [PATCH v3 5/5] CIFS: Separate protocol specific part from setlk Pavel Shilovsky
4 siblings, 1 reply; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-27 11:38 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
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 2e541f0..6e72b1f 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;
@@ -1141,24 +1165,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");
@@ -1181,7 +1208,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;
@@ -1209,19 +1236,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 "
@@ -1369,7 +1395,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/5] CIFS: Separate protocol specific part from getlk
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
` (2 preceding siblings ...)
2012-03-27 11:38 ` [PATCH v3 3/5] CIFS: Separate protocol specific lock type handling Pavel Shilovsky
@ 2012-03-27 11:38 ` Pavel Shilovsky
2012-03-27 11:38 ` [PATCH v3 5/5] CIFS: Separate protocol specific part from setlk Pavel Shilovsky
4 siblings, 0 replies; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-27 11:38 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
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 6e72b1f..8cd44e4 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);
@@ -1192,6 +1198,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)
{
@@ -1223,12 +1238,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 "
@@ -1241,13 +1255,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/5] CIFS: Separate protocol specific part from setlk
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
` (3 preceding siblings ...)
2012-03-27 11:38 ` [PATCH v3 4/5] CIFS: Separate protocol specific part from getlk Pavel Shilovsky
@ 2012-03-27 11:38 ` Pavel Shilovsky
4 siblings, 0 replies; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-27 11:38 UTC (permalink / raw)
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA
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 8cd44e4..9ba62f0 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1435,8 +1435,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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/5] CIFS: Move locks to cifsFileInfo structure
[not found] ` <1332848319-6483-2-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
@ 2012-03-29 19:13 ` Jeff Layton
[not found] ` <20120329151345.1c66f659-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2012-03-29 19:13 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, 27 Mar 2012 15:38:35 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> CIFS brlock cache can be used by several file handles if we have a
> write-caching lease on the file that is supported by SMB2 protocol.
> Prepate the code to handle this situation correctly by sorting brlocks
> by a fid to easily push them in portions when lease break comes.
>
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
I'm not sure I understand this. It seems like keeping them all as part
of the inode would be simpler. How does it simplify things to put them
in separate per-FID buckets?
I suppose one argument is that you could open the file twice and get 2
separate FIDs and 2 separate leases, and have different locks against
each. Then the server might recall one lease but not the other. In that
case, you'd want to push out only the locks associated with that FID?
Is that the main concern, or am I missing the point here?
> ---
> 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 eee522c..3ac61a7 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -939,7 +939,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 */
It would be nice to comment that the above list is intended to be
protected by the lock_mutex (right?)
> 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 2bf04e9..cc54033 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);
> @@ -687,19 +685,19 @@ cifs_locks_delete_block(struct file_lock *waiter)
> }
>
> 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) {
list_for_each_entry should be fine here since you're not doing
gets/puts on these entries as you go.
> 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;
> @@ -710,11 +708,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);
NAK. list_for_each_entry_safe only protects you from removing the
current entry of the list by preloading the next entry into "tmp". It's
possible (even likely) that while this code is running, another thread
could do the final cifsFileInfo_put on the entry that has already been
preloaded into "tmp" and cause an oops or worse.
> + 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)
Do we really need this wrapper? There's only one caller of
cifs_find_lock_conflict, AFAICT.
> {
> +
> return __cifs_find_lock_conflict(cinode, lock->offset, lock->length,
> - lock->type, lock->netfid, conf_lock);
> + lock->type, netfid, conf_lock);
> }
>
> /*
> @@ -725,17 +748,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;
> @@ -754,10 +778,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);
> }
>
> @@ -768,10 +793,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;
>
> @@ -779,9 +805,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;
> }
> @@ -928,7 +955,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);
> @@ -1144,7 +1171,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) {
> @@ -1164,8 +1190,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;
>
> @@ -1252,15 +1277,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) {
> @@ -1273,7 +1296,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);
> @@ -1287,10 +1310,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
> /*
> @@ -1305,7 +1328,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);
> @@ -1316,7 +1339,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);
> @@ -1336,7 +1359,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) {
> @@ -1363,11 +1385,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)
> @@ -1380,7 +1402,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);
>
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/5] CIFS: Convert lock type to 32 bit variable
[not found] ` <1332848319-6483-3-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
@ 2012-03-29 19:16 ` Jeff Layton
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2012-03-29 19:16 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, 27 Mar 2012 15:38:36 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> 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 cc54033..2e541f0 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;
> }
> @@ -1120,7 +1121,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)
> @@ -1164,7 +1165,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;
> @@ -1352,7 +1353,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;
> @@ -1423,7 +1424,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();
Assuming that this doesn't need to be respun in light of the problems in the first patch...
Acked-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/5] CIFS: Separate protocol specific lock type handling
[not found] ` <1332848319-6483-4-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
@ 2012-03-29 19:34 ` Jeff Layton
[not found] ` <20120329153410.5fbe217b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
0 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2012-03-29 19:34 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, 27 Mar 2012 15:38:37 +0400
Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
> 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 2e541f0..6e72b1f 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;
> +}
> +
I assume that at some point you'll need to add a set of SMB2 equivalent
ops and then swap them in (probably at NEGOTIATE time)?
If so, would it make sense to bundle these up into a "struct
smb_protocol_operations" and hang it off the TCP_Server_Info? Then
you'd just turn the calls below into stuff like:
server->proto_ops->large_lock_type
By doing that, it'll be easier to merge in SMB2 code without disturbing
the existing code as much. Also, if we find later that we need a new
set of ops for 2.1, 2.2, 2.3, etc, then we can simply drop in
replacement operations for them.
I think doing that will be more sound engineering than sprinkling
"is_smb2" checks all over the place...
> 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;
> @@ -1141,24 +1165,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");
> @@ -1181,7 +1208,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;
> @@ -1209,19 +1236,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 "
> @@ -1369,7 +1395,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;
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/5] CIFS: Separate protocol specific lock type handling
[not found] ` <20120329153410.5fbe217b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2012-03-30 11:51 ` Pavel Shilovsky
0 siblings, 0 replies; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-30 11:51 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
29 марта 2012 г. 23:34 пользователь Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> написал:
> On Tue, 27 Mar 2012 15:38:37 +0400
> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>
>> 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 2e541f0..6e72b1f 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;
>> +}
>> +
>
> I assume that at some point you'll need to add a set of SMB2 equivalent
> ops and then swap them in (probably at NEGOTIATE time)?
>
> If so, would it make sense to bundle these up into a "struct
> smb_protocol_operations" and hang it off the TCP_Server_Info? Then
> you'd just turn the calls below into stuff like:
>
> server->proto_ops->large_lock_type
>
> By doing that, it'll be easier to merge in SMB2 code without disturbing
> the existing code as much. Also, if we find later that we need a new
> set of ops for 2.1, 2.2, 2.3, etc, then we can simply drop in
> replacement operations for them.
>
> I think doing that will be more sound engineering than sprinkling
> "is_smb2" checks all over the place...
>
Yes, moving these functions to a separate transport ops structure that
is going to become a part of TCP_Server_Info will be the next step.
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/5] CIFS: Move locks to cifsFileInfo structure
[not found] ` <20120329151345.1c66f659-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2012-03-30 12:03 ` Pavel Shilovsky
0 siblings, 0 replies; 11+ messages in thread
From: Pavel Shilovsky @ 2012-03-30 12:03 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
29 марта 2012 г. 23:13 пользователь Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> написал:
> On Tue, 27 Mar 2012 15:38:35 +0400
> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org> wrote:
>
>> CIFS brlock cache can be used by several file handles if we have a
>> write-caching lease on the file that is supported by SMB2 protocol.
>> Prepate the code to handle this situation correctly by sorting brlocks
>> by a fid to easily push them in portions when lease break comes.
>>
>> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>
> I'm not sure I understand this. It seems like keeping them all as part
> of the inode would be simpler. How does it simplify things to put them
> in separate per-FID buckets?
>
> I suppose one argument is that you could open the file twice and get 2
> separate FIDs and 2 separate leases, and have different locks against
> each. Then the server might recall one lease but not the other. In that
> case, you'd want to push out only the locks associated with that FID?
>
> Is that the main concern, or am I missing the point here?
Two FIDS will have the same lease and when lease break comes they have
to push their locks to the server. To make this process faster we can
send a chunk of locks in one request, as we already do for cifs
mandatory locks. If we have all FID's lock in one place we don't need
to tun over the whole inode lock list to create groups when oplock
break comes.
The only difference between CIFS and SMB2 is that in SMB2 lock_element
structure in lock request doesn't contain pid element (that exists for
CIFS), assuming that all locks from the request associated with pid
from the header. So to handle this right we need further to replace
cifsFileInfo lock list (introduced by this patch) with "pid_lock" list
struct pid_lock {
struct list_head lock_list;
pid_t pid;
}
that group FID's locks by pid.
>
>> ---
>> 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 eee522c..3ac61a7 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -939,7 +939,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 */
>
> It would be nice to comment that the above list is intended to be
> protected by the lock_mutex (right?)
>
>> 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 2bf04e9..cc54033 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);
>> @@ -687,19 +685,19 @@ cifs_locks_delete_block(struct file_lock *waiter)
>> }
>>
>> 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) {
>
> list_for_each_entry should be fine here since you're not doing
> gets/puts on these entries as you go.
>
>> 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;
>> @@ -710,11 +708,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);
>
> NAK. list_for_each_entry_safe only protects you from removing the
> current entry of the list by preloading the next entry into "tmp". It's
> possible (even likely) that while this code is running, another thread
> could do the final cifsFileInfo_put on the entry that has already been
> preloaded into "tmp" and cause an oops or worse.
Yes, makes sense. Thanks!
>
>> + 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)
>
> Do we really need this wrapper? There's only one caller of
> cifs_find_lock_conflict, AFAICT.
This makes a caller code smarter, I think.
>
>> {
>> +
>> return __cifs_find_lock_conflict(cinode, lock->offset, lock->length,
>> - lock->type, lock->netfid, conf_lock);
>> + lock->type, netfid, conf_lock);
>> }
>>
>> /*
>> @@ -725,17 +748,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;
>> @@ -754,10 +778,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);
>> }
>>
>> @@ -768,10 +793,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;
>>
>> @@ -779,9 +805,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;
>> }
>> @@ -928,7 +955,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);
>> @@ -1144,7 +1171,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) {
>> @@ -1164,8 +1190,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;
>>
>> @@ -1252,15 +1277,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) {
>> @@ -1273,7 +1296,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);
>> @@ -1287,10 +1310,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
>> /*
>> @@ -1305,7 +1328,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);
>> @@ -1316,7 +1339,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);
>> @@ -1336,7 +1359,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) {
>> @@ -1363,11 +1385,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)
>> @@ -1380,7 +1402,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);
>>
>
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-03-30 12:03 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-27 11:38 [PATCH v3 0/5] Prepare byte-range locking code for future SMB2 usage Pavel Shilovsky
[not found] ` <1332848319-6483-1-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-27 11:38 ` [PATCH v3 1/5] CIFS: Move locks to cifsFileInfo structure Pavel Shilovsky
[not found] ` <1332848319-6483-2-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-29 19:13 ` Jeff Layton
[not found] ` <20120329151345.1c66f659-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2012-03-30 12:03 ` Pavel Shilovsky
2012-03-27 11:38 ` [PATCH v3 2/5] CIFS: Convert lock type to 32 bit variable Pavel Shilovsky
[not found] ` <1332848319-6483-3-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-29 19:16 ` Jeff Layton
2012-03-27 11:38 ` [PATCH v3 3/5] CIFS: Separate protocol specific lock type handling Pavel Shilovsky
[not found] ` <1332848319-6483-4-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2012-03-29 19:34 ` Jeff Layton
[not found] ` <20120329153410.5fbe217b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2012-03-30 11:51 ` Pavel Shilovsky
2012-03-27 11:38 ` [PATCH v3 4/5] CIFS: Separate protocol specific part from getlk Pavel Shilovsky
2012-03-27 11:38 ` [PATCH v3 5/5] CIFS: Separate protocol specific part from setlk Pavel Shilovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.