* [RFC PATCH 1/2] locks: introduce i_blockleases to close lease races
[not found] ` <1305746388-18241-1-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-05-18 19:19 ` J. Bruce Fields
2011-05-18 19:19 ` [RFC PATCH 2/2] locks: break lease on unlink J. Bruce Fields
1 sibling, 0 replies; 3+ messages in thread
From: J. Bruce Fields @ 2011-05-18 19:19 UTC (permalink / raw)
To: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
Cc: J. Bruce Fields
Since break_lease is called before i_writecount is incremented, there's
a window between the two where a setlease call would have no way to know
that an open is about to happen.
We fix this by adding a new inode field, i_blockleases, that is
incremented while a lease-breaking operation is in progress.
We will later reuse i_blockleases to enforce lease-breaking for rename,
unlink, etc.
Signed-off-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/inode.c | 1 +
fs/locks.c | 28 ++++++++++++++++++++++++++++
fs/namei.c | 6 +++++-
include/linux/fs.h | 3 +++
4 files changed, 37 insertions(+), 1 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 33c963d..4f253a2 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -198,6 +198,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
inode->i_uid = 0;
inode->i_gid = 0;
atomic_set(&inode->i_writecount, 0);
+ atomic_set(&inode->i_blockleases, 0);
inode->i_size = 0;
inode->i_blocks = 0;
inode->i_bytes = 0;
diff --git a/fs/locks.c b/fs/locks.c
index 0a4f50d..7699b1b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1164,6 +1164,32 @@ static void time_out_leases(struct inode *inode)
}
}
+/* Disallow all leases (read or write): */
+void disallow_leases(struct inode *inode, int flags)
+{
+ if (!inode)
+ return;
+ if (!S_ISREG(inode->i_mode))
+ return;
+ if ((flags & O_ACCMODE) == O_RDONLY)
+ return;
+ atomic_inc(&inode->i_blockleases);
+}
+EXPORT_SYMBOL_GPL(disallow_leases);
+
+void reallow_leases(struct inode *inode, int flags)
+{
+ if (!inode)
+ return;
+ if (!S_ISREG(inode->i_mode))
+ return;
+ if ((flags & O_ACCMODE) == O_RDONLY)
+ return;
+ BUG_ON(atomic_read(&inode->i_blockleases) <= 0);
+ atomic_dec(&inode->i_blockleases);
+}
+EXPORT_SYMBOL_GPL(reallow_leases);
+
/**
* __break_lease - revoke all outstanding leases on file
* @inode: the inode of the file to return
@@ -1369,6 +1395,8 @@ int generic_setlease(struct file *filp, long arg, struct file_lock **flp)
if (arg != F_UNLCK) {
error = -EAGAIN;
+ if (atomic_read(&inode->i_blockleases))
+ goto out;
if ((arg == F_RDLCK) && (atomic_read(&inode->i_writecount) > 0))
goto out;
if ((arg == F_WRLCK)
diff --git a/fs/namei.c b/fs/namei.c
index 3cb616d..079e68c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2277,10 +2277,14 @@ ok:
want_write = 1;
}
common:
+ disallow_leases(nd->path.dentry->d_inode, open_flag);
error = may_open(&nd->path, acc_mode, open_flag);
- if (error)
+ if (error) {
+ reallow_leases(nd->path.dentry->d_inode, open_flag);
goto exit;
+ }
filp = nameidata_to_filp(nd);
+ reallow_leases(nd->path.dentry->d_inode, open_flag);
if (!IS_ERR(filp)) {
error = ima_file_check(filp, op->acc_mode);
if (error) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1b95af3..daf8443 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -796,6 +796,7 @@ struct inode {
#ifdef CONFIG_IMA
atomic_t i_readcount; /* struct files open RO */
#endif
+ atomic_t i_blockleases; /* setlease fails when >0 */
atomic_t i_writecount;
#ifdef CONFIG_SECURITY
void *i_security;
@@ -1161,6 +1162,8 @@ extern void lease_get_mtime(struct inode *, struct timespec *time);
extern int generic_setlease(struct file *, long, struct file_lock **);
extern int vfs_setlease(struct file *, long, struct file_lock **);
extern int lease_modify(struct file_lock **, int);
+extern void disallow_leases(struct inode *, int flags);
+extern void reallow_leases(struct inode *, int flags);
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 lock_flocks(void);
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 3+ messages in thread* [RFC PATCH 2/2] locks: break lease on unlink
[not found] ` <1305746388-18241-1-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-05-18 19:19 ` [RFC PATCH 1/2] locks: introduce i_blockleases to close lease races J. Bruce Fields
@ 2011-05-18 19:19 ` J. Bruce Fields
1 sibling, 0 replies; 3+ messages in thread
From: J. Bruce Fields @ 2011-05-18 19:19 UTC (permalink / raw)
To: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
Cc: J. Bruce Fields
NFSv4 uses leases (delegations) to allow clients to do opens locally.
An open takes a component name, so to do a local open a client needs to
know that at least that last component name points at the same time as
long as they hold the delegation. For that reason, the NFSv4 spec
requires that delegations be broken on unlink.
Waiting for a lease to be broken may mean waiting for an nfs client or a
user process to give it up, so doing that while holding the i_mutex
would risk deadlocks. So instead we do a non-blocking lease break, then
drop the i_mutex and do a blocking lease break if necessary.
Signed-off-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/namei.c | 32 ++++++++++++++++++++++++++++----
fs/nfsd/vfs.c | 6 +++++-
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 079e68c..4661469 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2799,6 +2799,7 @@ static long do_unlinkat(int dfd, const char __user *pathname)
nd.flags &= ~LOOKUP_PARENT;
+retry:
mutex_lock_nested(&nd.path.dentry->d_inode->i_mutex, I_MUTEX_PARENT);
dentry = lookup_hash(&nd);
error = PTR_ERR(dentry);
@@ -2806,15 +2807,27 @@ static long do_unlinkat(int dfd, const char __user *pathname)
/* Why not before? Because we want correct error value */
if (nd.last.name[nd.last.len])
goto slashes;
- inode = dentry->d_inode;
- if (inode)
- ihold(inode);
+ if (inode && inode != dentry->d_inode) {
+ reallow_leases(inode, O_WRONLY);
+ iput(inode);
+ inode = NULL;
+ }
+ if (!inode) {
+ inode = dentry->d_inode;
+ if (inode)
+ ihold(inode);
+ disallow_leases(inode, O_WRONLY);
+ }
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit2;
error = security_path_unlink(&nd.path, dentry);
if (error)
goto exit3;
+ if (inode)
+ error = break_lease(inode, O_WRONLY|O_NONBLOCK);
+ if (error)
+ goto exit3;
error = vfs_unlink(nd.path.dentry->d_inode, dentry);
exit3:
mnt_drop_write(nd.path.mnt);
@@ -2822,8 +2835,19 @@ exit3:
dput(dentry);
}
mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
- if (inode)
+ if (inode) {
+ /* XXX: safe to use EWOULDBLOCK==EAGAIN to do this?: */
+ if (error == -EWOULDBLOCK) {
+ /*
+ * Wait this time, then retry with leases left
+ * disabled:
+ */
+ break_lease(inode, O_WRONLY);
+ goto retry;
+ }
+ reallow_leases(inode, O_WRONLY);
iput(inode); /* truncate the inode here */
+ }
exit1:
path_put(&nd.path);
putname(name);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f4e056a..ef01b98 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1826,13 +1826,17 @@ nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
if (host_err)
goto out_put;
+ disallow_leases(rdentry->d_inode, O_WRONLY);
host_err = nfsd_break_lease(rdentry->d_inode);
- if (host_err)
+ if (host_err) {
+ reallow_leases(rdentry->d_inode, O_WRONLY);
goto out_drop_write;
+ }
if (type != S_IFDIR)
host_err = vfs_unlink(dirp, rdentry);
else
host_err = vfs_rmdir(dirp, rdentry);
+ reallow_leases(rdentry->d_inode, O_WRONLY);
if (!host_err)
host_err = commit_metadata(fhp);
out_drop_write:
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 3+ messages in thread