Skip to content

Commit

Permalink
Merge tag 'locks-v3.17-2' of git://git.samba.org/jlayton/linux
Browse files Browse the repository at this point in the history 
Pull file locking bugfixes from Jeff Layton:
 "Most of these patches are to fix a long-standing regression that crept
  in when the BKL was removed from the file-locking code.  The code was
  converted to use a conventional spinlock, but some fl_release_private
  ops can block and you can end up sleeping inside the lock.

  There's also a patch to make /proc/locks show delegations as 'DELEG'"

* tag 'locks-v3.17-2' of git://git.samba.org/jlayton/linux:
  locks: update Locking documentation to clarify fl_release_private behavior
  locks: move locks_free_lock calls in do_fcntl_add_lease outside spinlock
  locks: defer freeing locks in locks_delete_lock until after i_lock has been dropped
  locks: don't reuse file_lock in __posix_lock_file
  locks: don't call locks_release_private from locks_copy_lock
  locks: show delegations as "DELEG" in /proc/locks
  • Loading branch information
@torvalds
torvalds committed Aug 16, 2014
2 parents da06df5 + 2ece173 commit 53b95d6
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 30 deletions.
6 changes: 5 additions & 1 deletion Documentation/filesystems/Locking
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,11 @@ prototypes:
locking rules:
inode->i_lock may block
fl_copy_lock: yes no
fl_release_private: maybe no
fl_release_private: maybe maybe[1]

[1]: ->fl_release_private for flock or POSIX locks is currently allowed
to block. Leases however can still be freed while the i_lock is held and
so fl_release_private called on a lease should not block.

----------------------- lock_manager_operations ---------------------------
prototypes:
Expand Down
86 changes: 57 additions & 29 deletions fs/locks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,18 @@ void locks_free_lock(struct file_lock *fl)
}
EXPORT_SYMBOL(locks_free_lock);

static void
locks_dispose_list(struct list_head *dispose)
{
struct file_lock *fl;

while (!list_empty(dispose)) {
fl = list_first_entry(dispose, struct file_lock, fl_block);
list_del_init(&fl->fl_block);
locks_free_lock(fl);
}
}

void locks_init_lock(struct file_lock *fl)
{
memset(fl, 0, sizeof(struct file_lock));
Expand Down Expand Up @@ -285,7 +297,8 @@ EXPORT_SYMBOL(__locks_copy_lock);

void locks_copy_lock(struct file_lock *new, struct file_lock *fl)
{
locks_release_private(new);
/* "new" must be a freshly-initialized lock */
WARN_ON_ONCE(new->fl_ops);

__locks_copy_lock(new, fl);
new->fl_file = fl->fl_file;
Expand Down Expand Up @@ -650,12 +663,16 @@ static void locks_unlink_lock(struct file_lock **thisfl_p)
*
* Must be called with i_lock held!
*/
static void locks_delete_lock(struct file_lock **thisfl_p)
static void locks_delete_lock(struct file_lock **thisfl_p,
struct list_head *dispose)
{
struct file_lock *fl = *thisfl_p;

locks_unlink_lock(thisfl_p);
locks_free_lock(fl);
if (dispose)
list_add(&fl->fl_block, dispose);
else
locks_free_lock(fl);
}

/* Determine if lock sys_fl blocks lock caller_fl. Common functionality
Expand Down Expand Up @@ -811,6 +828,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
struct inode * inode = file_inode(filp);
int error = 0;
int found = 0;
LIST_HEAD(dispose);

if (!(request->fl_flags & FL_ACCESS) && (request->fl_type != F_UNLCK)) {
new_fl = locks_alloc_lock();
Expand All @@ -833,7 +851,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
if (request->fl_type == fl->fl_type)
goto out;
found = 1;
locks_delete_lock(before);
locks_delete_lock(before, &dispose);
break;
}

Expand Down Expand Up @@ -880,6 +898,7 @@ static int flock_lock_file(struct file *filp, struct file_lock *request)
spin_unlock(&inode->i_lock);
if (new_fl)
locks_free_lock(new_fl);
locks_dispose_list(&dispose);
return error;
}

Expand All @@ -893,6 +912,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
struct file_lock **before;
int error;
bool added = false;
LIST_HEAD(dispose);

/*
* We may need two file_lock structures for this operation,
Expand Down Expand Up @@ -988,7 +1008,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
else
request->fl_end = fl->fl_end;
if (added) {
locks_delete_lock(before);
locks_delete_lock(before, &dispose);
continue;
}
request = fl;
Expand Down Expand Up @@ -1018,21 +1038,24 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
* one (This may happen several times).
*/
if (added) {
locks_delete_lock(before);
locks_delete_lock(before, &dispose);
continue;
}
/* Replace the old lock with the new one.
* Wake up anybody waiting for the old one,
* as the change in lock type might satisfy
* their needs.
/*
* Replace the old lock with new_fl, and
* remove the old one. It's safe to do the
* insert here since we know that we won't be
* using new_fl later, and that the lock is
* just replacing an existing lock.
*/
locks_wake_up_blocks(fl);
fl->fl_start = request->fl_start;
fl->fl_end = request->fl_end;
fl->fl_type = request->fl_type;
locks_release_private(fl);
locks_copy_private(fl, request);
request = fl;
error = -ENOLCK;
if (!new_fl)
goto out;
locks_copy_lock(new_fl, request);
request = new_fl;
new_fl = NULL;
locks_delete_lock(before, &dispose);
locks_insert_lock(before, request);
added = true;
}
}
Expand Down Expand Up @@ -1093,6 +1116,7 @@ static int __posix_lock_file(struct inode *inode, struct file_lock *request, str
locks_free_lock(new_fl);
if (new_fl2)
locks_free_lock(new_fl2);
locks_dispose_list(&dispose);
return error;
}

Expand Down Expand Up @@ -1268,7 +1292,7 @@ int lease_modify(struct file_lock **before, int arg)
printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync);
fl->fl_fasync = NULL;
}
locks_delete_lock(before);
locks_delete_lock(before, NULL);
}
return 0;
}
Expand Down Expand Up @@ -1737,13 +1761,10 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
ret = fl;
spin_lock(&inode->i_lock);
error = __vfs_setlease(filp, arg, &ret);
if (error) {
spin_unlock(&inode->i_lock);
locks_free_lock(fl);
goto out_free_fasync;
}
if (ret != fl)
locks_free_lock(fl);
if (error)
goto out_unlock;
if (ret == fl)
fl = NULL;

/*
* fasync_insert_entry() returns the old entry if any.
Expand All @@ -1755,9 +1776,10 @@ static int do_fcntl_add_lease(unsigned int fd, struct file *filp, long arg)
new = NULL;

error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
out_unlock:
spin_unlock(&inode->i_lock);

out_free_fasync:
if (fl)
locks_free_lock(fl);
if (new)
fasync_free(new);
return error;
Expand Down Expand Up @@ -2320,6 +2342,7 @@ void locks_remove_file(struct file *filp)
struct inode * inode = file_inode(filp);
struct file_lock *fl;
struct file_lock **before;
LIST_HEAD(dispose);

if (!inode->i_flock)
return;
Expand Down Expand Up @@ -2365,12 +2388,13 @@ void locks_remove_file(struct file *filp)
fl->fl_type, fl->fl_flags,
fl->fl_start, fl->fl_end);

locks_delete_lock(before);
locks_delete_lock(before, &dispose);
continue;
}
before = &fl->fl_next;
}
spin_unlock(&inode->i_lock);
locks_dispose_list(&dispose);
}

/**
Expand Down Expand Up @@ -2452,7 +2476,11 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl,
seq_puts(f, "FLOCK ADVISORY ");
}
} else if (IS_LEASE(fl)) {
seq_puts(f, "LEASE ");
if (fl->fl_flags & FL_DELEG)
seq_puts(f, "DELEG ");
else
seq_puts(f, "LEASE ");

if (lease_breaking(fl))
seq_puts(f, "BREAKING ");
else if (fl->fl_file)
Expand Down

0 comments on commit 53b95d6

Please sign in to comment.