Commit cc028a10 authored by Chuck Lever's avatar Chuck Lever Committed by J. Bruce Fields
Browse files

NFSD: Hoist status code encoding into XDR encoder functions



The original intent was presumably to reduce code duplication. The
trade-off was:

- No support for an NFSD proc function returning a non-success
  RPC accept_stat value.
- No support for void NFS replies to non-NULL procedures.
- Everyone pays for the deduplication with a few extra conditional
  branches in a hot path.

In addition, nfsd_dispatch() leaves *statp uninitialized in the
success path, unlike svc_generic_dispatch().

Address all of these problems by moving the logic for encoding
the NFS status code into the NFS XDR encoders themselves. Then
update the NFS .pc_func methods to return an RPC accept_stat
value.

Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
Signed-off-by: default avatarJ. Bruce Fields <bfields@redhat.com>
parent 4b74fd79
Loading
Loading
Loading
Loading
+12 −7
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@
static __be32
nfsacld_proc_null(struct svc_rqst *rqstp)
{
	return nfs_ok;
	return rpc_success;
}

/*
@@ -79,7 +79,7 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst *rqstp)

	/* resp->acl_{access,default} are released in nfssvc_release_getacl. */
out:
	return resp->status;
	return rpc_success;

fail:
	posix_acl_release(resp->acl_access);
@@ -131,7 +131,7 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
	   nfssvc_decode_setaclargs. */
	posix_acl_release(argp->acl_access);
	posix_acl_release(argp->acl_default);
	return resp->status;
	return rpc_success;

out_drop_lock:
	fh_unlock(fh);
@@ -157,7 +157,7 @@ static __be32 nfsacld_proc_getattr(struct svc_rqst *rqstp)
		goto out;
	resp->status = fh_getattr(&resp->fh, &resp->stat);
out:
	return resp->status;
	return rpc_success;
}

/*
@@ -179,7 +179,7 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp)
		goto out;
	resp->status = fh_getattr(&resp->fh, &resp->stat);
out:
	return resp->status;
	return rpc_success;
}

/*
@@ -275,6 +275,7 @@ static int nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p)
	int n;
	int w;

	*p++ = resp->status;
	if (resp->status != nfs_ok)
		return xdr_ressize_check(rqstp, p);

@@ -317,10 +318,12 @@ static int nfsaclsvc_encode_attrstatres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd_attrstat *resp = rqstp->rq_resp;

	*p++ = resp->status;
	if (resp->status != nfs_ok)
		return xdr_ressize_check(rqstp, p);
		goto out;

	p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
out:
	return xdr_ressize_check(rqstp, p);
}

@@ -329,11 +332,13 @@ static int nfsaclsvc_encode_accessres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_accessres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	if (resp->status != nfs_ok)
		return xdr_ressize_check(rqstp, p);
		goto out;

	p = nfs2svc_encode_fattr(rqstp, p, &resp->fh, &resp->stat);
	*p++ = htonl(resp->access);
out:
	return xdr_ressize_check(rqstp, p);
}

+5 −4
Original line number Diff line number Diff line
@@ -19,7 +19,7 @@
static __be32
nfsd3_proc_null(struct svc_rqst *rqstp)
{
	return nfs_ok;
	return rpc_success;
}

/*
@@ -71,7 +71,7 @@ static __be32 nfsd3_proc_getacl(struct svc_rqst *rqstp)

	/* resp->acl_{access,default} are released in nfs3svc_release_getacl. */
out:
	return resp->status;
	return rpc_success;

fail:
	posix_acl_release(resp->acl_access);
@@ -118,7 +118,7 @@ static __be32 nfsd3_proc_setacl(struct svc_rqst *rqstp)
	   nfs3svc_decode_setaclargs. */
	posix_acl_release(argp->acl_access);
	posix_acl_release(argp->acl_default);
	return resp->status;
	return rpc_success;
}

/*
@@ -173,6 +173,7 @@ static int nfs3svc_encode_getaclres(struct svc_rqst *rqstp, __be32 *p)
	struct nfsd3_getaclres *resp = rqstp->rq_resp;
	struct dentry *dentry = resp->fh.fh_dentry;

	*p++ = resp->status;
	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);
	if (resp->status == 0 && dentry && d_really_is_positive(dentry)) {
		struct inode *inode = d_inode(dentry);
@@ -217,8 +218,8 @@ static int nfs3svc_encode_setaclres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_attrstat *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = nfs3svc_encode_post_op_attr(rqstp, p, &resp->fh);

	return xdr_ressize_check(rqstp, p);
}

+22 −22
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ static int nfs3_ftypes[] = {
static __be32
nfsd3_proc_null(struct svc_rqst *rqstp)
{
	return nfs_ok;
	return rpc_success;
}

/*
@@ -55,7 +55,7 @@ nfsd3_proc_getattr(struct svc_rqst *rqstp)

	resp->status = fh_getattr(&resp->fh, &resp->stat);
out:
	return resp->status;
	return rpc_success;
}

/*
@@ -73,7 +73,7 @@ nfsd3_proc_setattr(struct svc_rqst *rqstp)
	fh_copy(&resp->fh, &argp->fh);
	resp->status = nfsd_setattr(rqstp, &resp->fh, &argp->attrs,
				    argp->check_guard, argp->guardtime);
	return resp->status;
	return rpc_success;
}

/*
@@ -96,7 +96,7 @@ nfsd3_proc_lookup(struct svc_rqst *rqstp)
	resp->status = nfsd_lookup(rqstp, &resp->dirfh,
				   argp->name, argp->len,
				   &resp->fh);
	return resp->status;
	return rpc_success;
}

/*
@@ -115,7 +115,7 @@ nfsd3_proc_access(struct svc_rqst *rqstp)
	fh_copy(&resp->fh, &argp->fh);
	resp->access = argp->access;
	resp->status = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
	return resp->status;
	return rpc_success;
}

/*
@@ -133,7 +133,7 @@ nfsd3_proc_readlink(struct svc_rqst *rqstp)
	fh_copy(&resp->fh, &argp->fh);
	resp->len = NFS3_MAXPATHLEN;
	resp->status = nfsd_readlink(rqstp, &resp->fh, argp->buffer, &resp->len);
	return resp->status;
	return rpc_success;
}

/*
@@ -163,7 +163,7 @@ nfsd3_proc_read(struct svc_rqst *rqstp)
	resp->status = nfsd_read(rqstp, &resp->fh, argp->offset,
				 rqstp->rq_vec, argp->vlen, &resp->count,
				 &resp->eof);
	return resp->status;
	return rpc_success;
}

/*
@@ -196,7 +196,7 @@ nfsd3_proc_write(struct svc_rqst *rqstp)
				  resp->committed, resp->verf);
	resp->count = cnt;
out:
	return resp->status;
	return rpc_success;
}

/*
@@ -234,7 +234,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp)
	resp->status = do_nfsd_create(rqstp, dirfhp, argp->name, argp->len,
				      attr, newfhp, argp->createmode,
				      (u32 *)argp->verf, NULL, NULL);
	return resp->status;
	return rpc_success;
}

/*
@@ -257,7 +257,7 @@ nfsd3_proc_mkdir(struct svc_rqst *rqstp)
	resp->status = nfsd_create(rqstp, &resp->dirfh, argp->name, argp->len,
				   &argp->attrs, S_IFDIR, 0, &resp->fh);
	fh_unlock(&resp->dirfh);
	return resp->status;
	return rpc_success;
}

static __be32
@@ -294,7 +294,7 @@ nfsd3_proc_symlink(struct svc_rqst *rqstp)
				    argp->flen, argp->tname, &resp->fh);
	kfree(argp->tname);
out:
	return resp->status;
	return rpc_success;
}

/*
@@ -337,7 +337,7 @@ nfsd3_proc_mknod(struct svc_rqst *rqstp)
				   &argp->attrs, type, rdev, &resp->fh);
	fh_unlock(&resp->dirfh);
out:
	return resp->status;
	return rpc_success;
}

/*
@@ -359,7 +359,7 @@ nfsd3_proc_remove(struct svc_rqst *rqstp)
	resp->status = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR,
				   argp->name, argp->len);
	fh_unlock(&resp->fh);
	return resp->status;
	return rpc_success;
}

/*
@@ -380,7 +380,7 @@ nfsd3_proc_rmdir(struct svc_rqst *rqstp)
	resp->status = nfsd_unlink(rqstp, &resp->fh, S_IFDIR,
				   argp->name, argp->len);
	fh_unlock(&resp->fh);
	return resp->status;
	return rpc_success;
}

static __be32
@@ -402,7 +402,7 @@ nfsd3_proc_rename(struct svc_rqst *rqstp)
	fh_copy(&resp->tfh, &argp->tfh);
	resp->status = nfsd_rename(rqstp, &resp->ffh, argp->fname, argp->flen,
				   &resp->tfh, argp->tname, argp->tlen);
	return resp->status;
	return rpc_success;
}

static __be32
@@ -422,7 +422,7 @@ nfsd3_proc_link(struct svc_rqst *rqstp)
	fh_copy(&resp->tfh, &argp->tfh);
	resp->status = nfsd_link(rqstp, &resp->tfh, argp->tname, argp->tlen,
				 &resp->fh);
	return resp->status;
	return rpc_success;
}

/*
@@ -481,7 +481,7 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
		resp->offset = NULL;
	}

	return resp->status;
	return rpc_success;
}

/*
@@ -551,7 +551,7 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
	}

out:
	return resp->status;
	return rpc_success;
}

/*
@@ -568,7 +568,7 @@ nfsd3_proc_fsstat(struct svc_rqst *rqstp)

	resp->status = nfsd_statfs(rqstp, &argp->fh, &resp->stats, 0);
	fh_put(&argp->fh);
	return resp->status;
	return rpc_success;
}

/*
@@ -611,7 +611,7 @@ nfsd3_proc_fsinfo(struct svc_rqst *rqstp)
	}

	fh_put(&argp->fh);
	return resp->status;
	return rpc_success;
}

/*
@@ -653,7 +653,7 @@ nfsd3_proc_pathconf(struct svc_rqst *rqstp)
	}

	fh_put(&argp->fh);
	return resp->status;
	return rpc_success;
}

/*
@@ -679,7 +679,7 @@ nfsd3_proc_commit(struct svc_rqst *rqstp)
	resp->status = nfsd_commit(rqstp, &resp->fh, argp->offset,
				   argp->count, resp->verf);
out:
	return resp->status;
	return rpc_success;
}


+15 −4
Original line number Diff line number Diff line
@@ -641,10 +641,7 @@ nfs3svc_decode_commitargs(struct svc_rqst *rqstp, __be32 *p)
/*
 * XDR encode functions
 */
/*
 * There must be an encoding function for void results so svc_process
 * will work properly.
 */

int
nfs3svc_encode_voidres(struct svc_rqst *rqstp, __be32 *p)
{
@@ -657,6 +654,7 @@ nfs3svc_encode_attrstat(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_attrstat *resp = rqstp->rq_resp;

	*p++ = resp->status;
	if (resp->status == 0) {
		lease_get_mtime(d_inode(resp->fh.fh_dentry),
				&resp->stat.mtime);
@@ -671,6 +669,7 @@ nfs3svc_encode_wccstat(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_attrstat *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_wcc_data(rqstp, p, &resp->fh);
	return xdr_ressize_check(rqstp, p);
}
@@ -681,6 +680,7 @@ nfs3svc_encode_diropres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_diropres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	if (resp->status == 0) {
		p = encode_fh(p, &resp->fh);
		p = encode_post_op_attr(rqstp, p, &resp->fh);
@@ -695,6 +695,7 @@ nfs3svc_encode_accessres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_accessres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_post_op_attr(rqstp, p, &resp->fh);
	if (resp->status == 0)
		*p++ = htonl(resp->access);
@@ -707,6 +708,7 @@ nfs3svc_encode_readlinkres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_readlinkres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_post_op_attr(rqstp, p, &resp->fh);
	if (resp->status == 0) {
		*p++ = htonl(resp->len);
@@ -729,6 +731,7 @@ nfs3svc_encode_readres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_readres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_post_op_attr(rqstp, p, &resp->fh);
	if (resp->status == 0) {
		*p++ = htonl(resp->count);
@@ -754,6 +757,7 @@ nfs3svc_encode_writeres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_writeres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_wcc_data(rqstp, p, &resp->fh);
	if (resp->status == 0) {
		*p++ = htonl(resp->count);
@@ -770,6 +774,7 @@ nfs3svc_encode_createres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_diropres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	if (resp->status == 0) {
		*p++ = xdr_one;
		p = encode_fh(p, &resp->fh);
@@ -785,6 +790,7 @@ nfs3svc_encode_renameres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_renameres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_wcc_data(rqstp, p, &resp->ffh);
	p = encode_wcc_data(rqstp, p, &resp->tfh);
	return xdr_ressize_check(rqstp, p);
@@ -796,6 +802,7 @@ nfs3svc_encode_linkres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_linkres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_post_op_attr(rqstp, p, &resp->fh);
	p = encode_wcc_data(rqstp, p, &resp->tfh);
	return xdr_ressize_check(rqstp, p);
@@ -807,6 +814,7 @@ nfs3svc_encode_readdirres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_readdirres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_post_op_attr(rqstp, p, &resp->fh);

	if (resp->status == 0) {
@@ -1059,6 +1067,7 @@ nfs3svc_encode_fsstatres(struct svc_rqst *rqstp, __be32 *p)
	struct kstatfs	*s = &resp->stats;
	u64		bs = s->f_bsize;

	*p++ = resp->status;
	*p++ = xdr_zero;	/* no post_op_attr */

	if (resp->status == 0) {
@@ -1079,6 +1088,7 @@ nfs3svc_encode_fsinfores(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_fsinfores *resp = rqstp->rq_resp;

	*p++ = resp->status;
	*p++ = xdr_zero;	/* no post_op_attr */

	if (resp->status == 0) {
@@ -1124,6 +1134,7 @@ nfs3svc_encode_commitres(struct svc_rqst *rqstp, __be32 *p)
{
	struct nfsd3_commitres *resp = rqstp->rq_resp;

	*p++ = resp->status;
	p = encode_wcc_data(rqstp, p, &resp->fh);
	/* Write verifier */
	if (resp->status == 0) {
+3 −4
Original line number Diff line number Diff line
@@ -2165,7 +2165,7 @@ nfsd4_removexattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
static __be32
nfsd4_proc_null(struct svc_rqst *rqstp)
{
	return nfs_ok;
	return rpc_success;
}

static inline void nfsd4_increment_op_stats(u32 opnum)
@@ -2457,15 +2457,14 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
		nfsd4_increment_op_stats(op->opnum);
	}

	cstate->status = status;
	fh_put(current_fh);
	fh_put(save_fh);
	BUG_ON(cstate->replay_owner);
out:
	cstate->status = status;
	/* Reset deferral mechanism for RPC deferrals */
	set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
	dprintk("nfsv4 compound returned %d\n", ntohl(status));
	return status;
	return rpc_success;
}

#define op_encode_hdr_size		(2)
Loading