From: Anna Schumaker <[email protected]>
I skimmed through the file and cleaned up a few things that stuck out to me.
Thoughts?
Anna
Anna Schumaker (10):
NFS: Fix inconsistent indentation in nfs4proc.c
NFS: Clean up _nfs4_is_integrity_protected()
NFS: Remove nfs4_wait_for_completion_rpc_task()
NFS: Return errors directly in _nfs4_opendata_reclaim_to_nfs4_state()
NFS: Remove an extra if in _nfs4_recover_proc_open()
NFS: Remove nfs4_recover_expired_lease()
NFS: Remove unnecessary goto in nfs4_lookup_root_sec()
NFS: No need to set and return status in nfs41_lock_expired()
NFS: Clean up nfs41_same_server_scope()
NFS: Return the comparison result directly in nfs41_match_stateid()
fs/nfs/nfs4proc.c | 132 ++++++++++++++++++++----------------------------------
1 file changed, 48 insertions(+), 84 deletions(-)
--
2.11.0
From: Anna Schumaker <[email protected]>
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 00cb8b6c2c70..950e9cd15e5d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1244,14 +1244,6 @@ static void nfs4_opendata_put(struct nfs4_opendata *p)
kref_put(&p->kref, nfs4_opendata_free);
}
-static int nfs4_wait_for_completion_rpc_task(struct rpc_task *task)
-{
- int ret;
-
- ret = rpc_wait_for_completion_task(task);
- return ret;
-}
-
static bool nfs4_mode_match_open_stateid(struct nfs4_state *state,
fmode_t fmode)
{
@@ -2038,7 +2030,7 @@ static int _nfs4_proc_open_confirm(struct nfs4_opendata *data)
task = rpc_run_task(&task_setup_data);
if (IS_ERR(task))
return PTR_ERR(task);
- status = nfs4_wait_for_completion_rpc_task(task);
+ status = rpc_wait_for_completion_task(task);
if (status != 0) {
data->cancelled = 1;
smp_wmb();
@@ -2205,7 +2197,7 @@ static int nfs4_run_open_task(struct nfs4_opendata *data, int isrecover)
task = rpc_run_task(&task_setup_data);
if (IS_ERR(task))
return PTR_ERR(task);
- status = nfs4_wait_for_completion_rpc_task(task);
+ status = rpc_wait_for_completion_task(task);
if (status != 0) {
data->cancelled = 1;
smp_wmb();
@@ -5722,7 +5714,7 @@ static int _nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, co
return PTR_ERR(task);
if (!issync)
goto out;
- status = nfs4_wait_for_completion_rpc_task(task);
+ status = rpc_wait_for_completion_task(task);
if (status != 0)
goto out;
status = data->rpc_status;
@@ -5992,7 +5984,7 @@ static int nfs4_proc_unlck(struct nfs4_state *state, int cmd, struct file_lock *
status = PTR_ERR(task);
if (IS_ERR(task))
goto out;
- status = nfs4_wait_for_completion_rpc_task(task);
+ status = rpc_wait_for_completion_task(task);
rpc_put_task(task);
out:
request->fl_flags = fl_flags;
@@ -6221,7 +6213,7 @@ static int _nfs4_do_setlk(struct nfs4_state *state, int cmd, struct file_lock *f
task = rpc_run_task(&task_setup_data);
if (IS_ERR(task))
return PTR_ERR(task);
- ret = nfs4_wait_for_completion_rpc_task(task);
+ ret = rpc_wait_for_completion_task(task);
if (ret == 0) {
ret = data->rpc_status;
if (ret)
@@ -8289,7 +8281,7 @@ static int nfs41_proc_reclaim_complete(struct nfs_client *clp,
status = PTR_ERR(task);
goto out;
}
- status = nfs4_wait_for_completion_rpc_task(task);
+ status = rpc_wait_for_completion_task(task);
if (status == 0)
status = task->tk_status;
rpc_put_task(task);
@@ -8520,7 +8512,7 @@ nfs4_proc_layoutget(struct nfs4_layoutget *lgp, long *timeout, gfp_t gfp_flags)
task = rpc_run_task(&task_setup_data);
if (IS_ERR(task))
return ERR_CAST(task);
- status = nfs4_wait_for_completion_rpc_task(task);
+ status = rpc_wait_for_completion_task(task);
if (status == 0) {
status = nfs4_layoutget_handle_exception(task, lgp, &exception);
*timeout = exception.timeout;
--
2.11.0
From: Anna Schumaker <[email protected]>
Once again, it's easier and cleaner just to return the error directly.
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index bde48f02bd28..67aa9cf5f569 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3412,16 +3412,11 @@ static int nfs4_lookup_root_sec(struct nfs_server *server, struct nfs_fh *fhandl
.pseudoflavor = flavor,
};
struct rpc_auth *auth;
- int ret;
auth = rpcauth_create(&auth_args, server->client);
- if (IS_ERR(auth)) {
- ret = -EACCES;
- goto out;
- }
- ret = nfs4_lookup_root(server, fhandle, info);
-out:
- return ret;
+ if (IS_ERR(auth))
+ return -EACCES;
+ return nfs4_lookup_root(server, fhandle, info);
}
/*
--
2.11.0
From: Anna Schumaker <[email protected]>
There is no need for a goto just to return an error code without any
cleanup. Returning the error directly helps to clean up the code.
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 950e9cd15e5d..83d31c93065e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1638,17 +1638,15 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data)
int ret;
if (!data->rpc_done) {
- if (data->rpc_status) {
- ret = data->rpc_status;
- goto err;
- }
+ if (data->rpc_status)
+ return ERR_PTR(data->rpc_status);
/* cached opens have already been processed */
goto update;
}
ret = nfs_refresh_inode(inode, &data->f_attr);
if (ret)
- goto err;
+ return ERR_PTR(ret);
if (data->o_res.delegation_type != 0)
nfs4_opendata_check_deleg(data, state);
@@ -1658,9 +1656,6 @@ _nfs4_opendata_reclaim_to_nfs4_state(struct nfs4_opendata *data)
atomic_inc(&state->count);
return state;
-err:
- return ERR_PTR(ret);
-
}
static struct nfs4_state *
--
2.11.0
From: Anna Schumaker <[email protected]>
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 56 +++++++++++++++++++++++++++----------------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 94fdf26f9bf4..e10ec98ef5ce 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2208,15 +2208,15 @@ static int nfs4_run_open_task(struct nfs4_opendata *data, int isrecover)
data->is_recover = 1;
}
task = rpc_run_task(&task_setup_data);
- if (IS_ERR(task))
- return PTR_ERR(task);
- status = nfs4_wait_for_completion_rpc_task(task);
- if (status != 0) {
- data->cancelled = 1;
- smp_wmb();
- } else
- status = data->rpc_status;
- rpc_put_task(task);
+ if (IS_ERR(task))
+ return PTR_ERR(task);
+ status = nfs4_wait_for_completion_rpc_task(task);
+ if (status != 0) {
+ data->cancelled = 1;
+ smp_wmb();
+ } else
+ status = data->rpc_status;
+ rpc_put_task(task);
return status;
}
@@ -2225,7 +2225,7 @@ static int _nfs4_recover_proc_open(struct nfs4_opendata *data)
{
struct inode *dir = d_inode(data->dir);
struct nfs_openres *o_res = &data->o_res;
- int status;
+ int status;
status = nfs4_run_open_task(data, 1);
if (status != 0 || !data->rpc_done)
@@ -2856,12 +2856,12 @@ static int _nfs4_do_setattr(struct inode *inode,
struct nfs_open_context *ctx)
{
struct nfs_server *server = NFS_SERVER(inode);
- struct rpc_message msg = {
+ struct rpc_message msg = {
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETATTR],
.rpc_argp = arg,
.rpc_resp = res,
.rpc_cred = cred,
- };
+ };
struct rpc_cred *delegation_cred = NULL;
unsigned long timestamp = jiffies;
fmode_t fmode;
@@ -2909,18 +2909,18 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred,
{
struct nfs_server *server = NFS_SERVER(inode);
struct nfs4_state *state = ctx ? ctx->state : NULL;
- struct nfs_setattrargs arg = {
- .fh = NFS_FH(inode),
- .iap = sattr,
+ struct nfs_setattrargs arg = {
+ .fh = NFS_FH(inode),
+ .iap = sattr,
.server = server,
.bitmask = server->attr_bitmask,
.label = ilabel,
- };
- struct nfs_setattrres res = {
+ };
+ struct nfs_setattrres res = {
.fattr = fattr,
.label = olabel,
.server = server,
- };
+ };
struct nfs4_exception exception = {
.state = state,
.inode = inode,
@@ -3034,7 +3034,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
}
}
- /* hmm. we are done with the inode, and in the process of freeing
+ /* hmm. we are done with the inode, and in the process of freeing
* the state_owner. we keep this around to process errors
*/
switch (task->tk_status) {
@@ -4885,8 +4885,8 @@ static int buf_to_pages_noslab(const void *buf, size_t buflen,
if (newpage == NULL)
goto unwind;
memcpy(page_address(newpage), buf, len);
- buf += len;
- buflen -= len;
+ buf += len;
+ buflen -= len;
*pages++ = newpage;
rc++;
} while (buflen != 0);
@@ -5209,8 +5209,8 @@ static int _nfs4_do_set_security_label(struct inode *inode,
struct nfs_server *server = NFS_SERVER(inode);
const u32 bitmask[3] = { 0, 0, FATTR4_WORD2_SECURITY_LABEL };
struct nfs_setattrargs arg = {
- .fh = NFS_FH(inode),
- .iap = &sattr,
+ .fh = NFS_FH(inode),
+ .iap = &sattr,
.server = server,
.bitmask = bitmask,
.label = ilabel,
@@ -5221,9 +5221,9 @@ static int _nfs4_do_set_security_label(struct inode *inode,
.server = server,
};
struct rpc_message msg = {
- .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETATTR],
- .rpc_argp = &arg,
- .rpc_resp = &res,
+ .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SETATTR],
+ .rpc_argp = &arg,
+ .rpc_resp = &res,
};
int status;
@@ -5769,8 +5769,8 @@ static int _nfs4_proc_getlk(struct nfs4_state *state, int cmd, struct file_lock
};
struct rpc_message msg = {
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_LOCKT],
- .rpc_argp = &arg,
- .rpc_resp = &res,
+ .rpc_argp = &arg,
+ .rpc_resp = &res,
.rpc_cred = state->owner->so_cred,
};
struct nfs4_lock_state *lsp;
--
2.11.0
From: Anna Schumaker <[email protected]>
We can cut out the if statement and return the results of the comparison
directly.
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e10ec98ef5ce..00cb8b6c2c70 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -578,12 +578,7 @@ nfs4_async_handle_error(struct rpc_task *task, struct nfs_server *server,
static bool _nfs4_is_integrity_protected(struct nfs_client *clp)
{
rpc_authflavor_t flavor = clp->cl_rpcclient->cl_auth->au_flavor;
-
- if (flavor == RPC_AUTH_GSS_KRB5I ||
- flavor == RPC_AUTH_GSS_KRB5P)
- return true;
-
- return false;
+ return (flavor == RPC_AUTH_GSS_KRB5I) || (flavor == RPC_AUTH_GSS_KRB5P);
}
static void do_renew_lease(struct nfs_client *clp, unsigned long timestamp)
--
2.11.0
From: Anna Schumaker <[email protected]>
It's simpler just to return the status unconditionally
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 83d31c93065e..02767aced45c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2215,11 +2215,8 @@ static int _nfs4_recover_proc_open(struct nfs4_opendata *data)
nfs_fattr_map_and_free_names(NFS_SERVER(dir), &data->f_attr);
- if (o_res->rflags & NFS4_OPEN_RESULT_CONFIRM) {
+ if (o_res->rflags & NFS4_OPEN_RESULT_CONFIRM)
status = _nfs4_proc_open_confirm(data);
- if (status != 0)
- return status;
- }
return status;
}
--
2.11.0
From: Anna Schumaker <[email protected]>
This function doesn't add much, since all it does is access the server's
nfs_client variable.
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 02767aced45c..bde48f02bd28 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2308,11 +2308,6 @@ static int _nfs4_proc_open(struct nfs4_opendata *data)
return 0;
}
-static int nfs4_recover_expired_lease(struct nfs_server *server)
-{
- return nfs4_client_recover_expired_lease(server->nfs_client);
-}
-
/*
* OPEN_EXPIRED:
* reclaim state on the server after a network partition.
@@ -2689,7 +2684,7 @@ static int _nfs4_do_open(struct inode *dir,
dprintk("nfs4_do_open: nfs4_get_state_owner failed!\n");
goto out_err;
}
- status = nfs4_recover_expired_lease(server);
+ status = nfs4_client_recover_expired_lease(server->nfs_client);
if (status != 0)
goto err_put_state_owner;
if (d_really_is_positive(dentry))
--
2.11.0
From: Anna Schumaker <[email protected]>
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 67aa9cf5f569..c6d0a93ca652 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6274,8 +6274,7 @@ static int nfs41_lock_expired(struct nfs4_state *state, struct file_lock *reques
if (test_bit(NFS_LOCK_INITIALIZED, &lsp->ls_flags) ||
test_bit(NFS_LOCK_LOST, &lsp->ls_flags))
return 0;
- status = nfs4_lock_expired(state, request);
- return status;
+ return nfs4_lock_expired(state, request);
}
#endif
--
2.11.0
From: Anna Schumaker <[email protected]>
The function is cleaner this way, since we can use the result of
memcmp() directly
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index c6d0a93ca652..8395432c6fb1 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7112,11 +7112,9 @@ static bool
nfs41_same_server_scope(struct nfs41_server_scope *a,
struct nfs41_server_scope *b)
{
- if (a->server_scope_sz == b->server_scope_sz &&
- memcmp(a->server_scope, b->server_scope, a->server_scope_sz) == 0)
- return true;
-
- return false;
+ if (a->server_scope_sz != b->server_scope_sz)
+ return false;
+ return memcmp(a->server_scope, b->server_scope, a->server_scope_sz) == 0;
}
static void
--
2.11.0
From: Anna Schumaker <[email protected]>
Signed-off-by: Anna Schumaker <[email protected]>
---
fs/nfs/nfs4proc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 8395432c6fb1..06190beb2c52 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9107,10 +9107,8 @@ static bool nfs41_match_stateid(const nfs4_stateid *s1,
if (s1->seqid == s2->seqid)
return true;
- if (s1->seqid == 0 || s2->seqid == 0)
- return true;
- return false;
+ return s1->seqid == 0 || s2->seqid == 0;
}
#endif /* CONFIG_NFS_V4_1 */
--
2.11.0