Commit a078a7dc authored by NeilBrown's avatar NeilBrown Committed by Chuck Lever
Browse files

nfsd: untangle code in nfsd4_deleg_getattr_conflict() 



The code in nfsd4_deleg_getattr_conflict() is convoluted and buggy.

With this patch we:
 - properly handle non-nfsd leases.  We must not assume flc_owner is a
    delegation unless fl_lmops == &nfsd_lease_mng_ops
 - move the main code out of the for loop
 - have a single exit which calls nfs4_put_stid()
   (and other exits which don't need to call that)

[ jlayton: refactored on top of Neil's other patch: nfsd: fix
	   nfsd4_deleg_getattr_conflict in presence of third party lease ]

Fixes: c5967721 ("NFSD: handle GETATTR conflict with write delegation")
Signed-off-by: default avatarNeilBrown <neilb@suse.de>
Signed-off-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
parent 5559c157

fs/nfsd/nfs4state.c

+62 −69
Original line number Diff line number Diff line
@@ -8834,6 +8834,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, 
	__be32 status;

	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);

	struct file_lock_context *ctx;

	struct nfs4_delegation *dp = NULL;

	struct file_lease *fl;

	struct iattr attrs;

	struct nfs4_cb_fattr *ncf;
@@ -8843,53 +8844,48 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, 
	ctx = locks_inode_context(inode);

	if (!ctx)

		return 0;



#define NON_NFSD_LEASE ((void *)1)



	spin_lock(&ctx->flc_lock);

	for_each_file_lock(fl, &ctx->flc_lease) {

		unsigned char type = fl->c.flc_type;



		if (fl->c.flc_flags == FL_LAYOUT)

			continue;

		if (fl->fl_lmops != &nfsd_lease_mng_ops) {

			/*

			 * non-nfs lease, if it's a lease with F_RDLCK then

			 * we are done; there isn't any write delegation

			 * on this inode

			 */

			if (type == F_RDLCK)

		if (fl->c.flc_type == F_WRLCK) {

			if (fl->fl_lmops == &nfsd_lease_mng_ops)

				dp = fl->c.flc_owner;

			else

				dp = NON_NFSD_LEASE;

		}

		break;



			nfsd_stats_wdeleg_getattr_inc(nn);

	}

	if (dp == NULL || dp == NON_NFSD_LEASE ||

	    dp->dl_recall.cb_clp == *(rqstp->rq_lease_breaker)) {

		spin_unlock(&ctx->flc_lock);



			status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ));

		if (dp == NON_NFSD_LEASE) {

			status = nfserrno(nfsd_open_break_lease(inode,

								NFSD_MAY_READ));

			if (status != nfserr_jukebox ||

			    !nfsd_wait_for_delegreturn(rqstp, inode))

				return status;

			return 0;

		}

		if (type == F_WRLCK) {

			struct nfs4_delegation *dp = fl->c.flc_owner;



			if (dp->dl_recall.cb_clp == *(rqstp->rq_lease_breaker)) {

				spin_unlock(&ctx->flc_lock);

		return 0;

	}



	nfsd_stats_wdeleg_getattr_inc(nn);

			dp = fl->c.flc_owner;

	refcount_inc(&dp->dl_stid.sc_count);

	ncf = &dp->dl_cb_fattr;

	nfs4_cb_getattr(&dp->dl_cb_fattr);

	spin_unlock(&ctx->flc_lock);



	wait_on_bit_timeout(&ncf->ncf_cb_flags, CB_GETATTR_BUSY,

			    TASK_INTERRUPTIBLE, NFSD_CB_GETATTR_TIMEOUT);

	if (ncf->ncf_cb_status) {

		/* Recall delegation only if client didn't respond */

		status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ));

		if (status != nfserr_jukebox ||

						!nfsd_wait_for_delegreturn(rqstp, inode)) {

					nfs4_put_stid(&dp->dl_stid);

					return status;

				}

		    !nfsd_wait_for_delegreturn(rqstp, inode))

			goto out_status;

	}

	if (!ncf->ncf_file_modified &&

	    (ncf->ncf_initial_cinfo != ncf->ncf_cb_change ||
@@ -8909,18 +8905,15 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry, 
		err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);

		inode_unlock(inode);

		if (err) {

					nfs4_put_stid(&dp->dl_stid);

					return nfserrno(err);

			status = nfserrno(err);

			goto out_status;

		}

		ncf->ncf_cur_fsize = ncf->ncf_cb_fsize;

		*size = ncf->ncf_cur_fsize;

		*modified = true;

	}

	status = 0;

out_status:

	nfs4_put_stid(&dp->dl_stid);

			return 0;

		}

		break;

	}

	spin_unlock(&ctx->flc_lock);

	return 0;

	return status;

}