diff options
author | Jeff Layton <jlayton@primarydata.com> | 2014-09-01 07:12:07 -0400 |
---|---|---|
committer | Jeff Layton <jlayton@primarydata.com> | 2014-10-07 14:06:13 -0400 |
commit | c45198eda2794bb72601c9f96266d8b95db66dd5 (patch) | |
tree | 59c973d0d5356e113efd22ece677267cbc07e0af | |
parent | f82b4b6780afabce9d9a91c84fae17ec3d63b9d7 (diff) |
locks: move freeing of leases outside of i_lock
There was only one place where we still could free a file_lock while
holding the i_lock -- lease_modify. Add a new list_head argument to the
lm_change operation, pass in a private list when calling it, and fix
those callers to dispose of the list once the lock has been dropped.
Signed-off-by: Jeff Layton <jlayton@primarydata.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
-rw-r--r-- | Documentation/filesystems/Locking | 3 | ||||
-rw-r--r-- | fs/locks.c | 34 | ||||
-rw-r--r-- | fs/nfsd/nfs4state.c | 6 | ||||
-rw-r--r-- | include/linux/fs.h | 7 |
4 files changed, 30 insertions, 20 deletions
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking index 4af288e38f13..94d93b1f8b53 100644 --- a/Documentation/filesystems/Locking +++ b/Documentation/filesystems/Locking | |||
@@ -469,8 +469,7 @@ prototypes: | |||
469 | }; | 469 | }; |
470 | 470 | ||
471 | locking rules: | 471 | locking rules: |
472 | All may block except for ->setlease. | 472 | All may block. |
473 | No VFS locks held on entry except for ->setlease. | ||
474 | 473 | ||
475 | ->llseek() locking has moved from llseek to the individual llseek | 474 | ->llseek() locking has moved from llseek to the individual llseek |
476 | implementations. If your fs is not using generic_file_llseek, you | 475 | implementations. If your fs is not using generic_file_llseek, you |
diff --git a/fs/locks.c b/fs/locks.c index eb463257f867..c0f789dfa655 100644 --- a/fs/locks.c +++ b/fs/locks.c | |||
@@ -1292,7 +1292,7 @@ static void lease_clear_pending(struct file_lock *fl, int arg) | |||
1292 | } | 1292 | } |
1293 | 1293 | ||
1294 | /* We already had a lease on this file; just change its type */ | 1294 | /* We already had a lease on this file; just change its type */ |
1295 | int lease_modify(struct file_lock **before, int arg) | 1295 | int lease_modify(struct file_lock **before, int arg, struct list_head *dispose) |
1296 | { | 1296 | { |
1297 | struct file_lock *fl = *before; | 1297 | struct file_lock *fl = *before; |
1298 | int error = assign_type(fl, arg); | 1298 | int error = assign_type(fl, arg); |
@@ -1311,7 +1311,7 @@ int lease_modify(struct file_lock **before, int arg) | |||
1311 | printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync); | 1311 | printk(KERN_ERR "locks_delete_lock: fasync == %p\n", fl->fl_fasync); |
1312 | fl->fl_fasync = NULL; | 1312 | fl->fl_fasync = NULL; |
1313 | } | 1313 | } |
1314 | locks_delete_lock(before, NULL); | 1314 | locks_delete_lock(before, dispose); |
1315 | } | 1315 | } |
1316 | return 0; | 1316 | return 0; |
1317 | } | 1317 | } |
@@ -1325,7 +1325,7 @@ static bool past_time(unsigned long then) | |||
1325 | return time_after(jiffies, then); | 1325 | return time_after(jiffies, then); |
1326 | } | 1326 | } |
1327 | 1327 | ||
1328 | static void time_out_leases(struct inode *inode) | 1328 | static void time_out_leases(struct inode *inode, struct list_head *dispose) |
1329 | { | 1329 | { |
1330 | struct file_lock **before; | 1330 | struct file_lock **before; |
1331 | struct file_lock *fl; | 1331 | struct file_lock *fl; |
@@ -1336,9 +1336,9 @@ static void time_out_leases(struct inode *inode) | |||
1336 | while ((fl = *before) && IS_LEASE(fl) && lease_breaking(fl)) { | 1336 | while ((fl = *before) && IS_LEASE(fl) && lease_breaking(fl)) { |
1337 | trace_time_out_leases(inode, fl); | 1337 | trace_time_out_leases(inode, fl); |
1338 | if (past_time(fl->fl_downgrade_time)) | 1338 | if (past_time(fl->fl_downgrade_time)) |
1339 | lease_modify(before, F_RDLCK); | 1339 | lease_modify(before, F_RDLCK, dispose); |
1340 | if (past_time(fl->fl_break_time)) | 1340 | if (past_time(fl->fl_break_time)) |
1341 | lease_modify(before, F_UNLCK); | 1341 | lease_modify(before, F_UNLCK, dispose); |
1342 | if (fl == *before) /* lease_modify may have freed fl */ | 1342 | if (fl == *before) /* lease_modify may have freed fl */ |
1343 | before = &fl->fl_next; | 1343 | before = &fl->fl_next; |
1344 | } | 1344 | } |
@@ -1373,6 +1373,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type) | |||
1373 | int i_have_this_lease = 0; | 1373 | int i_have_this_lease = 0; |
1374 | bool lease_conflict = false; | 1374 | bool lease_conflict = false; |
1375 | int want_write = (mode & O_ACCMODE) != O_RDONLY; | 1375 | int want_write = (mode & O_ACCMODE) != O_RDONLY; |
1376 | LIST_HEAD(dispose); | ||
1376 | 1377 | ||
1377 | new_fl = lease_alloc(NULL, want_write ? F_WRLCK : F_RDLCK); | 1378 | new_fl = lease_alloc(NULL, want_write ? F_WRLCK : F_RDLCK); |
1378 | if (IS_ERR(new_fl)) | 1379 | if (IS_ERR(new_fl)) |
@@ -1381,7 +1382,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type) | |||
1381 | 1382 | ||
1382 | spin_lock(&inode->i_lock); | 1383 | spin_lock(&inode->i_lock); |
1383 | 1384 | ||
1384 | time_out_leases(inode); | 1385 | time_out_leases(inode, &dispose); |
1385 | 1386 | ||
1386 | flock = inode->i_flock; | 1387 | flock = inode->i_flock; |
1387 | if ((flock == NULL) || !IS_LEASE(flock)) | 1388 | if ((flock == NULL) || !IS_LEASE(flock)) |
@@ -1436,6 +1437,7 @@ restart: | |||
1436 | locks_insert_block(flock, new_fl); | 1437 | locks_insert_block(flock, new_fl); |
1437 | trace_break_lease_block(inode, new_fl); | 1438 | trace_break_lease_block(inode, new_fl); |
1438 | spin_unlock(&inode->i_lock); | 1439 | spin_unlock(&inode->i_lock); |
1440 | locks_dispose_list(&dispose); | ||
1439 | error = wait_event_interruptible_timeout(new_fl->fl_wait, | 1441 | error = wait_event_interruptible_timeout(new_fl->fl_wait, |
1440 | !new_fl->fl_next, break_time); | 1442 | !new_fl->fl_next, break_time); |
1441 | spin_lock(&inode->i_lock); | 1443 | spin_lock(&inode->i_lock); |
@@ -1443,7 +1445,7 @@ restart: | |||
1443 | locks_delete_block(new_fl); | 1445 | locks_delete_block(new_fl); |
1444 | if (error >= 0) { | 1446 | if (error >= 0) { |
1445 | if (error == 0) | 1447 | if (error == 0) |
1446 | time_out_leases(inode); | 1448 | time_out_leases(inode, &dispose); |
1447 | /* | 1449 | /* |
1448 | * Wait for the next conflicting lease that has not been | 1450 | * Wait for the next conflicting lease that has not been |
1449 | * broken yet | 1451 | * broken yet |
@@ -1458,6 +1460,7 @@ restart: | |||
1458 | 1460 | ||
1459 | out: | 1461 | out: |
1460 | spin_unlock(&inode->i_lock); | 1462 | spin_unlock(&inode->i_lock); |
1463 | locks_dispose_list(&dispose); | ||
1461 | locks_free_lock(new_fl); | 1464 | locks_free_lock(new_fl); |
1462 | return error; | 1465 | return error; |
1463 | } | 1466 | } |
@@ -1522,9 +1525,10 @@ int fcntl_getlease(struct file *filp) | |||
1522 | struct file_lock *fl; | 1525 | struct file_lock *fl; |
1523 | struct inode *inode = file_inode(filp); | 1526 | struct inode *inode = file_inode(filp); |
1524 | int type = F_UNLCK; | 1527 | int type = F_UNLCK; |
1528 | LIST_HEAD(dispose); | ||
1525 | 1529 | ||
1526 | spin_lock(&inode->i_lock); | 1530 | spin_lock(&inode->i_lock); |
1527 | time_out_leases(file_inode(filp)); | 1531 | time_out_leases(file_inode(filp), &dispose); |
1528 | for (fl = file_inode(filp)->i_flock; fl && IS_LEASE(fl); | 1532 | for (fl = file_inode(filp)->i_flock; fl && IS_LEASE(fl); |
1529 | fl = fl->fl_next) { | 1533 | fl = fl->fl_next) { |
1530 | if (fl->fl_file == filp) { | 1534 | if (fl->fl_file == filp) { |
@@ -1533,6 +1537,7 @@ int fcntl_getlease(struct file *filp) | |||
1533 | } | 1537 | } |
1534 | } | 1538 | } |
1535 | spin_unlock(&inode->i_lock); | 1539 | spin_unlock(&inode->i_lock); |
1540 | locks_dispose_list(&dispose); | ||
1536 | return type; | 1541 | return type; |
1537 | } | 1542 | } |
1538 | 1543 | ||
@@ -1570,6 +1575,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr | |||
1570 | struct inode *inode = dentry->d_inode; | 1575 | struct inode *inode = dentry->d_inode; |
1571 | bool is_deleg = (*flp)->fl_flags & FL_DELEG; | 1576 | bool is_deleg = (*flp)->fl_flags & FL_DELEG; |
1572 | int error; | 1577 | int error; |
1578 | LIST_HEAD(dispose); | ||
1573 | 1579 | ||
1574 | lease = *flp; | 1580 | lease = *flp; |
1575 | trace_generic_add_lease(inode, lease); | 1581 | trace_generic_add_lease(inode, lease); |
@@ -1593,7 +1599,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr | |||
1593 | } | 1599 | } |
1594 | 1600 | ||
1595 | spin_lock(&inode->i_lock); | 1601 | spin_lock(&inode->i_lock); |
1596 | time_out_leases(inode); | 1602 | time_out_leases(inode, &dispose); |
1597 | error = check_conflicting_open(dentry, arg); | 1603 | error = check_conflicting_open(dentry, arg); |
1598 | if (error) | 1604 | if (error) |
1599 | goto out; | 1605 | goto out; |
@@ -1630,7 +1636,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr | |||
1630 | 1636 | ||
1631 | if (my_before != NULL) { | 1637 | if (my_before != NULL) { |
1632 | lease = *my_before; | 1638 | lease = *my_before; |
1633 | error = lease->fl_lmops->lm_change(my_before, arg); | 1639 | error = lease->fl_lmops->lm_change(my_before, arg, &dispose); |
1634 | if (error) | 1640 | if (error) |
1635 | goto out; | 1641 | goto out; |
1636 | goto out_setup; | 1642 | goto out_setup; |
@@ -1660,6 +1666,7 @@ out_setup: | |||
1660 | lease->fl_lmops->lm_setup(lease, priv); | 1666 | lease->fl_lmops->lm_setup(lease, priv); |
1661 | out: | 1667 | out: |
1662 | spin_unlock(&inode->i_lock); | 1668 | spin_unlock(&inode->i_lock); |
1669 | locks_dispose_list(&dispose); | ||
1663 | if (is_deleg) | 1670 | if (is_deleg) |
1664 | mutex_unlock(&inode->i_mutex); | 1671 | mutex_unlock(&inode->i_mutex); |
1665 | if (!error && !my_before) | 1672 | if (!error && !my_before) |
@@ -1676,8 +1683,10 @@ static int generic_delete_lease(struct file *filp) | |||
1676 | struct file_lock *fl, **before; | 1683 | struct file_lock *fl, **before; |
1677 | struct dentry *dentry = filp->f_path.dentry; | 1684 | struct dentry *dentry = filp->f_path.dentry; |
1678 | struct inode *inode = dentry->d_inode; | 1685 | struct inode *inode = dentry->d_inode; |
1686 | LIST_HEAD(dispose); | ||
1679 | 1687 | ||
1680 | spin_lock(&inode->i_lock); | 1688 | spin_lock(&inode->i_lock); |
1689 | time_out_leases(inode, &dispose); | ||
1681 | for (before = &inode->i_flock; | 1690 | for (before = &inode->i_flock; |
1682 | ((fl = *before) != NULL) && IS_LEASE(fl); | 1691 | ((fl = *before) != NULL) && IS_LEASE(fl); |
1683 | before = &fl->fl_next) { | 1692 | before = &fl->fl_next) { |
@@ -1686,8 +1695,9 @@ static int generic_delete_lease(struct file *filp) | |||
1686 | } | 1695 | } |
1687 | trace_generic_delete_lease(inode, fl); | 1696 | trace_generic_delete_lease(inode, fl); |
1688 | if (fl) | 1697 | if (fl) |
1689 | error = fl->fl_lmops->lm_change(before, F_UNLCK); | 1698 | error = fl->fl_lmops->lm_change(before, F_UNLCK, &dispose); |
1690 | spin_unlock(&inode->i_lock); | 1699 | spin_unlock(&inode->i_lock); |
1700 | locks_dispose_list(&dispose); | ||
1691 | return error; | 1701 | return error; |
1692 | } | 1702 | } |
1693 | 1703 | ||
@@ -2372,7 +2382,7 @@ void locks_remove_file(struct file *filp) | |||
2372 | while ((fl = *before) != NULL) { | 2382 | while ((fl = *before) != NULL) { |
2373 | if (fl->fl_file == filp) { | 2383 | if (fl->fl_file == filp) { |
2374 | if (IS_LEASE(fl)) { | 2384 | if (IS_LEASE(fl)) { |
2375 | lease_modify(before, F_UNLCK); | 2385 | lease_modify(before, F_UNLCK, &dispose); |
2376 | continue; | 2386 | continue; |
2377 | } | 2387 | } |
2378 | 2388 | ||
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index 5349528136e2..604ab6decd28 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c | |||
@@ -3427,11 +3427,11 @@ static void nfsd_break_deleg_cb(struct file_lock *fl) | |||
3427 | spin_unlock(&fp->fi_lock); | 3427 | spin_unlock(&fp->fi_lock); |
3428 | } | 3428 | } |
3429 | 3429 | ||
3430 | static | 3430 | static int |
3431 | int nfsd_change_deleg_cb(struct file_lock **onlist, int arg) | 3431 | nfsd_change_deleg_cb(struct file_lock **onlist, int arg, struct list_head *dispose) |
3432 | { | 3432 | { |
3433 | if (arg & F_UNLCK) | 3433 | if (arg & F_UNLCK) |
3434 | return lease_modify(onlist, arg); | 3434 | return lease_modify(onlist, arg, dispose); |
3435 | else | 3435 | else |
3436 | return -EAGAIN; | 3436 | return -EAGAIN; |
3437 | } | 3437 | } |
diff --git a/include/linux/fs.h b/include/linux/fs.h index 9a6d56154dd5..f419f718e447 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h | |||
@@ -873,7 +873,7 @@ struct lock_manager_operations { | |||
873 | void (*lm_notify)(struct file_lock *); /* unblock callback */ | 873 | void (*lm_notify)(struct file_lock *); /* unblock callback */ |
874 | int (*lm_grant)(struct file_lock *, int); | 874 | int (*lm_grant)(struct file_lock *, int); |
875 | void (*lm_break)(struct file_lock *); | 875 | void (*lm_break)(struct file_lock *); |
876 | int (*lm_change)(struct file_lock **, int); | 876 | int (*lm_change)(struct file_lock **, int, struct list_head *); |
877 | void (*lm_setup)(struct file_lock *, void **); | 877 | void (*lm_setup)(struct file_lock *, void **); |
878 | }; | 878 | }; |
879 | 879 | ||
@@ -985,7 +985,7 @@ extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int t | |||
985 | extern void lease_get_mtime(struct inode *, struct timespec *time); | 985 | extern void lease_get_mtime(struct inode *, struct timespec *time); |
986 | extern int generic_setlease(struct file *, long, struct file_lock **, void **priv); | 986 | extern int generic_setlease(struct file *, long, struct file_lock **, void **priv); |
987 | extern int vfs_setlease(struct file *, long, struct file_lock **, void **); | 987 | extern int vfs_setlease(struct file *, long, struct file_lock **, void **); |
988 | extern int lease_modify(struct file_lock **, int); | 988 | extern int lease_modify(struct file_lock **, int, struct list_head *); |
989 | #else /* !CONFIG_FILE_LOCKING */ | 989 | #else /* !CONFIG_FILE_LOCKING */ |
990 | static inline int fcntl_getlk(struct file *file, unsigned int cmd, | 990 | static inline int fcntl_getlk(struct file *file, unsigned int cmd, |
991 | struct flock __user *user) | 991 | struct flock __user *user) |
@@ -1112,7 +1112,8 @@ static inline int vfs_setlease(struct file *filp, long arg, | |||
1112 | return -EINVAL; | 1112 | return -EINVAL; |
1113 | } | 1113 | } |
1114 | 1114 | ||
1115 | static inline int lease_modify(struct file_lock **before, int arg) | 1115 | static inline int lease_modify(struct file_lock **before, int arg, |
1116 | struct list_head *dispose) | ||
1116 | { | 1117 | { |
1117 | return -EINVAL; | 1118 | return -EINVAL; |
1118 | } | 1119 | } |