From e6f5c78930e409f3a6b37f5484313a416359ac7f Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 22 Aug 2014 10:40:25 -0400 Subject: locks: plumb a "priv" pointer into the setlease routines In later patches, we're going to add a new lock_manager_operation to finish setting up the lease while still holding the i_lock. To do this, we'll need to pass a little bit of info in the fcntl setlease case (primarily an fasync structure). Plumb the extra pointer into there in advance of that. We declare this pointer as a void ** to make it clear that this is private info, and that the caller isn't required to set this unless the lm_setup specifically requires it. Signed-off-by: Jeff Layton Reviewed-by: Christoph Hellwig --- Documentation/filesystems/Locking | 2 +- Documentation/filesystems/vfs.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'Documentation/filesystems') diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking index f1997e9da61f..3d92049ae71d 100644 --- a/Documentation/filesystems/Locking +++ b/Documentation/filesystems/Locking @@ -464,7 +464,7 @@ prototypes: size_t, unsigned int); ssize_t (*splice_read)(struct file *, loff_t *, struct pipe_inode_info *, size_t, unsigned int); - int (*setlease)(struct file *, long, struct file_lock **); + int (*setlease)(struct file *, long, struct file_lock **, void **); long (*fallocate)(struct file *, int, loff_t, loff_t); }; diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt index 61d65cc65c54..28ebd49f169f 100644 --- a/Documentation/filesystems/vfs.txt +++ b/Documentation/filesystems/vfs.txt @@ -826,7 +826,7 @@ struct file_operations { int (*flock) (struct file *, int, struct file_lock *); ssize_t (*splice_write)(struct pipe_inode_info *, struct file *, size_t, unsigned int); ssize_t (*splice_read)(struct file *, struct pipe_inode_info *, size_t, unsigned int); - int (*setlease)(struct file *, long arg, struct file_lock **); + int (*setlease)(struct file *, long arg, struct file_lock **, void **); long (*fallocate)(struct file *, int mode, loff_t offset, loff_t len); int (*show_fdinfo)(struct seq_file *m, struct file *f); }; -- cgit v1.2.3-59-g8ed1b From f82b4b6780afabce9d9a91c84fae17ec3d63b9d7 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Fri, 22 Aug 2014 18:50:48 -0400 Subject: locks: move i_lock acquisition into generic_*_lease handlers Now that we have a saner internal API for managing leases, we no longer need to mandate that the inode->i_lock be held over most of the lease code. Push it down into generic_add_lease and generic_delete_lease. Signed-off-by: Jeff Layton Reviewed-by: Christoph Hellwig --- Documentation/filesystems/Locking | 6 ++++-- Documentation/filesystems/vfs.txt | 5 +++-- fs/locks.c | 21 +++++++++------------ 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'Documentation/filesystems') diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking index 3d92049ae71d..4af288e38f13 100644 --- a/Documentation/filesystems/Locking +++ b/Documentation/filesystems/Locking @@ -472,8 +472,6 @@ locking rules: All may block except for ->setlease. No VFS locks held on entry except for ->setlease. -->setlease has the file_list_lock held and must not sleep. - ->llseek() locking has moved from llseek to the individual llseek implementations. If your fs is not using generic_file_llseek, you need to acquire and release the appropriate locks in your ->llseek(). @@ -496,6 +494,10 @@ components. And there are other reasons why the current interface is a mess... ->read on directories probably must go away - we should just enforce -EISDIR in sys_read() and friends. +->setlease operations should call generic_setlease() before or after setting +the lease within the individual filesystem to record the result of the +operation + --------------------------- dquot_operations ------------------------------- prototypes: int (*write_dquot) (struct dquot *); diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt index 28ebd49f169f..8be1ea3bdd5a 100644 --- a/Documentation/filesystems/vfs.txt +++ b/Documentation/filesystems/vfs.txt @@ -895,8 +895,9 @@ otherwise noted. splice_read: called by the VFS to splice data from file to a pipe. This method is used by the splice(2) system call - setlease: called by the VFS to set or release a file lock lease. - setlease has the file_lock_lock held and must not sleep. + setlease: called by the VFS to set or release a file lock lease. setlease + implementations should call generic_setlease to record or remove + the lease in the inode after setting it. fallocate: called by the VFS to preallocate blocks or punch a hole. diff --git a/fs/locks.c b/fs/locks.c index a237ba632e8d..eb463257f867 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1330,6 +1330,8 @@ static void time_out_leases(struct inode *inode) struct file_lock **before; struct file_lock *fl; + lockdep_assert_held(&inode->i_lock); + before = &inode->i_flock; while ((fl = *before) && IS_LEASE(fl) && lease_breaking(fl)) { trace_time_out_leases(inode, fl); @@ -1590,6 +1592,8 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr return -EINVAL; } + spin_lock(&inode->i_lock); + time_out_leases(inode); error = check_conflicting_open(dentry, arg); if (error) goto out; @@ -1655,6 +1659,7 @@ out_setup: if (lease->fl_lmops->lm_setup) lease->fl_lmops->lm_setup(lease, priv); out: + spin_unlock(&inode->i_lock); if (is_deleg) mutex_unlock(&inode->i_mutex); if (!error && !my_before) @@ -1672,6 +1677,7 @@ static int generic_delete_lease(struct file *filp) struct dentry *dentry = filp->f_path.dentry; struct inode *inode = dentry->d_inode; + spin_lock(&inode->i_lock); for (before = &inode->i_flock; ((fl = *before) != NULL) && IS_LEASE(fl); before = &fl->fl_next) { @@ -1681,6 +1687,7 @@ static int generic_delete_lease(struct file *filp) trace_generic_delete_lease(inode, fl); if (fl) error = fl->fl_lmops->lm_change(before, F_UNLCK); + spin_unlock(&inode->i_lock); return error; } @@ -1694,8 +1701,6 @@ static int generic_delete_lease(struct file *filp) * * The (input) flp->fl_lmops->lm_break function is required * by break_lease(). - * - * Called with inode->i_lock held. */ int generic_setlease(struct file *filp, long arg, struct file_lock **flp, void **priv) @@ -1712,8 +1717,6 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp, if (error) return error; - time_out_leases(inode); - switch (arg) { case F_UNLCK: return generic_delete_lease(filp); @@ -1750,16 +1753,10 @@ EXPORT_SYMBOL(generic_setlease); int vfs_setlease(struct file *filp, long arg, struct file_lock **lease, void **priv) { - struct inode *inode = file_inode(filp); - int error; - - spin_lock(&inode->i_lock); if (filp->f_op->setlease) - error = filp->f_op->setlease(filp, arg, lease, priv); + return filp->f_op->setlease(filp, arg, lease, priv); else - error = generic_setlease(filp, arg, lease, priv); - spin_unlock(&inode->i_lock); - return error; + return generic_setlease(filp, arg, lease, priv); } EXPORT_SYMBOL_GPL(vfs_setlease); -- cgit v1.2.3-59-g8ed1b From c45198eda2794bb72601c9f96266d8b95db66dd5 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 1 Sep 2014 07:12:07 -0400 Subject: 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 Reviewed-by: Christoph Hellwig --- Documentation/filesystems/Locking | 3 +-- fs/locks.c | 34 ++++++++++++++++++++++------------ fs/nfsd/nfs4state.c | 6 +++--- include/linux/fs.h | 7 ++++--- 4 files changed, 30 insertions(+), 20 deletions(-) (limited to 'Documentation/filesystems') 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: }; locking rules: - All may block except for ->setlease. - No VFS locks held on entry except for ->setlease. + All may block. ->llseek() locking has moved from llseek to the individual llseek 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) } /* We already had a lease on this file; just change its type */ -int lease_modify(struct file_lock **before, int arg) +int lease_modify(struct file_lock **before, int arg, struct list_head *dispose) { struct file_lock *fl = *before; int error = assign_type(fl, arg); @@ -1311,7 +1311,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, NULL); + locks_delete_lock(before, dispose); } return 0; } @@ -1325,7 +1325,7 @@ static bool past_time(unsigned long then) return time_after(jiffies, then); } -static void time_out_leases(struct inode *inode) +static void time_out_leases(struct inode *inode, struct list_head *dispose) { struct file_lock **before; struct file_lock *fl; @@ -1336,9 +1336,9 @@ static void time_out_leases(struct inode *inode) while ((fl = *before) && IS_LEASE(fl) && lease_breaking(fl)) { trace_time_out_leases(inode, fl); if (past_time(fl->fl_downgrade_time)) - lease_modify(before, F_RDLCK); + lease_modify(before, F_RDLCK, dispose); if (past_time(fl->fl_break_time)) - lease_modify(before, F_UNLCK); + lease_modify(before, F_UNLCK, dispose); if (fl == *before) /* lease_modify may have freed fl */ before = &fl->fl_next; } @@ -1373,6 +1373,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type) int i_have_this_lease = 0; bool lease_conflict = false; int want_write = (mode & O_ACCMODE) != O_RDONLY; + LIST_HEAD(dispose); new_fl = lease_alloc(NULL, want_write ? F_WRLCK : F_RDLCK); if (IS_ERR(new_fl)) @@ -1381,7 +1382,7 @@ int __break_lease(struct inode *inode, unsigned int mode, unsigned int type) spin_lock(&inode->i_lock); - time_out_leases(inode); + time_out_leases(inode, &dispose); flock = inode->i_flock; if ((flock == NULL) || !IS_LEASE(flock)) @@ -1436,6 +1437,7 @@ restart: locks_insert_block(flock, new_fl); trace_break_lease_block(inode, new_fl); spin_unlock(&inode->i_lock); + locks_dispose_list(&dispose); error = wait_event_interruptible_timeout(new_fl->fl_wait, !new_fl->fl_next, break_time); spin_lock(&inode->i_lock); @@ -1443,7 +1445,7 @@ restart: locks_delete_block(new_fl); if (error >= 0) { if (error == 0) - time_out_leases(inode); + time_out_leases(inode, &dispose); /* * Wait for the next conflicting lease that has not been * broken yet @@ -1458,6 +1460,7 @@ restart: out: spin_unlock(&inode->i_lock); + locks_dispose_list(&dispose); locks_free_lock(new_fl); return error; } @@ -1522,9 +1525,10 @@ int fcntl_getlease(struct file *filp) struct file_lock *fl; struct inode *inode = file_inode(filp); int type = F_UNLCK; + LIST_HEAD(dispose); spin_lock(&inode->i_lock); - time_out_leases(file_inode(filp)); + time_out_leases(file_inode(filp), &dispose); for (fl = file_inode(filp)->i_flock; fl && IS_LEASE(fl); fl = fl->fl_next) { if (fl->fl_file == filp) { @@ -1533,6 +1537,7 @@ int fcntl_getlease(struct file *filp) } } spin_unlock(&inode->i_lock); + locks_dispose_list(&dispose); return type; } @@ -1570,6 +1575,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr struct inode *inode = dentry->d_inode; bool is_deleg = (*flp)->fl_flags & FL_DELEG; int error; + LIST_HEAD(dispose); lease = *flp; trace_generic_add_lease(inode, lease); @@ -1593,7 +1599,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr } spin_lock(&inode->i_lock); - time_out_leases(inode); + time_out_leases(inode, &dispose); error = check_conflicting_open(dentry, arg); if (error) goto out; @@ -1630,7 +1636,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr if (my_before != NULL) { lease = *my_before; - error = lease->fl_lmops->lm_change(my_before, arg); + error = lease->fl_lmops->lm_change(my_before, arg, &dispose); if (error) goto out; goto out_setup; @@ -1660,6 +1666,7 @@ out_setup: lease->fl_lmops->lm_setup(lease, priv); out: spin_unlock(&inode->i_lock); + locks_dispose_list(&dispose); if (is_deleg) mutex_unlock(&inode->i_mutex); if (!error && !my_before) @@ -1676,8 +1683,10 @@ static int generic_delete_lease(struct file *filp) struct file_lock *fl, **before; struct dentry *dentry = filp->f_path.dentry; struct inode *inode = dentry->d_inode; + LIST_HEAD(dispose); spin_lock(&inode->i_lock); + time_out_leases(inode, &dispose); for (before = &inode->i_flock; ((fl = *before) != NULL) && IS_LEASE(fl); before = &fl->fl_next) { @@ -1686,8 +1695,9 @@ static int generic_delete_lease(struct file *filp) } trace_generic_delete_lease(inode, fl); if (fl) - error = fl->fl_lmops->lm_change(before, F_UNLCK); + error = fl->fl_lmops->lm_change(before, F_UNLCK, &dispose); spin_unlock(&inode->i_lock); + locks_dispose_list(&dispose); return error; } @@ -2372,7 +2382,7 @@ void locks_remove_file(struct file *filp) while ((fl = *before) != NULL) { if (fl->fl_file == filp) { if (IS_LEASE(fl)) { - lease_modify(before, F_UNLCK); + lease_modify(before, F_UNLCK, &dispose); continue; } 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) spin_unlock(&fp->fi_lock); } -static -int nfsd_change_deleg_cb(struct file_lock **onlist, int arg) +static int +nfsd_change_deleg_cb(struct file_lock **onlist, int arg, struct list_head *dispose) { if (arg & F_UNLCK) - return lease_modify(onlist, arg); + return lease_modify(onlist, arg, dispose); else return -EAGAIN; } 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 { void (*lm_notify)(struct file_lock *); /* unblock callback */ int (*lm_grant)(struct file_lock *, int); void (*lm_break)(struct file_lock *); - int (*lm_change)(struct file_lock **, int); + int (*lm_change)(struct file_lock **, int, struct list_head *); void (*lm_setup)(struct file_lock *, void **); }; @@ -985,7 +985,7 @@ extern int __break_lease(struct inode *inode, unsigned int flags, unsigned int t extern void lease_get_mtime(struct inode *, struct timespec *time); extern int generic_setlease(struct file *, long, struct file_lock **, void **priv); extern int vfs_setlease(struct file *, long, struct file_lock **, void **); -extern int lease_modify(struct file_lock **, int); +extern int lease_modify(struct file_lock **, int, struct list_head *); #else /* !CONFIG_FILE_LOCKING */ static inline int fcntl_getlk(struct file *file, unsigned int cmd, struct flock __user *user) @@ -1112,7 +1112,8 @@ static inline int vfs_setlease(struct file *filp, long arg, return -EINVAL; } -static inline int lease_modify(struct file_lock **before, int arg) +static inline int lease_modify(struct file_lock **before, int arg, + struct list_head *dispose) { return -EINVAL; } -- cgit v1.2.3-59-g8ed1b