[PATCH 0/5] skip op encoders in error cases

Linux NFS development
 help / color / mirror / Atom feed

* [PATCH 0/5] skip op encoders in error cases
@ 2017-07-14 20:24 J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 1/5] nfsd4: move some nfsd4 op definitions to xdr4.h J. Bruce Fields
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-14 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

This is just some cleanup:

f961e3f2acae "nfsd: encoders mustn't use unitialized values in error
cases" fixed a couple cases like this:

	const struct nfsd4_layout_ops *ops =
                nfsd4_layout_ops[lgp->lg_layout_type];
        __be32 *p;

        dprintk("%s: err %d\n", __func__, nfserr);
        if (nfserr)
                goto out;

which ignored the fact that lg_layout_type isn't even initialized in
some error cases.

That seemed like an easy mistake to make, and made me wonder why we even
obther calling the xdr encoders in the error case, since almost all of
them are no-ops then.

So, these patches skip that.  I think it's an improvement, but I'll
listen to reason.

--b.

J. Bruce Fields (5):
  nfsd4: move some nfsd4 op definitions to xdr4.h
  opdesc will be useful outside nfs4proc.c
  nfsd4: define ->op_release for compound ops
  nfsd4: skip encoder in trivial error cases
  nfsd4: individual encoders no longer see error cases

 fs/nfsd/nfs4proc.c | 109 ++++++++----------
 fs/nfsd/nfs4xdr.c  | 322 ++++++++++++++++++-----------------------------------
 fs/nfsd/xdr4.h     |  49 ++++++++
 3 files changed, 208 insertions(+), 272 deletions(-)

-- 
2.9.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] nfsd4: move some nfsd4 op definitions to xdr4.h
  2017-07-14 20:24 [PATCH 0/5] skip op encoders in error cases J. Bruce Fields
@ 2017-07-14 20:24 ` J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 2/5] opdesc will be useful outside nfs4proc.c J. Bruce Fields
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-14 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

I want code in nfs4xdr.c to have access to this stuff.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c | 43 -------------------------------------------
 fs/nfsd/xdr4.h     | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index d27e75ad25e3..9a627a6c52aa 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1541,49 +1541,6 @@ static inline void nfsd4_increment_op_stats(u32 opnum)
 		nfsdstats.nfs4_opcount[opnum]++;
 }
 
-enum nfsd4_op_flags {
-	ALLOWED_WITHOUT_FH = 1 << 0,	/* No current filehandle required */
-	ALLOWED_ON_ABSENT_FS = 1 << 1,	/* ops processed on absent fs */
-	ALLOWED_AS_FIRST_OP = 1 << 2,	/* ops reqired first in compound */
-	/* For rfc 5661 section 2.6.3.1.1: */
-	OP_HANDLES_WRONGSEC = 1 << 3,
-	OP_IS_PUTFH_LIKE = 1 << 4,
-	/*
-	 * These are the ops whose result size we estimate before
-	 * encoding, to avoid performing an op then not being able to
-	 * respond or cache a response.  This includes writes and setattrs
-	 * as well as the operations usually called "nonidempotent":
-	 */
-	OP_MODIFIES_SOMETHING = 1 << 5,
-	/*
-	 * Cache compounds containing these ops in the xid-based drc:
-	 * We use the DRC for compounds containing non-idempotent
-	 * operations, *except* those that are 4.1-specific (since
-	 * sessions provide their own EOS), and except for stateful
-	 * operations other than setclientid and setclientid_confirm
-	 * (since sequence numbers provide EOS for open, lock, etc in
-	 * the v4.0 case).
-	 */
-	OP_CACHEME = 1 << 6,
-	/*
-	 * These are ops which clear current state id.
-	 */
-	OP_CLEAR_STATEID = 1 << 7,
-};
-
-struct nfsd4_operation {
-	__be32 (*op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
-			union nfsd4_op_u *);
-	u32 op_flags;
-	char *op_name;
-	/* Try to get response size before operation */
-	u32 (*op_rsize_bop)(struct svc_rqst *, struct nfsd4_op *);
-	void (*op_get_currentstateid)(struct nfsd4_compound_state *,
-			union nfsd4_op_u *);
-	void (*op_set_currentstateid)(struct nfsd4_compound_state *,
-			union nfsd4_op_u *);
-};
-
 static const struct nfsd4_operation nfsd4_ops[];
 
 static const char *nfsd4_op_name(unsigned opnum);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 72c6ad136107..fcdfe1c3ff4f 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -748,6 +748,50 @@ extern __be32 nfsd4_free_stateid(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *, union nfsd4_op_u *);
 extern void nfsd4_bump_seqid(struct nfsd4_compound_state *, __be32 nfserr);
 
+enum nfsd4_op_flags {
+	ALLOWED_WITHOUT_FH = 1 << 0,    /* No current filehandle required */
+	ALLOWED_ON_ABSENT_FS = 1 << 1,  /* ops processed on absent fs */
+	ALLOWED_AS_FIRST_OP = 1 << 2,   /* ops reqired first in compound */
+	/* For rfc 5661 section 2.6.3.1.1: */
+	OP_HANDLES_WRONGSEC = 1 << 3,
+	OP_IS_PUTFH_LIKE = 1 << 4,
+	/*
+	 * These are the ops whose result size we estimate before
+	 * encoding, to avoid performing an op then not being able to
+	 * respond or cache a response.  This includes writes and setattrs
+	 * as well as the operations usually called "nonidempotent":
+	 */
+	OP_MODIFIES_SOMETHING = 1 << 5,
+	/*
+	 * Cache compounds containing these ops in the xid-based drc:
+	 * We use the DRC for compounds containing non-idempotent
+	 * operations, *except* those that are 4.1-specific (since
+	 * sessions provide their own EOS), and except for stateful
+	 * operations other than setclientid and setclientid_confirm
+	 * (since sequence numbers provide EOS for open, lock, etc in
+	 * the v4.0 case).
+	 */
+	OP_CACHEME = 1 << 6,
+	/*
+	 * These are ops which clear current state id.
+	 */
+	OP_CLEAR_STATEID = 1 << 7,
+};
+
+struct nfsd4_operation {
+	__be32 (*op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
+			union nfsd4_op_u *);
+	u32 op_flags;
+	char *op_name;
+	/* Try to get response size before operation */
+	u32 (*op_rsize_bop)(struct svc_rqst *, struct nfsd4_op *);
+	void (*op_get_currentstateid)(struct nfsd4_compound_state *,
+			union nfsd4_op_u *);
+	void (*op_set_currentstateid)(struct nfsd4_compound_state *,
+			union nfsd4_op_u *);
+};
+
+
 #endif
 
 /*
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] opdesc will be useful outside nfs4proc.c
  2017-07-14 20:24 [PATCH 0/5] skip op encoders in error cases J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 1/5] nfsd4: move some nfsd4 op definitions to xdr4.h J. Bruce Fields
@ 2017-07-14 20:24 ` J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 3/5] nfsd4: define ->op_release for compound ops J. Bruce Fields
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-14 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

---
 fs/nfsd/nfs4proc.c | 25 +++++++++++--------------
 fs/nfsd/nfs4xdr.c  |  1 +
 fs/nfsd/xdr4.h     |  2 ++
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9a627a6c52aa..01d7f2456f62 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1578,7 +1578,7 @@ static __be32 nfs41_check_op_ordering(struct nfsd4_compoundargs *args)
 	return nfs_ok;
 }
 
-static inline const struct nfsd4_operation *OPDESC(struct nfsd4_op *op)
+const struct nfsd4_operation *OPDESC(struct nfsd4_op *op)
 {
 	return &nfsd4_ops[op->opnum];
 }
@@ -1651,7 +1651,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 	struct nfsd4_compoundargs *args = rqstp->rq_argp;
 	struct nfsd4_compoundres *resp = rqstp->rq_resp;
 	struct nfsd4_op	*op;
-	const struct nfsd4_operation *opdesc;
 	struct nfsd4_compound_state *cstate = &resp->cstate;
 	struct svc_fh *current_fh = &cstate->current_fh;
 	struct svc_fh *save_fh = &cstate->save_fh;
@@ -1704,15 +1703,13 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 			goto encode_op;
 		}
 
-		opdesc = OPDESC(op);
-
 		if (!current_fh->fh_dentry) {
-			if (!(opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
+			if (!(op->opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
 				op->status = nfserr_nofilehandle;
 				goto encode_op;
 			}
 		} else if (current_fh->fh_export->ex_fslocs.migrated &&
-			  !(opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) {
+			  !(op->opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) {
 			op->status = nfserr_moved;
 			goto encode_op;
 		}
@@ -1720,12 +1717,12 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 		fh_clear_wcc(current_fh);
 
 		/* If op is non-idempotent */
-		if (opdesc->op_flags & OP_MODIFIES_SOMETHING) {
+		if (op->opdesc->op_flags & OP_MODIFIES_SOMETHING) {
 			/*
 			 * Don't execute this op if we couldn't encode a
 			 * succesful reply:
 			 */
-			u32 plen = opdesc->op_rsize_bop(rqstp, op);
+			u32 plen = op->opdesc->op_rsize_bop(rqstp, op);
 			/*
 			 * Plus if there's another operation, make sure
 			 * we'll have space to at least encode an error:
@@ -1738,9 +1735,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 		if (op->status)
 			goto encode_op;
 
-		if (opdesc->op_get_currentstateid)
-			opdesc->op_get_currentstateid(cstate, &op->u);
-		op->status = opdesc->op_func(rqstp, cstate, &op->u);
+		if (op->opdesc->op_get_currentstateid)
+			op->opdesc->op_get_currentstateid(cstate, &op->u);
+		op->status = op->opdesc->op_func(rqstp, cstate, &op->u);
 
 		/* Only from SEQUENCE */
 		if (cstate->status == nfserr_replay_cache) {
@@ -1749,10 +1746,10 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
 			goto out;
 		}
 		if (!op->status) {
-			if (opdesc->op_set_currentstateid)
-				opdesc->op_set_currentstateid(cstate, &op->u);
+			if (op->opdesc->op_set_currentstateid)
+				op->opdesc->op_set_currentstateid(cstate, &op->u);
 
-			if (opdesc->op_flags & OP_CLEAR_STATEID)
+			if (op->opdesc->op_flags & OP_CLEAR_STATEID)
 				clear_current_stateid(cstate);
 
 			if (need_wrongsec_check(rqstp))
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 20fbcab97753..40ed23fda814 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1929,6 +1929,7 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)
 			op->opnum = OP_ILLEGAL;
 			op->status = nfserr_op_illegal;
 		}
+		op->opdesc = OPDESC(op);
 		/*
 		 * We'll try to cache the result in the DRC if any one
 		 * op in the compound wants to be cached:
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index fcdfe1c3ff4f..90b928006bc7 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -538,6 +538,7 @@ struct nfsd4_seek {
 
 struct nfsd4_op {
 	int					opnum;
+	const struct nfsd4_operation *		opdesc;
 	__be32					status;
 	union nfsd4_op_u {
 		struct nfsd4_access		access;
@@ -661,6 +662,7 @@ static inline bool nfsd4_last_compound_op(struct svc_rqst *rqstp)
 	return argp->opcnt == resp->opcnt;
 }
 
+const struct nfsd4_operation *OPDESC(struct nfsd4_op *op);
 int nfsd4_max_reply(struct svc_rqst *rqstp, struct nfsd4_op *op);
 void warn_on_nonidempotent_op(struct nfsd4_op *op);
 
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] nfsd4: define ->op_release for compound ops
  2017-07-14 20:24 [PATCH 0/5] skip op encoders in error cases J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 1/5] nfsd4: move some nfsd4 op definitions to xdr4.h J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 2/5] opdesc will be useful outside nfs4proc.c J. Bruce Fields
@ 2017-07-14 20:24 ` J. Bruce Fields
  2017-07-15  9:27   ` kbuild test robot
  2017-07-15  9:30   ` kbuild test robot
  2017-07-14 20:24 ` [PATCH 4/5] nfsd4: skip encoder in trivial error cases J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 5/5] nfsd4: individual encoders no longer see " J. Bruce Fields
  4 siblings, 2 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-14 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Run a separate ->op_release function if necessary instead of depending
on the xdr encoder to do this.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c | 31 +++++++++++++++++++++++++++++++
 fs/nfsd/nfs4xdr.c  |  9 +++------
 fs/nfsd/xdr4.h     |  1 +
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 01d7f2456f62..8944b00fd3cd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -784,6 +784,14 @@ nfsd4_read(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	return status;
 }
 
+
+static void
+nfsd4_read_release(union nfsd4_op_u *u)
+{
+	if (u->read.rd_filp)
+		fput(u->read.rd_filp);
+}
+
 static __be32
 nfsd4_readdir(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	      union nfsd4_op_u *u)
@@ -912,6 +920,13 @@ nfsd4_secinfo_no_name(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstat
 	return nfs_ok;
 }
 
+static void
+nfsd4_secinfo_release(union nfsd4_op_u *u)
+{
+	if (u->secinfo.si_exp)
+		exp_put(u->secinfo.si_exp);
+}
+
 static __be32
 nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	      union nfsd4_op_u *u)
@@ -1335,6 +1350,12 @@ nfsd4_getdeviceinfo(struct svc_rqst *rqstp,
 	return nfserr;
 }
 
+static void
+nfsd4_getdeviceinfo_release(union nfsd4_op_u *u)
+{
+	kfree(gdev->gd_device);
+}
+
 static __be32
 nfsd4_layoutget(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
@@ -1415,6 +1436,11 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
 	return nfserr;
 }
 
+nfsd4_layoutget_release(union nfsd4_op_u *u)
+{
+	kfree(lgp->lg_content);
+}
+
 static __be32
 nfsd4_layoutcommit(struct svc_rqst *rqstp,
 		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
@@ -2192,6 +2218,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_READ] = {
 		.op_func = nfsd4_read,
+		.op_release = nfsd4_read_release,
 		.op_name = "OP_READ",
 		.op_rsize_bop = nfsd4_read_rsize,
 		.op_get_currentstateid = nfsd4_get_readstateid,
@@ -2241,6 +2268,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_SECINFO] = {
 		.op_func = nfsd4_secinfo,
+		.op_release = nfsd4_secinfo_release,
 		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SECINFO",
 		.op_rsize_bop = nfsd4_secinfo_rsize,
@@ -2342,6 +2370,7 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_SECINFO_NO_NAME] = {
 		.op_func = nfsd4_secinfo_no_name,
+		.op_release = nfsd4_secinfo_release,
 		.op_flags = OP_HANDLES_WRONGSEC,
 		.op_name = "OP_SECINFO_NO_NAME",
 		.op_rsize_bop = nfsd4_secinfo_rsize,
@@ -2362,12 +2391,14 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 #ifdef CONFIG_NFSD_PNFS
 	[OP_GETDEVICEINFO] = {
 		.op_func = nfsd4_getdeviceinfo,
+		.op_release = nfsd4_getdeviceinfo_release,
 		.op_flags = ALLOWED_WITHOUT_FH,
 		.op_name = "OP_GETDEVICEINFO",
 		.op_rsize_bop = nfsd4_getdeviceinfo_rsize,
 	},
 	[OP_LAYOUTGET] = {
 		.op_func = nfsd4_layoutget,
+		.op_release = nfsd4_layoutget_release,
 		.op_flags = OP_MODIFIES_SOMETHING,
 		.op_name = "OP_LAYOUTGET",
 		.op_rsize_bop = nfsd4_layoutget_rsize,
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 40ed23fda814..7d683e3aebf0 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3593,8 +3593,6 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr,
 		xdr_truncate_encode(xdr, starting_len);
 
 out:
-	if (file)
-		fput(file);
 	return nfserr;
 }
 
@@ -3838,8 +3836,6 @@ nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
 	*flavorsp = htonl(supported);
 	nfserr = 0;
 out:
-	if (exp)
-		exp_put(exp);
 	return nfserr;
 }
 
@@ -4172,7 +4168,6 @@ nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
 
 	nfserr = 0;
 out:
-	kfree(gdev->gd_device);
 	dprintk("%s: done: %d\n", __func__, be32_to_cpu(nfserr));
 	return nfserr;
 
@@ -4221,7 +4216,6 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr,
 	ops = nfsd4_layout_ops[lgp->lg_layout_type];
 	nfserr = ops->encode_layoutget(xdr, lgp);
 out:
-	kfree(lgp->lg_content);
 	return nfserr;
 }
 
@@ -4452,6 +4446,7 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
 	struct xdr_stream *xdr = &resp->xdr;
 	struct nfs4_stateowner *so = resp->cstate.replay_owner;
 	struct svc_rqst *rqstp = resp->rqstp;
+	const struct nfsd4_operation *opdesc = op->opdesc;
 	int post_err_offset;
 	nfsd4_enc encoder;
 	__be32 *p;
@@ -4470,6 +4465,8 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
 	       !nfsd4_enc_ops[op->opnum]);
 	encoder = nfsd4_enc_ops[op->opnum];
 	op->status = encoder(resp, op->status, &op->u);
+	if (opdesc && opdesc->op_release)
+		opdesc->op_release(&op->u);
 	xdr_commit_encode(xdr);
 
 	/* nfsd4_check_resp_size guarantees enough room for error status */
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 90b928006bc7..1e6274e0e066 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -783,6 +783,7 @@ enum nfsd4_op_flags {
 struct nfsd4_operation {
 	__be32 (*op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
 			union nfsd4_op_u *);
+	void (*op_release)(union nfsd4_op_u *);
 	u32 op_flags;
 	char *op_name;
 	/* Try to get response size before operation */
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] nfsd4: define ->op_release for compound ops
  2017-07-14 20:24 ` [PATCH 3/5] nfsd4: define ->op_release for compound ops J. Bruce Fields
@ 2017-07-15  9:27   ` kbuild test robot
  2017-07-17 18:44     ` J. Bruce Fields
  2017-07-15  9:30   ` kbuild test robot
  1 sibling, 1 reply; 9+ messages in thread
From: kbuild test robot @ 2017-07-15  9:27 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: kbuild-all, linux-nfs, J. Bruce Fields

[-- Attachment #1: Type: text/plain, Size: 5047 bytes --]

Hi Bruce,

[auto build test ERROR on next-20170710]
[cannot apply to nfsd/nfsd-next v4.12 v4.12-rc7 v4.12-rc6 v4.12]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/J-Bruce-Fields/skip-op-encoders-in-error-cases/20170715-165807
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All error/warnings (new ones prefixed by >>):

   fs/nfsd/nfs4proc.c: In function 'nfsd4_getdeviceinfo_release':
>> fs/nfsd/nfs4proc.c:1356:8: error: 'gdev' undeclared (first use in this function)
     kfree(gdev->gd_device);
           ^
   fs/nfsd/nfs4proc.c:1356:8: note: each undeclared identifier is reported only once for each function it appears in
   fs/nfsd/nfs4proc.c: At top level:
>> fs/nfsd/nfs4proc.c:1439:1: warning: return type defaults to 'int' [-Wreturn-type]
    nfsd4_layoutget_release(union nfsd4_op_u *u)
    ^
   fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
>> fs/nfsd/nfs4proc.c:1441:8: error: 'lgp' undeclared (first use in this function)
     kfree(lgp->lg_content);
           ^
   fs/nfsd/nfs4proc.c: At top level:
>> fs/nfsd/nfs4proc.c:2401:3: warning: initialization from incompatible pointer type
      .op_release = nfsd4_layoutget_release,
      ^
   fs/nfsd/nfs4proc.c:2401:3: warning: (near initialization for 'nfsd4_ops[50].op_release')
   fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
>> fs/nfsd/nfs4proc.c:1442:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +/gdev +1356 fs/nfsd/nfs4proc.c

  1352	
  1353	static void
  1354	nfsd4_getdeviceinfo_release(union nfsd4_op_u *u)
  1355	{
> 1356		kfree(gdev->gd_device);
  1357	}
  1358	
  1359	static __be32
  1360	nfsd4_layoutget(struct svc_rqst *rqstp,
  1361			struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
  1362	{
  1363		struct nfsd4_layoutget *lgp = &u->layoutget;
  1364		struct svc_fh *current_fh = &cstate->current_fh;
  1365		const struct nfsd4_layout_ops *ops;
  1366		struct nfs4_layout_stateid *ls;
  1367		__be32 nfserr;
  1368		int accmode;
  1369	
  1370		switch (lgp->lg_seg.iomode) {
  1371		case IOMODE_READ:
  1372			accmode = NFSD_MAY_READ;
  1373			break;
  1374		case IOMODE_RW:
  1375			accmode = NFSD_MAY_READ | NFSD_MAY_WRITE;
  1376			break;
  1377		default:
  1378			dprintk("%s: invalid iomode %d\n",
  1379				__func__, lgp->lg_seg.iomode);
  1380			nfserr = nfserr_badiomode;
  1381			goto out;
  1382		}
  1383	
  1384		nfserr = fh_verify(rqstp, current_fh, 0, accmode);
  1385		if (nfserr)
  1386			goto out;
  1387	
  1388		nfserr = nfserr_layoutunavailable;
  1389		ops = nfsd4_layout_verify(current_fh->fh_export, lgp->lg_layout_type);
  1390		if (!ops)
  1391			goto out;
  1392	
  1393		/*
  1394		 * Verify minlength and range as per RFC5661:
  1395		 *  o  If loga_length is less than loga_minlength,
  1396		 *     the metadata server MUST return NFS4ERR_INVAL.
  1397		 *  o  If the sum of loga_offset and loga_minlength exceeds
  1398		 *     NFS4_UINT64_MAX, and loga_minlength is not
  1399		 *     NFS4_UINT64_MAX, the error NFS4ERR_INVAL MUST result.
  1400		 *  o  If the sum of loga_offset and loga_length exceeds
  1401		 *     NFS4_UINT64_MAX, and loga_length is not NFS4_UINT64_MAX,
  1402		 *     the error NFS4ERR_INVAL MUST result.
  1403		 */
  1404		nfserr = nfserr_inval;
  1405		if (lgp->lg_seg.length < lgp->lg_minlength ||
  1406		    (lgp->lg_minlength != NFS4_MAX_UINT64 &&
  1407		     lgp->lg_minlength > NFS4_MAX_UINT64 - lgp->lg_seg.offset) ||
  1408		    (lgp->lg_seg.length != NFS4_MAX_UINT64 &&
  1409		     lgp->lg_seg.length > NFS4_MAX_UINT64 - lgp->lg_seg.offset))
  1410			goto out;
  1411		if (lgp->lg_seg.length == 0)
  1412			goto out;
  1413	
  1414		nfserr = nfsd4_preprocess_layout_stateid(rqstp, cstate, &lgp->lg_sid,
  1415							true, lgp->lg_layout_type, &ls);
  1416		if (nfserr) {
  1417			trace_layout_get_lookup_fail(&lgp->lg_sid);
  1418			goto out;
  1419		}
  1420	
  1421		nfserr = nfserr_recallconflict;
  1422		if (atomic_read(&ls->ls_stid.sc_file->fi_lo_recalls))
  1423			goto out_put_stid;
  1424	
  1425		nfserr = ops->proc_layoutget(d_inode(current_fh->fh_dentry),
  1426					     current_fh, lgp);
  1427		if (nfserr)
  1428			goto out_put_stid;
  1429	
  1430		nfserr = nfsd4_insert_layout(lgp, ls);
  1431	
  1432	out_put_stid:
  1433		mutex_unlock(&ls->ls_mutex);
  1434		nfs4_put_stid(&ls->ls_stid);
  1435	out:
  1436		return nfserr;
  1437	}
  1438	
> 1439	nfsd4_layoutget_release(union nfsd4_op_u *u)
  1440	{
> 1441		kfree(lgp->lg_content);
> 1442	}
  1443	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51015 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] nfsd4: define ->op_release for compound ops
  2017-07-15  9:27   ` kbuild test robot
@ 2017-07-17 18:44     ` J. Bruce Fields
  0 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-17 18:44 UTC (permalink / raw)
  To: kbuild test robot; +Cc: J. Bruce Fields, kbuild-all, linux-nfs

On Sat, Jul 15, 2017 at 05:27:48PM +0800, kbuild test robot wrote:
> Hi Bruce,
> 
> [auto build test ERROR on next-20170710]
> [cannot apply to nfsd/nfsd-next v4.12 v4.12-rc7 v4.12-rc6 v4.12]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

These patches depend on patches that are in 13-rc1.

--b.

> 
> url:    https://github.com/0day-ci/linux/commits/J-Bruce-Fields/skip-op-encoders-in-error-cases/20170715-165807
> config: xtensa-allyesconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 4.9.0
> reproduce:
>         wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=xtensa 
> 
> All error/warnings (new ones prefixed by >>):
> 
>    fs/nfsd/nfs4proc.c: In function 'nfsd4_getdeviceinfo_release':
> >> fs/nfsd/nfs4proc.c:1356:8: error: 'gdev' undeclared (first use in this function)
>      kfree(gdev->gd_device);
>            ^
>    fs/nfsd/nfs4proc.c:1356:8: note: each undeclared identifier is reported only once for each function it appears in
>    fs/nfsd/nfs4proc.c: At top level:
> >> fs/nfsd/nfs4proc.c:1439:1: warning: return type defaults to 'int' [-Wreturn-type]
>     nfsd4_layoutget_release(union nfsd4_op_u *u)
>     ^
>    fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
> >> fs/nfsd/nfs4proc.c:1441:8: error: 'lgp' undeclared (first use in this function)
>      kfree(lgp->lg_content);
>            ^
>    fs/nfsd/nfs4proc.c: At top level:
> >> fs/nfsd/nfs4proc.c:2401:3: warning: initialization from incompatible pointer type
>       .op_release = nfsd4_layoutget_release,
>       ^
>    fs/nfsd/nfs4proc.c:2401:3: warning: (near initialization for 'nfsd4_ops[50].op_release')
>    fs/nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
> >> fs/nfsd/nfs4proc.c:1442:1: warning: control reaches end of non-void function [-Wreturn-type]
>     }
>     ^
> 
> vim +/gdev +1356 fs/nfsd/nfs4proc.c
> 
>   1352	
>   1353	static void
>   1354	nfsd4_getdeviceinfo_release(union nfsd4_op_u *u)
>   1355	{
> > 1356		kfree(gdev->gd_device);
>   1357	}
>   1358	
>   1359	static __be32
>   1360	nfsd4_layoutget(struct svc_rqst *rqstp,
>   1361			struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
>   1362	{
>   1363		struct nfsd4_layoutget *lgp = &u->layoutget;
>   1364		struct svc_fh *current_fh = &cstate->current_fh;
>   1365		const struct nfsd4_layout_ops *ops;
>   1366		struct nfs4_layout_stateid *ls;
>   1367		__be32 nfserr;
>   1368		int accmode;
>   1369	
>   1370		switch (lgp->lg_seg.iomode) {
>   1371		case IOMODE_READ:
>   1372			accmode = NFSD_MAY_READ;
>   1373			break;
>   1374		case IOMODE_RW:
>   1375			accmode = NFSD_MAY_READ | NFSD_MAY_WRITE;
>   1376			break;
>   1377		default:
>   1378			dprintk("%s: invalid iomode %d\n",
>   1379				__func__, lgp->lg_seg.iomode);
>   1380			nfserr = nfserr_badiomode;
>   1381			goto out;
>   1382		}
>   1383	
>   1384		nfserr = fh_verify(rqstp, current_fh, 0, accmode);
>   1385		if (nfserr)
>   1386			goto out;
>   1387	
>   1388		nfserr = nfserr_layoutunavailable;
>   1389		ops = nfsd4_layout_verify(current_fh->fh_export, lgp->lg_layout_type);
>   1390		if (!ops)
>   1391			goto out;
>   1392	
>   1393		/*
>   1394		 * Verify minlength and range as per RFC5661:
>   1395		 *  o  If loga_length is less than loga_minlength,
>   1396		 *     the metadata server MUST return NFS4ERR_INVAL.
>   1397		 *  o  If the sum of loga_offset and loga_minlength exceeds
>   1398		 *     NFS4_UINT64_MAX, and loga_minlength is not
>   1399		 *     NFS4_UINT64_MAX, the error NFS4ERR_INVAL MUST result.
>   1400		 *  o  If the sum of loga_offset and loga_length exceeds
>   1401		 *     NFS4_UINT64_MAX, and loga_length is not NFS4_UINT64_MAX,
>   1402		 *     the error NFS4ERR_INVAL MUST result.
>   1403		 */
>   1404		nfserr = nfserr_inval;
>   1405		if (lgp->lg_seg.length < lgp->lg_minlength ||
>   1406		    (lgp->lg_minlength != NFS4_MAX_UINT64 &&
>   1407		     lgp->lg_minlength > NFS4_MAX_UINT64 - lgp->lg_seg.offset) ||
>   1408		    (lgp->lg_seg.length != NFS4_MAX_UINT64 &&
>   1409		     lgp->lg_seg.length > NFS4_MAX_UINT64 - lgp->lg_seg.offset))
>   1410			goto out;
>   1411		if (lgp->lg_seg.length == 0)
>   1412			goto out;
>   1413	
>   1414		nfserr = nfsd4_preprocess_layout_stateid(rqstp, cstate, &lgp->lg_sid,
>   1415							true, lgp->lg_layout_type, &ls);
>   1416		if (nfserr) {
>   1417			trace_layout_get_lookup_fail(&lgp->lg_sid);
>   1418			goto out;
>   1419		}
>   1420	
>   1421		nfserr = nfserr_recallconflict;
>   1422		if (atomic_read(&ls->ls_stid.sc_file->fi_lo_recalls))
>   1423			goto out_put_stid;
>   1424	
>   1425		nfserr = ops->proc_layoutget(d_inode(current_fh->fh_dentry),
>   1426					     current_fh, lgp);
>   1427		if (nfserr)
>   1428			goto out_put_stid;
>   1429	
>   1430		nfserr = nfsd4_insert_layout(lgp, ls);
>   1431	
>   1432	out_put_stid:
>   1433		mutex_unlock(&ls->ls_mutex);
>   1434		nfs4_put_stid(&ls->ls_stid);
>   1435	out:
>   1436		return nfserr;
>   1437	}
>   1438	
> > 1439	nfsd4_layoutget_release(union nfsd4_op_u *u)
>   1440	{
> > 1441		kfree(lgp->lg_content);
> > 1442	}
>   1443	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/5] nfsd4: define ->op_release for compound ops
  2017-07-14 20:24 ` [PATCH 3/5] nfsd4: define ->op_release for compound ops J. Bruce Fields
  2017-07-15  9:27   ` kbuild test robot
@ 2017-07-15  9:30   ` kbuild test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2017-07-15  9:30 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: kbuild-all, linux-nfs, J. Bruce Fields

[-- Attachment #1: Type: text/plain, Size: 15285 bytes --]

Hi Bruce,

[auto build test ERROR on next-20170710]
[cannot apply to nfsd/nfsd-next v4.12 v4.12-rc7 v4.12-rc6 v4.12]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/J-Bruce-Fields/skip-op-encoders-in-error-cases/20170715-165807
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   fs//nfsd/nfs4proc.c: In function 'nfsd4_getdeviceinfo_release':
   fs//nfsd/nfs4proc.c:1356:8: error: 'gdev' undeclared (first use in this function)
     kfree(gdev->gd_device);
           ^~~~
   fs//nfsd/nfs4proc.c:1356:8: note: each undeclared identifier is reported only once for each function it appears in
   fs//nfsd/nfs4proc.c: At top level:
   fs//nfsd/nfs4proc.c:1439:1: warning: return type defaults to 'int' [-Wreturn-type]
    nfsd4_layoutget_release(union nfsd4_op_u *u)
    ^~~~~~~~~~~~~~~~~~~~~~~
   fs//nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
   fs//nfsd/nfs4proc.c:1441:8: error: 'lgp' undeclared (first use in this function)
     kfree(lgp->lg_content);
           ^~~
   fs//nfsd/nfs4proc.c: At top level:
>> fs//nfsd/nfs4proc.c:2401:17: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
      .op_release = nfsd4_layoutget_release,
                    ^~~~~~~~~~~~~~~~~~~~~~~
   fs//nfsd/nfs4proc.c:2401:17: note: (near initialization for 'nfsd4_ops[50].op_release')
   fs//nfsd/nfs4proc.c: In function 'nfsd4_layoutget_release':
   fs//nfsd/nfs4proc.c:1442:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^
   cc1: some warnings being treated as errors

vim +2401 fs//nfsd/nfs4proc.c

  2089	
  2090	static const struct nfsd4_operation nfsd4_ops[] = {
  2091		[OP_ACCESS] = {
  2092			.op_func = nfsd4_access,
  2093			.op_name = "OP_ACCESS",
  2094			.op_rsize_bop = nfsd4_access_rsize,
  2095		},
  2096		[OP_CLOSE] = {
  2097			.op_func = nfsd4_close,
  2098			.op_flags = OP_MODIFIES_SOMETHING,
  2099			.op_name = "OP_CLOSE",
  2100			.op_rsize_bop = nfsd4_status_stateid_rsize,
  2101			.op_get_currentstateid = nfsd4_get_closestateid,
  2102			.op_set_currentstateid = nfsd4_set_closestateid,
  2103		},
  2104		[OP_COMMIT] = {
  2105			.op_func = nfsd4_commit,
  2106			.op_flags = OP_MODIFIES_SOMETHING,
  2107			.op_name = "OP_COMMIT",
  2108			.op_rsize_bop = nfsd4_commit_rsize,
  2109		},
  2110		[OP_CREATE] = {
  2111			.op_func = nfsd4_create,
  2112			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME | OP_CLEAR_STATEID,
  2113			.op_name = "OP_CREATE",
  2114			.op_rsize_bop = nfsd4_create_rsize,
  2115		},
  2116		[OP_DELEGRETURN] = {
  2117			.op_func = nfsd4_delegreturn,
  2118			.op_flags = OP_MODIFIES_SOMETHING,
  2119			.op_name = "OP_DELEGRETURN",
  2120			.op_rsize_bop = nfsd4_only_status_rsize,
  2121			.op_get_currentstateid = nfsd4_get_delegreturnstateid,
  2122		},
  2123		[OP_GETATTR] = {
  2124			.op_func = nfsd4_getattr,
  2125			.op_flags = ALLOWED_ON_ABSENT_FS,
  2126			.op_rsize_bop = nfsd4_getattr_rsize,
  2127			.op_name = "OP_GETATTR",
  2128		},
  2129		[OP_GETFH] = {
  2130			.op_func = nfsd4_getfh,
  2131			.op_name = "OP_GETFH",
  2132			.op_rsize_bop = nfsd4_getfh_rsize,
  2133		},
  2134		[OP_LINK] = {
  2135			.op_func = nfsd4_link,
  2136			.op_flags = ALLOWED_ON_ABSENT_FS | OP_MODIFIES_SOMETHING
  2137					| OP_CACHEME,
  2138			.op_name = "OP_LINK",
  2139			.op_rsize_bop = nfsd4_link_rsize,
  2140		},
  2141		[OP_LOCK] = {
  2142			.op_func = nfsd4_lock,
  2143			.op_flags = OP_MODIFIES_SOMETHING,
  2144			.op_name = "OP_LOCK",
  2145			.op_rsize_bop = nfsd4_lock_rsize,
  2146			.op_set_currentstateid = nfsd4_set_lockstateid,
  2147		},
  2148		[OP_LOCKT] = {
  2149			.op_func = nfsd4_lockt,
  2150			.op_name = "OP_LOCKT",
  2151			.op_rsize_bop = nfsd4_lock_rsize,
  2152		},
  2153		[OP_LOCKU] = {
  2154			.op_func = nfsd4_locku,
  2155			.op_flags = OP_MODIFIES_SOMETHING,
  2156			.op_name = "OP_LOCKU",
  2157			.op_rsize_bop = nfsd4_status_stateid_rsize,
  2158			.op_get_currentstateid = nfsd4_get_lockustateid,
  2159		},
  2160		[OP_LOOKUP] = {
  2161			.op_func = nfsd4_lookup,
  2162			.op_flags = OP_HANDLES_WRONGSEC | OP_CLEAR_STATEID,
  2163			.op_name = "OP_LOOKUP",
  2164			.op_rsize_bop = nfsd4_only_status_rsize,
  2165		},
  2166		[OP_LOOKUPP] = {
  2167			.op_func = nfsd4_lookupp,
  2168			.op_flags = OP_HANDLES_WRONGSEC | OP_CLEAR_STATEID,
  2169			.op_name = "OP_LOOKUPP",
  2170			.op_rsize_bop = nfsd4_only_status_rsize,
  2171		},
  2172		[OP_NVERIFY] = {
  2173			.op_func = nfsd4_nverify,
  2174			.op_name = "OP_NVERIFY",
  2175			.op_rsize_bop = nfsd4_only_status_rsize,
  2176		},
  2177		[OP_OPEN] = {
  2178			.op_func = nfsd4_open,
  2179			.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
  2180			.op_name = "OP_OPEN",
  2181			.op_rsize_bop = nfsd4_open_rsize,
  2182			.op_set_currentstateid = nfsd4_set_openstateid,
  2183		},
  2184		[OP_OPEN_CONFIRM] = {
  2185			.op_func = nfsd4_open_confirm,
  2186			.op_flags = OP_MODIFIES_SOMETHING,
  2187			.op_name = "OP_OPEN_CONFIRM",
  2188			.op_rsize_bop = nfsd4_status_stateid_rsize,
  2189		},
  2190		[OP_OPEN_DOWNGRADE] = {
  2191			.op_func = nfsd4_open_downgrade,
  2192			.op_flags = OP_MODIFIES_SOMETHING,
  2193			.op_name = "OP_OPEN_DOWNGRADE",
  2194			.op_rsize_bop = nfsd4_status_stateid_rsize,
  2195			.op_get_currentstateid = nfsd4_get_opendowngradestateid,
  2196			.op_set_currentstateid = nfsd4_set_opendowngradestateid,
  2197		},
  2198		[OP_PUTFH] = {
  2199			.op_func = nfsd4_putfh,
  2200			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2201					| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
  2202			.op_name = "OP_PUTFH",
  2203			.op_rsize_bop = nfsd4_only_status_rsize,
  2204		},
  2205		[OP_PUTPUBFH] = {
  2206			.op_func = nfsd4_putrootfh,
  2207			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2208					| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
  2209			.op_name = "OP_PUTPUBFH",
  2210			.op_rsize_bop = nfsd4_only_status_rsize,
  2211		},
  2212		[OP_PUTROOTFH] = {
  2213			.op_func = nfsd4_putrootfh,
  2214			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2215					| OP_IS_PUTFH_LIKE | OP_CLEAR_STATEID,
  2216			.op_name = "OP_PUTROOTFH",
  2217			.op_rsize_bop = nfsd4_only_status_rsize,
  2218		},
  2219		[OP_READ] = {
  2220			.op_func = nfsd4_read,
  2221			.op_release = nfsd4_read_release,
  2222			.op_name = "OP_READ",
  2223			.op_rsize_bop = nfsd4_read_rsize,
  2224			.op_get_currentstateid = nfsd4_get_readstateid,
  2225		},
  2226		[OP_READDIR] = {
  2227			.op_func = nfsd4_readdir,
  2228			.op_name = "OP_READDIR",
  2229			.op_rsize_bop = nfsd4_readdir_rsize,
  2230		},
  2231		[OP_READLINK] = {
  2232			.op_func = nfsd4_readlink,
  2233			.op_name = "OP_READLINK",
  2234			.op_rsize_bop = nfsd4_readlink_rsize,
  2235		},
  2236		[OP_REMOVE] = {
  2237			.op_func = nfsd4_remove,
  2238			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2239			.op_name = "OP_REMOVE",
  2240			.op_rsize_bop = nfsd4_remove_rsize,
  2241		},
  2242		[OP_RENAME] = {
  2243			.op_func = nfsd4_rename,
  2244			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2245			.op_name = "OP_RENAME",
  2246			.op_rsize_bop = nfsd4_rename_rsize,
  2247		},
  2248		[OP_RENEW] = {
  2249			.op_func = nfsd4_renew,
  2250			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2251					| OP_MODIFIES_SOMETHING,
  2252			.op_name = "OP_RENEW",
  2253			.op_rsize_bop = nfsd4_only_status_rsize,
  2254	
  2255		},
  2256		[OP_RESTOREFH] = {
  2257			.op_func = nfsd4_restorefh,
  2258			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2259					| OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING,
  2260			.op_name = "OP_RESTOREFH",
  2261			.op_rsize_bop = nfsd4_only_status_rsize,
  2262		},
  2263		[OP_SAVEFH] = {
  2264			.op_func = nfsd4_savefh,
  2265			.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
  2266			.op_name = "OP_SAVEFH",
  2267			.op_rsize_bop = nfsd4_only_status_rsize,
  2268		},
  2269		[OP_SECINFO] = {
  2270			.op_func = nfsd4_secinfo,
  2271			.op_release = nfsd4_secinfo_release,
  2272			.op_flags = OP_HANDLES_WRONGSEC,
  2273			.op_name = "OP_SECINFO",
  2274			.op_rsize_bop = nfsd4_secinfo_rsize,
  2275		},
  2276		[OP_SETATTR] = {
  2277			.op_func = nfsd4_setattr,
  2278			.op_name = "OP_SETATTR",
  2279			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2280			.op_rsize_bop = nfsd4_setattr_rsize,
  2281			.op_get_currentstateid = nfsd4_get_setattrstateid,
  2282		},
  2283		[OP_SETCLIENTID] = {
  2284			.op_func = nfsd4_setclientid,
  2285			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2286					| OP_MODIFIES_SOMETHING | OP_CACHEME,
  2287			.op_name = "OP_SETCLIENTID",
  2288			.op_rsize_bop = nfsd4_setclientid_rsize,
  2289		},
  2290		[OP_SETCLIENTID_CONFIRM] = {
  2291			.op_func = nfsd4_setclientid_confirm,
  2292			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2293					| OP_MODIFIES_SOMETHING | OP_CACHEME,
  2294			.op_name = "OP_SETCLIENTID_CONFIRM",
  2295			.op_rsize_bop = nfsd4_only_status_rsize,
  2296		},
  2297		[OP_VERIFY] = {
  2298			.op_func = nfsd4_verify,
  2299			.op_name = "OP_VERIFY",
  2300			.op_rsize_bop = nfsd4_only_status_rsize,
  2301		},
  2302		[OP_WRITE] = {
  2303			.op_func = nfsd4_write,
  2304			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2305			.op_name = "OP_WRITE",
  2306			.op_rsize_bop = nfsd4_write_rsize,
  2307			.op_get_currentstateid = nfsd4_get_writestateid,
  2308		},
  2309		[OP_RELEASE_LOCKOWNER] = {
  2310			.op_func = nfsd4_release_lockowner,
  2311			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
  2312					| OP_MODIFIES_SOMETHING,
  2313			.op_name = "OP_RELEASE_LOCKOWNER",
  2314			.op_rsize_bop = nfsd4_only_status_rsize,
  2315		},
  2316	
  2317		/* NFSv4.1 operations */
  2318		[OP_EXCHANGE_ID] = {
  2319			.op_func = nfsd4_exchange_id,
  2320			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
  2321					| OP_MODIFIES_SOMETHING,
  2322			.op_name = "OP_EXCHANGE_ID",
  2323			.op_rsize_bop = nfsd4_exchange_id_rsize,
  2324		},
  2325		[OP_BACKCHANNEL_CTL] = {
  2326			.op_func = nfsd4_backchannel_ctl,
  2327			.op_flags = ALLOWED_WITHOUT_FH | OP_MODIFIES_SOMETHING,
  2328			.op_name = "OP_BACKCHANNEL_CTL",
  2329			.op_rsize_bop = nfsd4_only_status_rsize,
  2330		},
  2331		[OP_BIND_CONN_TO_SESSION] = {
  2332			.op_func = nfsd4_bind_conn_to_session,
  2333			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
  2334					| OP_MODIFIES_SOMETHING,
  2335			.op_name = "OP_BIND_CONN_TO_SESSION",
  2336			.op_rsize_bop = nfsd4_bind_conn_to_session_rsize,
  2337		},
  2338		[OP_CREATE_SESSION] = {
  2339			.op_func = nfsd4_create_session,
  2340			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
  2341					| OP_MODIFIES_SOMETHING,
  2342			.op_name = "OP_CREATE_SESSION",
  2343			.op_rsize_bop = nfsd4_create_session_rsize,
  2344		},
  2345		[OP_DESTROY_SESSION] = {
  2346			.op_func = nfsd4_destroy_session,
  2347			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
  2348					| OP_MODIFIES_SOMETHING,
  2349			.op_name = "OP_DESTROY_SESSION",
  2350			.op_rsize_bop = nfsd4_only_status_rsize,
  2351		},
  2352		[OP_SEQUENCE] = {
  2353			.op_func = nfsd4_sequence,
  2354			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP,
  2355			.op_name = "OP_SEQUENCE",
  2356			.op_rsize_bop = nfsd4_sequence_rsize,
  2357		},
  2358		[OP_DESTROY_CLIENTID] = {
  2359			.op_func = nfsd4_destroy_clientid,
  2360			.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_AS_FIRST_OP
  2361					| OP_MODIFIES_SOMETHING,
  2362			.op_name = "OP_DESTROY_CLIENTID",
  2363			.op_rsize_bop = nfsd4_only_status_rsize,
  2364		},
  2365		[OP_RECLAIM_COMPLETE] = {
  2366			.op_func = nfsd4_reclaim_complete,
  2367			.op_flags = ALLOWED_WITHOUT_FH | OP_MODIFIES_SOMETHING,
  2368			.op_name = "OP_RECLAIM_COMPLETE",
  2369			.op_rsize_bop = nfsd4_only_status_rsize,
  2370		},
  2371		[OP_SECINFO_NO_NAME] = {
  2372			.op_func = nfsd4_secinfo_no_name,
  2373			.op_release = nfsd4_secinfo_release,
  2374			.op_flags = OP_HANDLES_WRONGSEC,
  2375			.op_name = "OP_SECINFO_NO_NAME",
  2376			.op_rsize_bop = nfsd4_secinfo_rsize,
  2377		},
  2378		[OP_TEST_STATEID] = {
  2379			.op_func = nfsd4_test_stateid,
  2380			.op_flags = ALLOWED_WITHOUT_FH,
  2381			.op_name = "OP_TEST_STATEID",
  2382			.op_rsize_bop = nfsd4_test_stateid_rsize,
  2383		},
  2384		[OP_FREE_STATEID] = {
  2385			.op_func = nfsd4_free_stateid,
  2386			.op_flags = ALLOWED_WITHOUT_FH | OP_MODIFIES_SOMETHING,
  2387			.op_name = "OP_FREE_STATEID",
  2388			.op_get_currentstateid = nfsd4_get_freestateid,
  2389			.op_rsize_bop = nfsd4_only_status_rsize,
  2390		},
  2391	#ifdef CONFIG_NFSD_PNFS
  2392		[OP_GETDEVICEINFO] = {
  2393			.op_func = nfsd4_getdeviceinfo,
  2394			.op_release = nfsd4_getdeviceinfo_release,
  2395			.op_flags = ALLOWED_WITHOUT_FH,
  2396			.op_name = "OP_GETDEVICEINFO",
  2397			.op_rsize_bop = nfsd4_getdeviceinfo_rsize,
  2398		},
  2399		[OP_LAYOUTGET] = {
  2400			.op_func = nfsd4_layoutget,
> 2401			.op_release = nfsd4_layoutget_release,
  2402			.op_flags = OP_MODIFIES_SOMETHING,
  2403			.op_name = "OP_LAYOUTGET",
  2404			.op_rsize_bop = nfsd4_layoutget_rsize,
  2405		},
  2406		[OP_LAYOUTCOMMIT] = {
  2407			.op_func = nfsd4_layoutcommit,
  2408			.op_flags = OP_MODIFIES_SOMETHING,
  2409			.op_name = "OP_LAYOUTCOMMIT",
  2410			.op_rsize_bop = nfsd4_layoutcommit_rsize,
  2411		},
  2412		[OP_LAYOUTRETURN] = {
  2413			.op_func = nfsd4_layoutreturn,
  2414			.op_flags = OP_MODIFIES_SOMETHING,
  2415			.op_name = "OP_LAYOUTRETURN",
  2416			.op_rsize_bop = nfsd4_layoutreturn_rsize,
  2417		},
  2418	#endif /* CONFIG_NFSD_PNFS */
  2419	
  2420		/* NFSv4.2 operations */
  2421		[OP_ALLOCATE] = {
  2422			.op_func = nfsd4_allocate,
  2423			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2424			.op_name = "OP_ALLOCATE",
  2425			.op_rsize_bop = nfsd4_only_status_rsize,
  2426		},
  2427		[OP_DEALLOCATE] = {
  2428			.op_func = nfsd4_deallocate,
  2429			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2430			.op_name = "OP_DEALLOCATE",
  2431			.op_rsize_bop = nfsd4_only_status_rsize,
  2432		},
  2433		[OP_CLONE] = {
  2434			.op_func = nfsd4_clone,
  2435			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2436			.op_name = "OP_CLONE",
  2437			.op_rsize_bop = nfsd4_only_status_rsize,
  2438		},
  2439		[OP_COPY] = {
  2440			.op_func = nfsd4_copy,
  2441			.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
  2442			.op_name = "OP_COPY",
  2443			.op_rsize_bop = nfsd4_copy_rsize,
  2444		},
  2445		[OP_SEEK] = {
  2446			.op_func = nfsd4_seek,
  2447			.op_name = "OP_SEEK",
  2448			.op_rsize_bop = nfsd4_seek_rsize,
  2449		},
  2450	};
  2451	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51461 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 4/5] nfsd4: skip encoder in trivial error cases
  2017-07-14 20:24 [PATCH 0/5] skip op encoders in error cases J. Bruce Fields
                   ` (2 preceding siblings ...)
  2017-07-14 20:24 ` [PATCH 3/5] nfsd4: define ->op_release for compound ops J. Bruce Fields
@ 2017-07-14 20:24 ` J. Bruce Fields
  2017-07-14 20:24 ` [PATCH 5/5] nfsd4: individual encoders no longer see " J. Bruce Fields
  4 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-14 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

Most encoders do nothing in the error case.  But they can still screw
things up in that case: most errors happen very early in rpc processing,
possibly before argument fields are filled in and bounds-tested, so
encoders that do anything other than immediately bail on error can
easily crash in odd error cases.

So just handle errors centrally most of the time to remove the chance of
error.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4proc.c | 10 +++++++---
 fs/nfsd/nfs4xdr.c  |  3 +++
 fs/nfsd/xdr4.h     |  2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 8944b00fd3cd..f37598240700 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2140,13 +2140,15 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	},
 	[OP_LOCK] = {
 		.op_func = nfsd4_lock,
-		.op_flags = OP_MODIFIES_SOMETHING,
+		.op_flags = OP_MODIFIES_SOMETHING |
+				OP_NONTRIVIAL_ERROR_ENCODE,
 		.op_name = "OP_LOCK",
 		.op_rsize_bop = nfsd4_lock_rsize,
 		.op_set_currentstateid = nfsd4_set_lockstateid,
 	},
 	[OP_LOCKT] = {
 		.op_func = nfsd4_lockt,
+		.op_flags = OP_NONTRIVIAL_ERROR_ENCODE,
 		.op_name = "OP_LOCKT",
 		.op_rsize_bop = nfsd4_lock_rsize,
 	},
@@ -2276,14 +2278,16 @@ static const struct nfsd4_operation nfsd4_ops[] = {
 	[OP_SETATTR] = {
 		.op_func = nfsd4_setattr,
 		.op_name = "OP_SETATTR",
-		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+		.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME
+				| OP_NONTRIVIAL_ERROR_ENCODE,
 		.op_rsize_bop = nfsd4_setattr_rsize,
 		.op_get_currentstateid = nfsd4_get_setattrstateid,
 	},
 	[OP_SETCLIENTID] = {
 		.op_func = nfsd4_setclientid,
 		.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
-				| OP_MODIFIES_SOMETHING | OP_CACHEME,
+				| OP_MODIFIES_SOMETHING | OP_CACHEME
+				| OP_NONTRIVIAL_ERROR_ENCODE,
 		.op_name = "OP_SETCLIENTID",
 		.op_rsize_bop = nfsd4_setclientid_rsize,
 	},
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 7d683e3aebf0..08badcf64554 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4461,6 +4461,9 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
 
 	if (op->opnum == OP_ILLEGAL)
 		goto status;
+	if (op->status && opdesc &&
+			!(opdesc->op_flags & OP_NONTRIVIAL_ERROR_ENCODE))
+		goto status;
 	BUG_ON(op->opnum < 0 || op->opnum >= ARRAY_SIZE(nfsd4_enc_ops) ||
 	       !nfsd4_enc_ops[op->opnum]);
 	encoder = nfsd4_enc_ops[op->opnum];
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 1e6274e0e066..f8a0b6549a88 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -778,6 +778,8 @@ enum nfsd4_op_flags {
 	 * These are ops which clear current state id.
 	 */
 	OP_CLEAR_STATEID = 1 << 7,
+	/* Most ops return only an error on failure; some may do more: */
+	OP_NONTRIVIAL_ERROR_ENCODE = 1 << 8,
 };
 
 struct nfsd4_operation {
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] nfsd4: individual encoders no longer see error cases
  2017-07-14 20:24 [PATCH 0/5] skip op encoders in error cases J. Bruce Fields
                   ` (3 preceding siblings ...)
  2017-07-14 20:24 ` [PATCH 4/5] nfsd4: skip encoder in trivial error cases J. Bruce Fields
@ 2017-07-14 20:24 ` J. Bruce Fields
  4 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2017-07-14 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: J. Bruce Fields

From: "J. Bruce Fields" <bfields@redhat.com>

With a few exceptions, most individual encoders don't handle error
cases.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4xdr.c | 309 ++++++++++++++++++------------------------------------
 1 file changed, 103 insertions(+), 206 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 08badcf64554..18b9c2fad29a 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3105,14 +3105,12 @@ nfsd4_encode_access(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 8);
-		if (!p)
-			return nfserr_resource;
-		*p++ = cpu_to_be32(access->ac_supported);
-		*p++ = cpu_to_be32(access->ac_resp_access);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, 8);
+	if (!p)
+		return nfserr_resource;
+	*p++ = cpu_to_be32(access->ac_supported);
+	*p++ = cpu_to_be32(access->ac_resp_access);
+	return 0;
 }
 
 static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_bind_conn_to_session *bcts)
@@ -3120,17 +3118,15 @@ static __be32 nfsd4_encode_bind_conn_to_session(struct nfsd4_compoundres *resp,
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, NFS4_MAX_SESSIONID_LEN + 8);
-		if (!p)
-			return nfserr_resource;
-		p = xdr_encode_opaque_fixed(p, bcts->sessionid.data,
-						NFS4_MAX_SESSIONID_LEN);
-		*p++ = cpu_to_be32(bcts->dir);
-		/* Upshifting from TCP to RDMA is not supported */
-		*p++ = cpu_to_be32(0);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, NFS4_MAX_SESSIONID_LEN + 8);
+	if (!p)
+		return nfserr_resource;
+	p = xdr_encode_opaque_fixed(p, bcts->sessionid.data,
+					NFS4_MAX_SESSIONID_LEN);
+	*p++ = cpu_to_be32(bcts->dir);
+	/* Upshifting from TCP to RDMA is not supported */
+	*p++ = cpu_to_be32(0);
+	return 0;
 }
 
 static __be32
@@ -3138,10 +3134,7 @@ nfsd4_encode_close(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_c
 {
 	struct xdr_stream *xdr = &resp->xdr;
 
-	if (!nfserr)
-		nfserr = nfsd4_encode_stateid(xdr, &close->cl_stateid);
-
-	return nfserr;
+	return nfsd4_encode_stateid(xdr, &close->cl_stateid);
 }
 
 
@@ -3151,14 +3144,12 @@ nfsd4_encode_commit(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, NFS4_VERIFIER_SIZE);
-		if (!p)
-			return nfserr_resource;
-		p = xdr_encode_opaque_fixed(p, commit->co_verf.data,
+	p = xdr_reserve_space(xdr, NFS4_VERIFIER_SIZE);
+	if (!p)
+		return nfserr_resource;
+	p = xdr_encode_opaque_fixed(p, commit->co_verf.data,
 						NFS4_VERIFIER_SIZE);
-	}
-	return nfserr;
+	return 0;
 }
 
 static __be32
@@ -3167,15 +3158,13 @@ nfsd4_encode_create(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 20);
-		if (!p)
-			return nfserr_resource;
-		encode_cinfo(p, &create->cr_cinfo);
-		nfserr = nfsd4_encode_bitmap(xdr, create->cr_bmval[0],
-				create->cr_bmval[1], create->cr_bmval[2]);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+	encode_cinfo(p, &create->cr_cinfo);
+	nfserr = nfsd4_encode_bitmap(xdr, create->cr_bmval[0],
+			create->cr_bmval[1], create->cr_bmval[2]);
+	return 0;
 }
 
 static __be32
@@ -3184,13 +3173,8 @@ nfsd4_encode_getattr(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
 	struct svc_fh *fhp = getattr->ga_fhp;
 	struct xdr_stream *xdr = &resp->xdr;
 
-	if (nfserr)
-		return nfserr;
-
-	nfserr = nfsd4_encode_fattr(xdr, fhp, fhp->fh_export, fhp->fh_dentry,
-				    getattr->ga_bmval,
-				    resp->rqstp, 0);
-	return nfserr;
+	return nfsd4_encode_fattr(xdr, fhp, fhp->fh_export, fhp->fh_dentry,
+				    getattr->ga_bmval, resp->rqstp, 0);
 }
 
 static __be32
@@ -3201,14 +3185,12 @@ nfsd4_encode_getfh(struct nfsd4_compoundres *resp, __be32 nfserr, struct svc_fh
 	unsigned int len;
 	__be32 *p;
 
-	if (!nfserr) {
-		len = fhp->fh_handle.fh_size;
-		p = xdr_reserve_space(xdr, len + 4);
-		if (!p)
-			return nfserr_resource;
-		p = xdr_encode_opaque(p, &fhp->fh_handle.fh_base, len);
-	}
-	return nfserr;
+	len = fhp->fh_handle.fh_size;
+	p = xdr_reserve_space(xdr, len + 4);
+	if (!p)
+		return nfserr_resource;
+	p = xdr_encode_opaque(p, &fhp->fh_handle.fh_base, len);
+	return 0;
 }
 
 /*
@@ -3278,10 +3260,7 @@ nfsd4_encode_locku(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_l
 {
 	struct xdr_stream *xdr = &resp->xdr;
 
-	if (!nfserr)
-		nfserr = nfsd4_encode_stateid(xdr, &locku->lu_stateid);
-
-	return nfserr;
+	return nfsd4_encode_stateid(xdr, &locku->lu_stateid);
 }
 
 
@@ -3291,13 +3270,11 @@ nfsd4_encode_link(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_li
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 20);
-		if (!p)
-			return nfserr_resource;
-		p = encode_cinfo(p, &link->li_cinfo);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+	p = encode_cinfo(p, &link->li_cinfo);
+	return 0;
 }
 
 
@@ -3307,12 +3284,9 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_op
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (nfserr)
-		goto out;
-
 	nfserr = nfsd4_encode_stateid(xdr, &open->op_stateid);
 	if (nfserr)
-		goto out;
+		return nfserr;
 	p = xdr_reserve_space(xdr, 24);
 	if (!p)
 		return nfserr_resource;
@@ -3322,7 +3296,7 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_op
 	nfserr = nfsd4_encode_bitmap(xdr, open->op_bmval[0], open->op_bmval[1],
 					open->op_bmval[2]);
 	if (nfserr)
-		goto out;
+		return nfserr;
 
 	p = xdr_reserve_space(xdr, 4);
 	if (!p)
@@ -3395,8 +3369,7 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_op
 		BUG();
 	}
 	/* XXX save filehandle here */
-out:
-	return nfserr;
+	return 0;
 }
 
 static __be32
@@ -3404,10 +3377,7 @@ nfsd4_encode_open_confirm(struct nfsd4_compoundres *resp, __be32 nfserr, struct
 {
 	struct xdr_stream *xdr = &resp->xdr;
 
-	if (!nfserr)
-		nfserr = nfsd4_encode_stateid(xdr, &oc->oc_resp_stateid);
-
-	return nfserr;
+	return nfsd4_encode_stateid(xdr, &oc->oc_resp_stateid);
 }
 
 static __be32
@@ -3415,10 +3385,7 @@ nfsd4_encode_open_downgrade(struct nfsd4_compoundres *resp, __be32 nfserr, struc
 {
 	struct xdr_stream *xdr = &resp->xdr;
 
-	if (!nfserr)
-		nfserr = nfsd4_encode_stateid(xdr, &od->od_stateid);
-
-	return nfserr;
+	return nfsd4_encode_stateid(xdr, &od->od_stateid);
 }
 
 static __be32 nfsd4_encode_splice_read(
@@ -3555,20 +3522,15 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct raparms *ra = NULL;
 	__be32 *p;
 
-	if (nfserr)
-		goto out;
-
 	p = xdr_reserve_space(xdr, 8); /* eof flag and byte count */
 	if (!p) {
 		WARN_ON_ONCE(test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags));
-		nfserr = nfserr_resource;
-		goto out;
+		return nfserr_resource;
 	}
 	if (resp->xdr.buf->page_len &&
 	    test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags)) {
 		WARN_ON_ONCE(1);
-		nfserr = nfserr_resource;
-		goto out;
+		return nfserr_resource;
 	}
 	xdr_commit_encode(xdr);
 
@@ -3592,7 +3554,6 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr,
 	if (nfserr)
 		xdr_truncate_encode(xdr, starting_len);
 
-out:
 	return nfserr;
 }
 
@@ -3606,9 +3567,6 @@ nfsd4_encode_readlink(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd
 	int length_offset = xdr->buf->len;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, 4);
 	if (!p)
 		return nfserr_resource;
@@ -3652,9 +3610,6 @@ nfsd4_encode_readdir(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4
 	int starting_len = xdr->buf->len;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, NFS4_VERIFIER_SIZE);
 	if (!p)
 		return nfserr_resource;
@@ -3740,13 +3695,11 @@ nfsd4_encode_remove(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 20);
-		if (!p)
-			return nfserr_resource;
-		p = encode_cinfo(p, &remove->rm_cinfo);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, 20);
+	if (!p)
+		return nfserr_resource;
+	p = encode_cinfo(p, &remove->rm_cinfo);
+	return 0;
 }
 
 static __be32
@@ -3755,19 +3708,16 @@ nfsd4_encode_rename(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 40);
-		if (!p)
-			return nfserr_resource;
-		p = encode_cinfo(p, &rename->rn_sinfo);
-		p = encode_cinfo(p, &rename->rn_tinfo);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, 40);
+	if (!p)
+		return nfserr_resource;
+	p = encode_cinfo(p, &rename->rn_sinfo);
+	p = encode_cinfo(p, &rename->rn_tinfo);
+	return 0;
 }
 
 static __be32
-nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
-			 __be32 nfserr, struct svc_export *exp)
+nfsd4_do_encode_secinfo(struct xdr_stream *xdr, struct svc_export *exp)
 {
 	u32 i, nflavs, supported;
 	struct exp_flavor_info *flavs;
@@ -3775,9 +3725,6 @@ nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
 	__be32 *p, *flavorsp;
 	static bool report = true;
 
-	if (nfserr)
-		goto out;
-	nfserr = nfserr_resource;
 	if (exp->ex_nflavors) {
 		flavs = exp->ex_flavors;
 		nflavs = exp->ex_nflavors;
@@ -3801,7 +3748,7 @@ nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
 	supported = 0;
 	p = xdr_reserve_space(xdr, 4);
 	if (!p)
-		goto out;
+		return nfserr_resource;
 	flavorsp = p++;		/* to be backfilled later */
 
 	for (i = 0; i < nflavs; i++) {
@@ -3813,7 +3760,7 @@ nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
 			p = xdr_reserve_space(xdr, 4 + 4 +
 					      XDR_LEN(info.oid.len) + 4 + 4);
 			if (!p)
-				goto out;
+				return nfserr_resource;
 			*p++ = cpu_to_be32(RPC_AUTH_GSS);
 			p = xdr_encode_opaque(p,  info.oid.data, info.oid.len);
 			*p++ = cpu_to_be32(info.qop);
@@ -3822,7 +3769,7 @@ nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
 			supported++;
 			p = xdr_reserve_space(xdr, 4);
 			if (!p)
-				goto out;
+				return nfserr_resource;
 			*p++ = cpu_to_be32(pf);
 		} else {
 			if (report)
@@ -3834,9 +3781,7 @@ nfsd4_do_encode_secinfo(struct xdr_stream *xdr,
 	if (nflavs != supported)
 		report = false;
 	*flavorsp = htonl(supported);
-	nfserr = 0;
-out:
-	return nfserr;
+	return 0;
 }
 
 static __be32
@@ -3845,7 +3790,7 @@ nfsd4_encode_secinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
 {
 	struct xdr_stream *xdr = &resp->xdr;
 
-	return nfsd4_do_encode_secinfo(xdr, nfserr, secinfo->si_exp);
+	return nfsd4_do_encode_secinfo(xdr, secinfo->si_exp);
 }
 
 static __be32
@@ -3854,7 +3799,7 @@ nfsd4_encode_secinfo_no_name(struct nfsd4_compoundres *resp, __be32 nfserr,
 {
 	struct xdr_stream *xdr = &resp->xdr;
 
-	return nfsd4_do_encode_secinfo(xdr, nfserr, secinfo->sin_exp);
+	return nfsd4_do_encode_secinfo(xdr, secinfo->sin_exp);
 }
 
 /*
@@ -3915,16 +3860,14 @@ nfsd4_encode_write(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_w
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 16);
-		if (!p)
-			return nfserr_resource;
-		*p++ = cpu_to_be32(write->wr_bytes_written);
-		*p++ = cpu_to_be32(write->wr_how_written);
-		p = xdr_encode_opaque_fixed(p, write->wr_verifier.data,
-							NFS4_VERIFIER_SIZE);
-	}
-	return nfserr;
+	p = xdr_reserve_space(xdr, 16);
+	if (!p)
+		return nfserr_resource;
+	*p++ = cpu_to_be32(write->wr_bytes_written);
+	*p++ = cpu_to_be32(write->wr_how_written);
+	p = xdr_encode_opaque_fixed(p, write->wr_verifier.data,
+						NFS4_VERIFIER_SIZE);
+	return 0;
 }
 
 static __be32
@@ -3937,12 +3880,8 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 	char *server_scope;
 	int major_id_sz;
 	int server_scope_sz;
-	int status = 0;
 	uint64_t minor_id = 0;
 
-	if (nfserr)
-		return nfserr;
-
 	major_id = utsname()->nodename;
 	major_id_sz = strlen(major_id);
 	server_scope = utsname()->nodename;
@@ -3967,19 +3906,19 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 		break;
 	case SP4_MACH_CRED:
 		/* spo_must_enforce bitmap: */
-		status = nfsd4_encode_bitmap(xdr,
+		nfserr = nfsd4_encode_bitmap(xdr,
 					exid->spo_must_enforce[0],
 					exid->spo_must_enforce[1],
 					exid->spo_must_enforce[2]);
-		if (status)
-			goto out;
+		if (nfserr)
+			return nfserr;
 		/* spo_must_allow bitmap: */
-		status = nfsd4_encode_bitmap(xdr,
+		nfserr = nfsd4_encode_bitmap(xdr,
 					exid->spo_must_allow[0],
 					exid->spo_must_allow[1],
 					exid->spo_must_allow[2]);
-		if (status)
-			goto out;
+		if (nfserr)
+			return nfserr;
 		break;
 	default:
 		WARN_ON_ONCE(1);
@@ -4006,8 +3945,6 @@ nfsd4_encode_exchange_id(struct nfsd4_compoundres *resp, __be32 nfserr,
 	/* Implementation id */
 	*p++ = cpu_to_be32(0);	/* zero length nfs_impl_id4 array */
 	return 0;
-out:
-	return status;
 }
 
 static __be32
@@ -4017,9 +3954,6 @@ nfsd4_encode_create_session(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, 24);
 	if (!p)
 		return nfserr_resource;
@@ -4073,9 +4007,6 @@ nfsd4_encode_sequence(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, NFS4_MAX_SESSIONID_LEN + 20);
 	if (!p)
 		return nfserr_resource;
@@ -4100,9 +4031,6 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct nfsd4_test_stateid_id *stateid, *next;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, 4 + (4 * test_stateid->ts_num_ids));
 	if (!p)
 		return nfserr_resource;
@@ -4112,7 +4040,7 @@ nfsd4_encode_test_stateid(struct nfsd4_compoundres *resp, __be32 nfserr,
 		*p++ = stateid->ts_id_status;
 	}
 
-	return nfserr;
+	return 0;
 }
 
 #ifdef CONFIG_NFSD_PNFS
@@ -4125,14 +4053,9 @@ nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
 	u32 starting_len = xdr->buf->len, needed_len;
 	__be32 *p;
 
-	dprintk("%s: err %d\n", __func__, be32_to_cpu(nfserr));
-	if (nfserr)
-		goto out;
-
-	nfserr = nfserr_resource;
 	p = xdr_reserve_space(xdr, 4);
 	if (!p)
-		goto out;
+		return nfserr_resource;
 
 	*p++ = cpu_to_be32(gdev->gd_layout_type);
 
@@ -4148,41 +4071,33 @@ nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
 			 */
 			if (xdr->buf->len + 4 > gdev->gd_maxcount)
 				goto toosmall;
-			goto out;
+			return nfserr;
 		}
 	}
 
-	nfserr = nfserr_resource;
 	if (gdev->gd_notify_types) {
 		p = xdr_reserve_space(xdr, 4 + 4);
 		if (!p)
-			goto out;
+			return nfserr_resource;
 		*p++ = cpu_to_be32(1);			/* bitmap length */
 		*p++ = cpu_to_be32(gdev->gd_notify_types);
 	} else {
 		p = xdr_reserve_space(xdr, 4);
 		if (!p)
-			goto out;
+			return nfserr_resource;
 		*p++ = 0;
 	}
 
-	nfserr = 0;
-out:
-	dprintk("%s: done: %d\n", __func__, be32_to_cpu(nfserr));
-	return nfserr;
-
+	return 0;
 toosmall:
 	dprintk("%s: maxcount too small\n", __func__);
 	needed_len = xdr->buf->len + 4 /* notifications */;
 	xdr_truncate_encode(xdr, starting_len);
 	p = xdr_reserve_space(xdr, 4);
-	if (!p) {
-		nfserr = nfserr_resource;
-	} else {
-		*p++ = cpu_to_be32(needed_len);
-		nfserr = nfserr_toosmall;
-	}
-	goto out;
+	if (!p)
+		return nfserr_resource;
+	*p++ = cpu_to_be32(needed_len);
+	return nfserr_toosmall;
 }
 
 static __be32
@@ -4193,14 +4108,9 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr,
 	const struct nfsd4_layout_ops *ops;
 	__be32 *p;
 
-	dprintk("%s: err %d\n", __func__, nfserr);
-	if (nfserr)
-		goto out;
-
-	nfserr = nfserr_resource;
 	p = xdr_reserve_space(xdr, 36 + sizeof(stateid_opaque_t));
 	if (!p)
-		goto out;
+		return nfserr_resource;
 
 	*p++ = cpu_to_be32(1);	/* we always set return-on-close */
 	*p++ = cpu_to_be32(lgp->lg_sid.si_generation);
@@ -4214,9 +4124,7 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr,
 	*p++ = cpu_to_be32(lgp->lg_layout_type);
 
 	ops = nfsd4_layout_ops[lgp->lg_layout_type];
-	nfserr = ops->encode_layoutget(xdr, lgp);
-out:
-	return nfserr;
+	return ops->encode_layoutget(xdr, lgp);
 }
 
 static __be32
@@ -4226,9 +4134,6 @@ nfsd4_encode_layoutcommit(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, 4);
 	if (!p)
 		return nfserr_resource;
@@ -4240,7 +4145,7 @@ nfsd4_encode_layoutcommit(struct nfsd4_compoundres *resp, __be32 nfserr,
 		p = xdr_encode_hyper(p, lcp->lc_newsize);
 	}
 
-	return nfs_ok;
+	return 0;
 }
 
 static __be32
@@ -4250,16 +4155,13 @@ nfsd4_encode_layoutreturn(struct nfsd4_compoundres *resp, __be32 nfserr,
 	struct xdr_stream *xdr = &resp->xdr;
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(xdr, 4);
 	if (!p)
 		return nfserr_resource;
 	*p++ = cpu_to_be32(lrp->lrs_present);
 	if (lrp->lrs_present)
 		return nfsd4_encode_stateid(xdr, &lrp->lr_sid);
-	return nfs_ok;
+	return 0;
 }
 #endif /* CONFIG_NFSD_PNFS */
 
@@ -4286,16 +4188,14 @@ nfsd4_encode_copy(struct nfsd4_compoundres *resp, __be32 nfserr,
 {
 	__be32 *p;
 
-	if (!nfserr) {
-		nfserr = nfsd42_encode_write_res(resp, &copy->cp_res);
-		if (nfserr)
-			return nfserr;
+	nfserr = nfsd42_encode_write_res(resp, &copy->cp_res);
+	if (nfserr)
+		return nfserr;
 
-		p = xdr_reserve_space(&resp->xdr, 4 + 4);
-		*p++ = cpu_to_be32(copy->cp_consecutive);
-		*p++ = cpu_to_be32(copy->cp_synchronous);
-	}
-	return nfserr;
+	p = xdr_reserve_space(&resp->xdr, 4 + 4);
+	*p++ = cpu_to_be32(copy->cp_consecutive);
+	*p++ = cpu_to_be32(copy->cp_synchronous);
+	return 0;
 }
 
 static __be32
@@ -4304,14 +4204,11 @@ nfsd4_encode_seek(struct nfsd4_compoundres *resp, __be32 nfserr,
 {
 	__be32 *p;
 
-	if (nfserr)
-		return nfserr;
-
 	p = xdr_reserve_space(&resp->xdr, 4 + 8);
 	*p++ = cpu_to_be32(seek->seek_eof);
 	p = xdr_encode_hyper(p, seek->seek_pos);
 
-	return nfserr;
+	return 0;
 }
 
 static __be32
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-07-17 18:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-14 20:24 [PATCH 0/5] skip op encoders in error cases J. Bruce Fields
2017-07-14 20:24 ` [PATCH 1/5] nfsd4: move some nfsd4 op definitions to xdr4.h J. Bruce Fields
2017-07-14 20:24 ` [PATCH 2/5] opdesc will be useful outside nfs4proc.c J. Bruce Fields
2017-07-14 20:24 ` [PATCH 3/5] nfsd4: define ->op_release for compound ops J. Bruce Fields
2017-07-15  9:27   ` kbuild test robot
2017-07-17 18:44     ` J. Bruce Fields
2017-07-15  9:30   ` kbuild test robot
2017-07-14 20:24 ` [PATCH 4/5] nfsd4: skip encoder in trivial error cases J. Bruce Fields
2017-07-14 20:24 ` [PATCH 5/5] nfsd4: individual encoders no longer see " J. Bruce Fields

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox