From 4b09ec4b14a168bf2c687e1f598140c3c11e9222 Mon Sep 17 00:00:00 2001 From: Benjamin Coddington Date: Thu, 5 Jan 2017 10:20:16 -0500 Subject: nfs: Don't take a reference on fl->fl_file for LOCK operation I have reports of a crash that look like __fput() was called twice for a NFSv4.0 file. It seems possible that the state manager could try to reclaim a lock and take a reference on the fl->fl_file at the same time the file is being released if, during the close(), a signal interrupts the wait for outstanding IO while removing locks which then skips the removal of that lock. Since 83bfff23e9ed ("nfs4: have do_vfs_lock take an inode pointer") has removed the need to traverse fl->fl_file->f_inode in nfs4_lock_done(), taking that reference is no longer necessary. Signed-off-by: Benjamin Coddington Reviewed-by: Jeff Layton Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs/nfs/nfs4proc.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 6dcbc5defb7a..700ed1fc1075 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -38,7 +38,6 @@ #include #include #include -#include #include #include #include @@ -6127,7 +6126,6 @@ static struct nfs4_lockdata *nfs4_alloc_lockdata(struct file_lock *fl, p->server = server; atomic_inc(&lsp->ls_count); p->ctx = get_nfs_open_context(ctx); - get_file(fl->fl_file); memcpy(&p->fl, fl, sizeof(p->fl)); return p; out_free_seqid: @@ -6240,7 +6238,6 @@ static void nfs4_lock_release(void *calldata) nfs_free_seqid(data->arg.lock_seqid); nfs4_put_lock_state(data->lsp); put_nfs_open_context(data->ctx); - fput(data->fl.fl_file); kfree(data); dprintk("%s: done!\n", __func__); } -- cgit v1.2.3 From 2dfc61736482441993bfb7dfaa971113b53f107c Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 11 Jan 2017 08:47:00 -0500 Subject: NFSv4: Call update_changeattr() from _nfs4_proc_open only if a file was created We don't want to invalidate the directory attribute and data cache unless we know that a file was created, or the change attribute differs from the one in our cache. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/nfs/nfs4proc.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 700ed1fc1075..4010c33151ad 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2390,11 +2390,12 @@ static int _nfs4_proc_open(struct nfs4_opendata *data) nfs_fattr_map_and_free_names(server, &data->f_attr); if (o_arg->open_flags & O_CREAT) { - update_changeattr(dir, &o_res->cinfo); if (o_arg->open_flags & O_EXCL) data->file_created = 1; else if (o_res->cinfo.before != o_res->cinfo.after) data->file_created = 1; + if (data->file_created || dir->i_version != o_res->cinfo.after) + update_changeattr(dir, &o_res->cinfo); } if ((o_res->rflags & NFS4_OPEN_RESULT_LOCKTYPE_POSIX) == 0) server->caps &= ~NFS_CAP_POSIX_LOCK; -- cgit v1.2.3 From c733c49c32624f927f443be6dbabb387006bbe42 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 11 Jan 2017 12:32:26 -0500 Subject: NFSv4: Don't apply change_info4 twice on rename within a directory If a file is renamed, but stays in the same directory, we will still receive 2 change_info4 structures describing the change to that directory, but we only want to apply it once. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'fs/nfs/nfs4proc.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 4010c33151ad..1e797bf74aaf 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4159,8 +4159,11 @@ static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir, if (nfs4_async_handle_error(task, res->server, NULL, &data->timeout) == -EAGAIN) return 0; - update_changeattr(old_dir, &res->old_cinfo); - update_changeattr(new_dir, &res->new_cinfo); + if (task->tk_status == 0) { + update_changeattr(old_dir, &res->old_cinfo); + if (new_dir != old_dir) + update_changeattr(new_dir, &res->new_cinfo); + } return 1; } -- cgit v1.2.3 From c40d52fe1c2ba25dcb8cd207c8d26ef5f57f0476 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 11 Jan 2017 12:36:11 -0500 Subject: NFSv4: Don't call update_changeattr() unless the unlink is successful If the unlink wasn't successful, then the directory has presumably not changed. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/nfs/nfs4proc.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 1e797bf74aaf..6a35204affa4 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4125,7 +4125,8 @@ static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir) if (nfs4_async_handle_error(task, res->server, NULL, &data->timeout) == -EAGAIN) return 0; - update_changeattr(dir, &res->cinfo); + if (task->tk_status == 0) + update_changeattr(dir, &res->cinfo); return 1; } -- cgit v1.2.3 From d3129ef672cac81c4d0185336af377c8dc1091d3 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Wed, 11 Jan 2017 22:07:28 -0500 Subject: NFSv4: update_changeattr should update the attribute timestamp Otherwise, the attribute cache remains marked as being expired. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) (limited to 'fs/nfs/nfs4proc.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 6a35204affa4..ecc151697fd4 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -1082,7 +1082,8 @@ int nfs4_call_sync(struct rpc_clnt *clnt, return nfs4_call_sync_sequence(clnt, server, msg, args, res); } -static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) +static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo, + unsigned long timestamp) { struct nfs_inode *nfsi = NFS_I(dir); @@ -1098,6 +1099,7 @@ static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo) NFS_INO_INVALID_ACL; } dir->i_version = cinfo->after; + nfsi->read_cache_jiffies = timestamp; nfsi->attr_gencount = nfs_inc_attr_generation_counter(); nfs_fscache_invalidate(dir); spin_unlock(&dir->i_lock); @@ -2395,7 +2397,8 @@ static int _nfs4_proc_open(struct nfs4_opendata *data) else if (o_res->cinfo.before != o_res->cinfo.after) data->file_created = 1; if (data->file_created || dir->i_version != o_res->cinfo.after) - update_changeattr(dir, &o_res->cinfo); + update_changeattr(dir, &o_res->cinfo, + o_res->f_attr->time_start); } if ((o_res->rflags & NFS4_OPEN_RESULT_LOCKTYPE_POSIX) == 0) server->caps &= ~NFS_CAP_POSIX_LOCK; @@ -4073,11 +4076,12 @@ static int _nfs4_proc_remove(struct inode *dir, const struct qstr *name) .rpc_argp = &args, .rpc_resp = &res, }; + unsigned long timestamp = jiffies; int status; status = nfs4_call_sync(server->client, server, &msg, &args.seq_args, &res.seq_res, 1); if (status == 0) - update_changeattr(dir, &res.cinfo); + update_changeattr(dir, &res.cinfo, timestamp); return status; } @@ -4126,7 +4130,7 @@ static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir) &data->timeout) == -EAGAIN) return 0; if (task->tk_status == 0) - update_changeattr(dir, &res->cinfo); + update_changeattr(dir, &res->cinfo, res->dir_attr->time_start); return 1; } @@ -4161,9 +4165,9 @@ static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir, return 0; if (task->tk_status == 0) { - update_changeattr(old_dir, &res->old_cinfo); + update_changeattr(old_dir, &res->old_cinfo, res->old_fattr->time_start); if (new_dir != old_dir) - update_changeattr(new_dir, &res->new_cinfo); + update_changeattr(new_dir, &res->new_cinfo, res->new_fattr->time_start); } return 1; } @@ -4201,7 +4205,7 @@ static int _nfs4_proc_link(struct inode *inode, struct inode *dir, const struct status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1); if (!status) { - update_changeattr(dir, &res.cinfo); + update_changeattr(dir, &res.cinfo, res.fattr->time_start); status = nfs_post_op_update_inode(inode, res.fattr); if (!status) nfs_setsecurity(inode, res.fattr, res.label); @@ -4276,7 +4280,8 @@ static int nfs4_do_create(struct inode *dir, struct dentry *dentry, struct nfs4_ int status = nfs4_call_sync(NFS_SERVER(dir)->client, NFS_SERVER(dir), &data->msg, &data->arg.seq_args, &data->res.seq_res, 1); if (status == 0) { - update_changeattr(dir, &data->res.dir_cinfo); + update_changeattr(dir, &data->res.dir_cinfo, + data->res.fattr->time_start); status = nfs_instantiate(dentry, data->res.fh, data->res.fattr, data->res.label); } return status; -- cgit v1.2.3