[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-cifs
Subject:    Re: [PATCH v2] CIFS: Fix VFS lock usage for oplocked files
From:       Pavel Shilovsky <piastry () etersoft ! ru>
Date:       2012-03-28 10:06:35
Message-ID: CAKywueSydqWZ895seRiQR-v93hwHKVYSMGS6ampdwLbMgK_WVw () mail ! gmail ! com
[Download RAW message or body]

28 อมาิม 2012šว. 13:54 ะฯฬฺุฯืมิลฬุ Pavel Shilovsky
<piastry@etersoft.ru> ฮมะษำมฬ:
> We can deadlock if we have a write oplock and two processes
> use the same file handle. In this case the first process can't
> unlock its lock if another process blocked on the lock in the
> same time.
>
> Fix this by removing lock_mutex protection from waiting on a
> blocked lock and protect only posix_lock_file call.
>
> Cc: stable@kernel.org
> Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
> ---
> šfs/cifs/file.c š š | š 16 ++++++++++++++--
> šfs/locks.c š š š š | š š3 ++-
> šinclude/linux/fs.h | š š5 +++++
> š3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 460d87b..89e4feb 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -840,8 +840,20 @@ 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);
> +
> + š š š 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) {
> + š š š š š š š š š š š locks_delete_block(flock);
> + š š š š š š š š š š š break;
> + š š š š š š š }
> + š š š š š š š mutex_lock(&cinode->lock_mutex);
> + š š š }

We need to check if we can cache brlocks after
wait_event_interruptible ends (similar to the code we have for
mandatory locking case) - will respin this.

> +
> š š š šreturn rc;
> š}
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 637694b..0d68f1f 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -510,12 +510,13 @@ static void __locks_delete_block(struct file_lock *waiter)
>
> š/*
> š*/
> -static void locks_delete_block(struct file_lock *waiter)
> +void locks_delete_block(struct file_lock *waiter)
> š{
> š š š šlock_flocks();
> š š š š__locks_delete_block(waiter);
> š š š šunlock_flocks();
> š}
> +EXPORT_SYMBOL(locks_delete_block);
>
> š/* Insert waiter into blocker's block list.
> š* We use a circular list so that processes can be easily woken up in
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index fa63f1b..d8fd8df 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1211,6 +1211,7 @@ extern int vfs_setlease(struct file *, long, struct file_lock **);
> šextern int lease_modify(struct file_lock **, int);
> šextern int lock_may_read(struct inode *, loff_t start, unsigned long count);
> šextern int lock_may_write(struct inode *, loff_t start, unsigned long count);
> +extern void locks_delete_block(struct file_lock *waiter);
> šextern void lock_flocks(void);
> šextern void unlock_flocks(void);
> š#else /* !CONFIG_FILE_LOCKING */
> @@ -1355,6 +1356,10 @@ static inline int lock_may_write(struct inode *inode, loff_t start,
> š š š šreturn 1;
> š}
>
> +static inline void locks_delete_block(struct file_lock *waiter)
> +{
> +}
> +
> šstatic inline void lock_flocks(void)
> š{
> š}
> --
> 1.7.1
>



-- 
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]