From ab83fa4b49a54e6199b076b7d8c1808144e80f0d Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Tue, 26 Jul 2011 20:10:51 -0400 Subject: locks: minor lease cleanup Use a helper function, to simplify upcoming changes. Reviewed-by: Jeff Layton Signed-off-by: J. Bruce Fields --- fs/locks.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'fs/locks.c') diff --git a/fs/locks.c b/fs/locks.c index 703f545097de..c528522862b1 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -133,6 +133,11 @@ #define IS_FLOCK(fl) (fl->fl_flags & FL_FLOCK) #define IS_LEASE(fl) (fl->fl_flags & FL_LEASE) +static bool lease_breaking(struct file_lock *fl) +{ + return fl->fl_type & F_INPROGRESS; +} + int leases_enable = 1; int lease_break_time = 45; @@ -1141,7 +1146,7 @@ static void time_out_leases(struct inode *inode) struct file_lock *fl; before = &inode->i_flock; - while ((fl = *before) && IS_LEASE(fl) && (fl->fl_type & F_INPROGRESS)) { + while ((fl = *before) && IS_LEASE(fl) && lease_breaking(fl)) { if ((fl->fl_break_time == 0) || time_before(jiffies, fl->fl_break_time)) { before = &fl->fl_next; @@ -1189,7 +1194,7 @@ int __break_lease(struct inode *inode, unsigned int mode) if (want_write) { /* If we want write access, we have to revoke any lease. */ future = F_UNLCK | F_INPROGRESS; - } else if (flock->fl_type & F_INPROGRESS) { + } else if (lease_breaking(flock)) { /* If the lease is already being broken, we just leave it */ future = flock->fl_type; } else if (flock->fl_type & F_WRLCK) { @@ -1246,7 +1251,7 @@ restart: /* Wait for the next lease that has not been broken yet */ for (flock = inode->i_flock; flock && IS_LEASE(flock); flock = flock->fl_next) { - if (flock->fl_type & F_INPROGRESS) + if (lease_breaking(flock)) goto restart; } error = 0; @@ -2126,7 +2131,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, } } else if (IS_LEASE(fl)) { seq_printf(f, "LEASE "); - if (fl->fl_type & F_INPROGRESS) + if (lease_breaking(fl)) seq_printf(f, "BREAKING "); else if (fl->fl_file) seq_printf(f, "ACTIVE "); @@ -2142,7 +2147,7 @@ static void lock_get_status(struct seq_file *f, struct file_lock *fl, : (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE "); } else { seq_printf(f, "%s ", - (fl->fl_type & F_INPROGRESS) + (lease_breaking(fl)) ? (fl->fl_type & F_UNLCK) ? "UNLCK" : "READ " : (fl->fl_type & F_WRLCK) ? "WRITE" : "READ "); } -- cgit v1.2.3-58-ga151 From 710b7216964d6455cf1b215c43b03a1a79008c7d Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Tue, 26 Jul 2011 16:28:29 -0400 Subject: locks: move F_INPROGRESS from fl_type to fl_flags field F_INPROGRESS isn't exposed to userspace. To me it makes more sense in fl_flags.... Reviewed-by: Jeff Layton Signed-off-by: J. Bruce Fields --- arch/alpha/include/asm/fcntl.h | 2 -- fs/locks.c | 14 ++++++++------ include/asm-generic/fcntl.h | 5 ----- include/linux/fs.h | 3 ++- 4 files changed, 10 insertions(+), 14 deletions(-) (limited to 'fs/locks.c') diff --git a/arch/alpha/include/asm/fcntl.h b/arch/alpha/include/asm/fcntl.h index 1b71ca70c9f6..6d9e805f18a7 100644 --- a/arch/alpha/include/asm/fcntl.h +++ b/arch/alpha/include/asm/fcntl.h @@ -51,8 +51,6 @@ #define F_EXLCK 16 /* or 3 */ #define F_SHLCK 32 /* or 4 */ -#define F_INPROGRESS 64 - #include #endif diff --git a/fs/locks.c b/fs/locks.c index c528522862b1..c4215418bca3 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -135,7 +135,7 @@ static bool lease_breaking(struct file_lock *fl) { - return fl->fl_type & F_INPROGRESS; + return fl->fl_flags & FL_INPROGRESS; } int leases_enable = 1; @@ -1132,6 +1132,7 @@ int lease_modify(struct file_lock **before, int arg) if (error) return error; + fl->fl_flags &= ~FL_INPROGRESS; locks_wake_up_blocks(fl); if (arg == F_UNLCK) locks_delete_lock(before); @@ -1152,7 +1153,7 @@ static void time_out_leases(struct inode *inode) before = &fl->fl_next; continue; } - lease_modify(before, fl->fl_type & ~F_INPROGRESS); + lease_modify(before, fl->fl_type); if (fl == *before) /* lease_modify may have freed fl */ before = &fl->fl_next; } @@ -1193,13 +1194,13 @@ int __break_lease(struct inode *inode, unsigned int mode) if (want_write) { /* If we want write access, we have to revoke any lease. */ - future = F_UNLCK | F_INPROGRESS; + future = F_UNLCK; } else if (lease_breaking(flock)) { /* If the lease is already being broken, we just leave it */ future = flock->fl_type; } else if (flock->fl_type & F_WRLCK) { /* Downgrade the exclusive lease to a read-only lease. */ - future = F_RDLCK | F_INPROGRESS; + future = F_RDLCK; } else { /* the existing lease was read-only, so we can read too. */ goto out; @@ -1221,6 +1222,7 @@ int __break_lease(struct inode *inode, unsigned int mode) for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) { if (fl->fl_type != future) { fl->fl_type = future; + fl->fl_flags |= FL_INPROGRESS; fl->fl_break_time = break_time; /* lease must have lmops break callback */ fl->fl_lmops->lm_break(fl); @@ -1319,7 +1321,7 @@ int fcntl_getlease(struct file *filp) for (fl = filp->f_path.dentry->d_inode->i_flock; fl && IS_LEASE(fl); fl = fl->fl_next) { if (fl->fl_file == filp) { - type = fl->fl_type & ~F_INPROGRESS; + type = fl->fl_type; break; } } @@ -1384,7 +1386,7 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) before = &fl->fl_next) { if (fl->fl_file == filp) my_before = before; - else if (fl->fl_type == (F_INPROGRESS | F_UNLCK)) + else if ((fl->fl_type == F_UNLCK) && lease_breaking(fl)) /* * Someone is in the process of opening this * file for writing so we may not take an diff --git a/include/asm-generic/fcntl.h b/include/asm-generic/fcntl.h index 84793c7025e2..9e5b0356e2bb 100644 --- a/include/asm-generic/fcntl.h +++ b/include/asm-generic/fcntl.h @@ -145,11 +145,6 @@ struct f_owner_ex { #define F_SHLCK 8 /* or 4 */ #endif -/* for leases */ -#ifndef F_INPROGRESS -#define F_INPROGRESS 16 -#endif - /* operations for bsd flock(), also used by the kernel implementation */ #define LOCK_SH 1 /* shared lock */ #define LOCK_EX 2 /* exclusive lock */ diff --git a/include/linux/fs.h b/include/linux/fs.h index 178cdb4f1d4a..327fdd4de85f 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1065,6 +1065,7 @@ static inline int file_check_writeable(struct file *filp) #define FL_LEASE 32 /* lease held on this file */ #define FL_CLOSE 64 /* unlock on close */ #define FL_SLEEP 128 /* A blocking lock */ +#define FL_INPROGRESS 256 /* Lease is being broken */ /* * Special return value from posix_lock_file() and vfs_lock_file() for @@ -1111,7 +1112,7 @@ struct file_lock { struct list_head fl_link; /* doubly linked list of all locks */ struct list_head fl_block; /* circular list of blocked processes */ fl_owner_t fl_owner; - unsigned char fl_flags; + unsigned int fl_flags; unsigned char fl_type; unsigned int fl_pid; struct pid *fl_nspid; -- cgit v1.2.3-58-ga151 From 778fc546f749c588aa2f6cd50215d2715c374252 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Tue, 26 Jul 2011 18:25:49 -0400 Subject: locks: fix tracking of inprogress lease breaks We currently use a bit in fl_flags to record whether a lease is being broken, and set fl_type to the type (RDLCK or UNLCK) that it will eventually have. This means that once the lease break starts, we forget what the lease's type *used* to be. Breaking a read lease will then result in blocking read opens, even though there's no conflict--because the lease type is now F_UNLCK and we can no longer tell whether it was previously a read or write lease. So, instead keep fl_type as the original type (the type which we enforce), and keep track of whether we're unlocking or merely downgrading by replacing the single FL_INPROGRESS flag by FL_UNLOCK_PENDING and FL_DOWNGRADE_PENDING flags. To get this right we also need to track separate downgrade and break times, to handle the case where a write-leased file gets conflicting opens first for read, then later for write. (I first considered just eliminating the downgrade behavior completely--nfsv4 doesn't need it, and nobody as far as I can tell actually uses it currently--but Jeremy Allison tells me that Windows oplocks do behave this way, so Samba will probably use this some day.) Reviewed-by: Jeff Layton Signed-off-by: J. Bruce Fields --- fs/locks.c | 87 ++++++++++++++++++++++++++++++++++-------------------- include/linux/fs.h | 7 +++-- 2 files changed, 60 insertions(+), 34 deletions(-) (limited to 'fs/locks.c') diff --git a/fs/locks.c b/fs/locks.c index c4215418bca3..c525aa4de234 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -135,7 +135,16 @@ static bool lease_breaking(struct file_lock *fl) { - return fl->fl_flags & FL_INPROGRESS; + return fl->fl_flags & (FL_UNLOCK_PENDING | FL_DOWNGRADE_PENDING); +} + +static int target_leasetype(struct file_lock *fl) +{ + if (fl->fl_flags & FL_UNLOCK_PENDING) + return F_UNLCK; + if (fl->fl_flags & FL_DOWNGRADE_PENDING) + return F_RDLCK; + return fl->fl_type; } int leases_enable = 1; @@ -1124,6 +1133,17 @@ int locks_mandatory_area(int read_write, struct inode *inode, EXPORT_SYMBOL(locks_mandatory_area); +static void lease_clear_pending(struct file_lock *fl, int arg) +{ + switch (arg) { + case F_UNLCK: + fl->fl_flags &= ~FL_UNLOCK_PENDING; + /* fall through: */ + case F_RDLCK: + fl->fl_flags &= ~FL_DOWNGRADE_PENDING; + } +} + /* We already had a lease on this file; just change its type */ int lease_modify(struct file_lock **before, int arg) { @@ -1132,7 +1152,7 @@ int lease_modify(struct file_lock **before, int arg) if (error) return error; - fl->fl_flags &= ~FL_INPROGRESS; + lease_clear_pending(fl, arg); locks_wake_up_blocks(fl); if (arg == F_UNLCK) locks_delete_lock(before); @@ -1141,6 +1161,14 @@ int lease_modify(struct file_lock **before, int arg) EXPORT_SYMBOL(lease_modify); +static bool past_time(unsigned long then) +{ + if (!then) + /* 0 is a special value meaning "this never expires": */ + return false; + return time_after(jiffies, then); +} + static void time_out_leases(struct inode *inode) { struct file_lock **before; @@ -1148,12 +1176,10 @@ static void time_out_leases(struct inode *inode) before = &inode->i_flock; while ((fl = *before) && IS_LEASE(fl) && lease_breaking(fl)) { - if ((fl->fl_break_time == 0) - || time_before(jiffies, fl->fl_break_time)) { - before = &fl->fl_next; - continue; - } - lease_modify(before, fl->fl_type); + if (past_time(fl->fl_downgrade_time)) + lease_modify(before, F_RDLCK); + if (past_time(fl->fl_break_time)) + lease_modify(before, F_UNLCK); if (fl == *before) /* lease_modify may have freed fl */ before = &fl->fl_next; } @@ -1171,7 +1197,7 @@ static void time_out_leases(struct inode *inode) */ int __break_lease(struct inode *inode, unsigned int mode) { - int error = 0, future; + int error = 0; struct file_lock *new_fl, *flock; struct file_lock *fl; unsigned long break_time; @@ -1188,24 +1214,13 @@ int __break_lease(struct inode *inode, unsigned int mode) if ((flock == NULL) || !IS_LEASE(flock)) goto out; + if (!locks_conflict(flock, new_fl)) + goto out; + for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) if (fl->fl_owner == current->files) i_have_this_lease = 1; - if (want_write) { - /* If we want write access, we have to revoke any lease. */ - future = F_UNLCK; - } else if (lease_breaking(flock)) { - /* If the lease is already being broken, we just leave it */ - future = flock->fl_type; - } else if (flock->fl_type & F_WRLCK) { - /* Downgrade the exclusive lease to a read-only lease. */ - future = F_RDLCK; - } else { - /* the existing lease was read-only, so we can read too. */ - goto out; - } - if (IS_ERR(new_fl) && !i_have_this_lease && ((mode & O_NONBLOCK) == 0)) { error = PTR_ERR(new_fl); @@ -1220,13 +1235,18 @@ int __break_lease(struct inode *inode, unsigned int mode) } for (fl = flock; fl && IS_LEASE(fl); fl = fl->fl_next) { - if (fl->fl_type != future) { - fl->fl_type = future; - fl->fl_flags |= FL_INPROGRESS; + if (want_write) { + if (fl->fl_flags & FL_UNLOCK_PENDING) + continue; + fl->fl_flags |= FL_UNLOCK_PENDING; fl->fl_break_time = break_time; - /* lease must have lmops break callback */ - fl->fl_lmops->lm_break(fl); + } else { + if (lease_breaking(flock)) + continue; + fl->fl_flags |= FL_DOWNGRADE_PENDING; + fl->fl_downgrade_time = break_time; } + fl->fl_lmops->lm_break(fl); } if (i_have_this_lease || (mode & O_NONBLOCK)) { @@ -1250,10 +1270,13 @@ restart: if (error >= 0) { if (error == 0) time_out_leases(inode); - /* Wait for the next lease that has not been broken yet */ + /* + * Wait for the next conflicting lease that has not been + * broken yet + */ for (flock = inode->i_flock; flock && IS_LEASE(flock); flock = flock->fl_next) { - if (lease_breaking(flock)) + if (locks_conflict(new_fl, flock)) goto restart; } error = 0; @@ -1321,7 +1344,7 @@ int fcntl_getlease(struct file *filp) for (fl = filp->f_path.dentry->d_inode->i_flock; fl && IS_LEASE(fl); fl = fl->fl_next) { if (fl->fl_file == filp) { - type = fl->fl_type; + type = target_leasetype(fl); break; } } @@ -1386,7 +1409,7 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) before = &fl->fl_next) { if (fl->fl_file == filp) my_before = before; - else if ((fl->fl_type == F_UNLCK) && lease_breaking(fl)) + else if (fl->fl_flags & FL_UNLOCK_PENDING) /* * Someone is in the process of opening this * file for writing so we may not take an diff --git a/include/linux/fs.h b/include/linux/fs.h index 327fdd4de85f..76460edf1648 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1065,7 +1065,8 @@ static inline int file_check_writeable(struct file *filp) #define FL_LEASE 32 /* lease held on this file */ #define FL_CLOSE 64 /* unlock on close */ #define FL_SLEEP 128 /* A blocking lock */ -#define FL_INPROGRESS 256 /* Lease is being broken */ +#define FL_DOWNGRADE_PENDING 256 /* Lease is being downgraded */ +#define FL_UNLOCK_PENDING 512 /* Lease is being broken */ /* * Special return value from posix_lock_file() and vfs_lock_file() for @@ -1122,7 +1123,9 @@ struct file_lock { loff_t fl_end; struct fasync_struct * fl_fasync; /* for lease break notifications */ - unsigned long fl_break_time; /* for nonblocking lease breaks */ + /* for lease breaks: */ + unsigned long fl_break_time; + unsigned long fl_downgrade_time; const struct file_lock_operations *fl_ops; /* Callbacks for filesystems */ const struct lock_manager_operations *fl_lmops; /* Callbacks for lockmanagers */ -- cgit v1.2.3-58-ga151 From c1f24ef4ed46f58ea5e524a2364c93b6847fb164 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Fri, 19 Aug 2011 10:59:49 -0400 Subject: locks: setlease cleanup There's an incorrect comment here. Also clean up the logic: the "rdlease" and "wrlease" locals are confusingly named, and don't really add anything since we can make a decision as soon as we hit one of these cases. Signed-off-by: J. Bruce Fields --- fs/locks.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) (limited to 'fs/locks.c') diff --git a/fs/locks.c b/fs/locks.c index c525aa4de234..9b8408eb6946 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1368,7 +1368,7 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) struct file_lock *fl, **before, **my_before = NULL, *lease; struct dentry *dentry = filp->f_path.dentry; struct inode *inode = dentry->d_inode; - int error, rdlease_count = 0, wrlease_count = 0; + int error; lease = *flp; @@ -1404,27 +1404,28 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) * then the file is not open by anyone (including us) * except for this filp. */ + error = -EAGAIN; for (before = &inode->i_flock; ((fl = *before) != NULL) && IS_LEASE(fl); before = &fl->fl_next) { - if (fl->fl_file == filp) + if (fl->fl_file == filp) { my_before = before; - else if (fl->fl_flags & FL_UNLOCK_PENDING) - /* - * Someone is in the process of opening this - * file for writing so we may not take an - * exclusive lease on it. - */ - wrlease_count++; - else - rdlease_count++; + continue; + } + /* + * No exclusive leases if someone else has a lease on + * this file: + */ + if (arg == F_WRLCK) + goto out; + /* + * Modifying our existing lease is OK, but no getting a + * new lease if someone else is opening for write: + */ + if (fl->fl_flags & FL_UNLOCK_PENDING) + goto out; } - error = -EAGAIN; - if ((arg == F_RDLCK && (wrlease_count > 0)) || - (arg == F_WRLCK && ((rdlease_count + wrlease_count) > 0))) - goto out; - if (my_before != NULL) { error = lease->fl_lmops->lm_change(my_before, arg); if (!error) -- cgit v1.2.3-58-ga151 From 8335ebd94b3f5bed7875cc35848bbe46d8381695 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Wed, 21 Sep 2011 08:34:32 -0400 Subject: leases: split up generic_setlease into lock/unlock cases Eventually we should probably do the same thing to the file operations as well. Signed-off-by: J. Bruce Fields --- fs/locks.c | 98 +++++++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 62 insertions(+), 36 deletions(-) (limited to 'fs/locks.c') diff --git a/fs/locks.c b/fs/locks.c index 9b8408eb6946..b342902c38bc 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1352,18 +1352,7 @@ int fcntl_getlease(struct file *filp) return type; } -/** - * generic_setlease - sets a lease on an open file - * @filp: file pointer - * @arg: type of lease to obtain - * @flp: input - file_lock to use, output - file_lock inserted - * - * The (input) flp->fl_lmops->lm_break function is required - * by break_lease(). - * - * Called with file_lock_lock held. - */ -int generic_setlease(struct file *filp, long arg, struct file_lock **flp) +int generic_add_lease(struct file *filp, long arg, struct file_lock **flp) { struct file_lock *fl, **before, **my_before = NULL, *lease; struct dentry *dentry = filp->f_path.dentry; @@ -1372,30 +1361,14 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) lease = *flp; - error = -EACCES; - if ((current_fsuid() != inode->i_uid) && !capable(CAP_LEASE)) - goto out; - error = -EINVAL; - if (!S_ISREG(inode->i_mode)) + error = -EAGAIN; + if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0)) goto out; - error = security_file_lock(filp, arg); - if (error) + if ((arg == F_WRLCK) + && ((dentry->d_count > 1) + || (atomic_read(&inode->i_count) > 1))) goto out; - time_out_leases(inode); - - BUG_ON(!(*flp)->fl_lmops->lm_break); - - if (arg != F_UNLCK) { - error = -EAGAIN; - if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0)) - goto out; - if ((arg == F_WRLCK) - && ((dentry->d_count > 1) - || (atomic_read(&inode->i_count) > 1))) - goto out; - } - /* * At this point, we know that if there is an exclusive * lease on this file, then we hold it on this filp @@ -1433,9 +1406,6 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) goto out; } - if (arg == F_UNLCK) - goto out; - error = -EINVAL; if (!leases_enable) goto out; @@ -1446,6 +1416,62 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp) out: return error; } + +int generic_delete_lease(struct file *filp, struct file_lock **flp) +{ + struct file_lock *fl, **before; + struct dentry *dentry = filp->f_path.dentry; + struct inode *inode = dentry->d_inode; + + for (before = &inode->i_flock; + ((fl = *before) != NULL) && IS_LEASE(fl); + before = &fl->fl_next) { + if (fl->fl_file != filp) + continue; + return (*flp)->fl_lmops->lm_change(before, F_UNLCK); + } + return -EAGAIN; +} + +/** + * generic_setlease - sets a lease on an open file + * @filp: file pointer + * @arg: type of lease to obtain + * @flp: input - file_lock to use, output - file_lock inserted + * + * The (input) flp->fl_lmops->lm_break function is required + * by break_lease(). + * + * Called with file_lock_lock held. + */ +int generic_setlease(struct file *filp, long arg, struct file_lock **flp) +{ + struct dentry *dentry = filp->f_path.dentry; + struct inode *inode = dentry->d_inode; + int error; + + if ((current_fsuid() != inode->i_uid) && !capable(CAP_LEASE)) + return -EACCES; + if (!S_ISREG(inode->i_mode)) + return -EINVAL; + error = security_file_lock(filp, arg); + if (error) + return error; + + time_out_leases(inode); + + BUG_ON(!(*flp)->fl_lmops->lm_break); + + switch (arg) { + case F_UNLCK: + return generic_delete_lease(filp, flp); + case F_RDLCK: + case F_WRLCK: + return generic_add_lease(filp, arg, flp); + default: + BUG(); + } +} EXPORT_SYMBOL(generic_setlease); static int __vfs_setlease(struct file *filp, long arg, struct file_lock **lease) -- cgit v1.2.3-58-ga151