2010-04-22 20:50:24

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 0/5] Please review bugfixes for mainline

The following patches have been queued up in the 'bugfixes' branch on
git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git

Cheers
Trond

Chuck Lever (1):
NFS: rsize and wsize settings ignored on v4 mounts

Dan Carpenter (1):
nfs: testing for null instead of ERR_PTR()

Trond Myklebust (3):
SUNRPC: Fix a bug in rpcauth_prune_expired
NFSv4: Don't attempt an atomic open if the file is a mountpoint
NFS: Fix an unstable write data integrity race

fs/nfs/client.c | 2 ++
fs/nfs/dir.c | 2 +-
fs/nfs/super.c | 2 +-
fs/nfs/write.c | 36 ++++++++++++++++++++++++++++++++----
include/linux/nfs_fs.h | 1 +
net/sunrpc/auth.c | 2 +-
6 files changed, 38 insertions(+), 7 deletions(-)



2010-04-22 20:50:26

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 2/5] NFSv4: Don't attempt an atomic open if the file is a mountpoint

Fix https://bugzilla.kernel.org/show_bug.cgi?id=15789

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/dir.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index be46f26..fbb4cf7 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1050,7 +1050,7 @@ static int nfs_open_revalidate(struct dentry *dentry, struct nameidata *nd)
struct inode *dir;
int openflags, ret = 0;

- if (!is_atomic_open(nd))
+ if (!is_atomic_open(nd) || d_mountpoint(dentry))
goto no_open;
parent = dget_parent(dentry);
dir = parent->d_inode;
--
1.6.6.1


2010-04-22 20:50:24

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 1/5] SUNRPC: Fix a bug in rpcauth_prune_expired

Don't want to evict a credential if cred->cr_expire == jiffies, since that
means that it was just placed on the cred_unused list. We therefore need to
use time_in_range() rather than time_in_range_open().

Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/auth.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index f394fc1..95afe79 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -237,7 +237,7 @@ rpcauth_prune_expired(struct list_head *free, int nr_to_scan)
list_for_each_entry_safe(cred, next, &cred_unused, cr_lru) {

/* Enforce a 60 second garbage collection moratorium */
- if (time_in_range_open(cred->cr_expire, expired, jiffies) &&
+ if (time_in_range(cred->cr_expire, expired, jiffies) &&
test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags) != 0)
continue;

--
1.6.6.1


2010-04-22 20:50:27

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 5/5] NFS: Fix an unstable write data integrity race

Commit 2c61be0a9478258f77b66208a0c4b1f5f8161c3c (NFS: Ensure that the WRITE
and COMMIT RPC calls are always uninterruptible) exposed a race on file
close. In order to ensure correct close-to-open behaviour, we want to wait
for all outstanding background commit operations to complete.

This patch adds an inode flag that indicates if a commit operation is under
way, and provides a mechanism to allow ->write_inode() to wait for its
completion if this is a data integrity flush.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/write.c | 36 ++++++++++++++++++++++++++++++++----
include/linux/nfs_fs.h | 1 +
2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index de38d63..ccde2ae 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1201,6 +1201,25 @@ int nfs_writeback_done(struct rpc_task *task, struct nfs_write_data *data)


#if defined(CONFIG_NFS_V3) || defined(CONFIG_NFS_V4)
+static int nfs_commit_set_lock(struct nfs_inode *nfsi, int may_wait)
+{
+ if (!test_and_set_bit(NFS_INO_COMMIT, &nfsi->flags))
+ return 1;
+ if (may_wait && !out_of_line_wait_on_bit_lock(&nfsi->flags,
+ NFS_INO_COMMIT, nfs_wait_bit_killable,
+ TASK_KILLABLE))
+ return 1;
+ return 0;
+}
+
+static void nfs_commit_clear_lock(struct nfs_inode *nfsi)
+{
+ clear_bit(NFS_INO_COMMIT, &nfsi->flags);
+ smp_mb__after_clear_bit();
+ wake_up_bit(&nfsi->flags, NFS_INO_COMMIT);
+}
+
+
static void nfs_commitdata_release(void *data)
{
struct nfs_write_data *wdata = data;
@@ -1262,8 +1281,6 @@ static int nfs_commit_rpcsetup(struct list_head *head,
task = rpc_run_task(&task_setup_data);
if (IS_ERR(task))
return PTR_ERR(task);
- if (how & FLUSH_SYNC)
- rpc_wait_for_completion_task(task);
rpc_put_task(task);
return 0;
}
@@ -1294,6 +1311,7 @@ nfs_commit_list(struct inode *inode, struct list_head *head, int how)
BDI_RECLAIMABLE);
nfs_clear_page_tag_locked(req);
}
+ nfs_commit_clear_lock(NFS_I(inode));
return -ENOMEM;
}

@@ -1349,6 +1367,7 @@ static void nfs_commit_release(void *calldata)
next:
nfs_clear_page_tag_locked(req);
}
+ nfs_commit_clear_lock(NFS_I(data->inode));
nfs_commitdata_release(calldata);
}

@@ -1363,8 +1382,11 @@ static const struct rpc_call_ops nfs_commit_ops = {
static int nfs_commit_inode(struct inode *inode, int how)
{
LIST_HEAD(head);
- int res;
+ int may_wait = how & FLUSH_SYNC;
+ int res = 0;

+ if (!nfs_commit_set_lock(NFS_I(inode), may_wait))
+ goto out;
spin_lock(&inode->i_lock);
res = nfs_scan_commit(inode, &head, 0, 0);
spin_unlock(&inode->i_lock);
@@ -1372,7 +1394,13 @@ static int nfs_commit_inode(struct inode *inode, int how)
int error = nfs_commit_list(inode, &head, how);
if (error < 0)
return error;
- }
+ if (may_wait)
+ wait_on_bit(&NFS_I(inode)->flags, NFS_INO_COMMIT,
+ nfs_wait_bit_killable,
+ TASK_KILLABLE);
+ } else
+ nfs_commit_clear_lock(NFS_I(inode));
+out:
return res;
}

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 1a0b85a..07ce460 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -209,6 +209,7 @@ struct nfs_inode {
#define NFS_INO_FLUSHING (4) /* inode is flushing out data */
#define NFS_INO_FSCACHE (5) /* inode can be cached by FS-Cache */
#define NFS_INO_FSCACHE_LOCK (6) /* FS-Cache cookie management lock */
+#define NFS_INO_COMMIT (7) /* inode is committing unstable writes */

static inline struct nfs_inode *NFS_I(const struct inode *inode)
{
--
1.6.6.1


2010-04-22 20:50:26

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 3/5] NFS: rsize and wsize settings ignored on v4 mounts

From: Chuck Lever <[email protected]>

NFSv4 mounts ignore the rsize and wsize mount options, and always use
the default transfer size for both. This seems to be because all
NFSv4 mounts are now cloned, and the cloning logic doesn't copy the
rsize and wsize settings from the parent nfs_server.

I tested Fedora's 2.6.32.11-99 and it seems to have this problem as
well, so I'm guessing that .33, .32, and perhaps older kernels have
this issue as well.

Signed-off-by: Chuck Lever <[email protected]>
Cc: Stable <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/client.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index a8766c4..acc9c49 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -966,6 +966,8 @@ out_error:
static void nfs_server_copy_userdata(struct nfs_server *target, struct nfs_server *source)
{
target->flags = source->flags;
+ target->rsize = source->rsize;
+ target->wsize = source->wsize;
target->acregmin = source->acregmin;
target->acregmax = source->acregmax;
target->acdirmin = source->acdirmin;
--
1.6.6.1


2010-04-22 20:50:26

by Myklebust, Trond

[permalink] [raw]
Subject: [PATCH 4/5] nfs: testing for null instead of ERR_PTR()

From: Dan Carpenter <[email protected]>

nfs_path() returns an ERR_PTR(), it doesn't return null.

Signed-off-by: Dan Carpenter <[email protected]>
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/super.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index e016372..f9327bb 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2657,7 +2657,7 @@ static void nfs_fix_devname(const struct path *path, struct vfsmount *mnt)
devname = nfs_path(path->mnt->mnt_devname,
path->mnt->mnt_root, path->dentry,
page, PAGE_SIZE);
- if (devname == NULL)
+ if (IS_ERR(devname))
goto out_freepage;
tmp = kstrdup(devname, GFP_KERNEL);
if (tmp == NULL)
--
1.6.6.1


2010-04-22 20:55:40

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 5/5] NFS: Fix an unstable write data integrity race

On Thu, 2010-04-22 at 16:50 -0400, Trond Myklebust wrote:
> Commit 2c61be0a9478258f77b66208a0c4b1f5f8161c3c (NFS: Ensure that the WRITE
> and COMMIT RPC calls are always uninterruptible) exposed a race on file
> close. In order to ensure correct close-to-open behaviour, we want to wait
> for all outstanding background commit operations to complete.
>
> This patch adds an inode flag that indicates if a commit operation is under
> way, and provides a mechanism to allow ->write_inode() to wait for its
> completion if this is a data integrity flush.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---

BTW: There is a bugzilla report associated to this problem:
https://bugzilla.kernel.org/show_bug.cgi?id=15819

Cheers
Trond