Mercurial > illumos > illumos-gate

changeset 10899:b4dce54e32ff

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
6870042 nfsgen nfsv4 delegation subset of testcases fail consistently since snv_120 6871991 rfs4_file_t fields updated without holding locks
author James Wahlig <James.Wahlig@Sun.COM>
date Wed, 28 Oct 2009 13:39:45 -0500
parents 1883b621b3ea
children bcc724d0f0a0
files usr/src/uts/common/fs/nfs/nfs4_srv.c usr/src/uts/common/nfs/nfs4.h
diffstat 2 files changed, 65 insertions(+), 49 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/fs/nfs/nfs4_srv.c	Wed Oct 28 10:36:39 2009 -0700
+++ b/usr/src/uts/common/fs/nfs/nfs4_srv.c	Wed Oct 28 13:39:45 2009 -0500
@@ -6586,8 +6586,8 @@
 	rfs4_file_t *fp;
 	bool_t screate = TRUE;
 	bool_t fcreate = TRUE;
-	uint32_t amodes;
-	uint32_t dmodes;
+	uint32_t open_a, share_a;
+	uint32_t open_d, share_d;
 	rfs4_deleg_state_t *dsp;
 	sysid_t sysid;
 	nfsstat4 status;
@@ -6640,10 +6640,17 @@
 	 * Calculate the new deny and access mode that this open is adding to
 	 * the file for this open owner;
 	 */
-	dmodes = (deny & ~sp->rs_share_deny);
-	amodes = (access & ~sp->rs_share_access);
-
-	first_open = (sp->rs_share_access & OPEN4_SHARE_ACCESS_BOTH) == 0;
+	open_d = (deny & ~sp->rs_open_deny);
+	open_a = (access & ~sp->rs_open_access);
+
+	/*
+	 * Calculate the new share access and share deny modes that this open
+	 * is adding to the file for this open owner;
+	 */
+	share_a = (access & ~sp->rs_share_access);
+	share_d = (deny & ~sp->rs_share_deny);
+
+	first_open = (sp->rs_open_access & OPEN4_SHARE_ACCESS_BOTH) == 0;
 
 	/*
 	 * Check to see the client has already sent an open for this
@@ -6656,7 +6663,7 @@
 	 * on shares modes.
 	 */
 
-	if (dmodes || amodes) {
+	if (share_a || share_d) {
 		if ((err = rfs4_share(sp, access, deny)) != 0) {
 			rfs4_dbe_unlock(sp->rs_dbe);
 			resp->status = err;
@@ -6685,7 +6692,7 @@
 
 		/* if state closed while lock was dropped */
 		if (sp->rs_closed) {
-			if (dmodes || amodes)
+			if (share_a || share_d)
 				(void) rfs4_unshare(sp);
 			rfs4_dbe_unlock(sp->rs_dbe);
 			rfs4_file_rele(fp);
@@ -6701,7 +6708,7 @@
 		/* Let's see if the delegation was returned */
 		if (rfs4_check_recall(sp, access)) {
 			rfs4_dbe_unlock(fp->rf_dbe);
-			if (dmodes || amodes)
+			if (share_a || share_d)
 				(void) rfs4_unshare(sp);
 			rfs4_dbe_unlock(sp->rs_dbe);
 			rfs4_file_rele(fp);
@@ -6736,7 +6743,7 @@
 		err = VOP_OPEN(&cs->vp, fflags, cs->cr, &ct);
 		if (err) {
 			rfs4_dbe_unlock(fp->rf_dbe);
-			if (dmodes || amodes)
+			if (share_a || share_d)
 				(void) rfs4_unshare(sp);
 			rfs4_dbe_unlock(sp->rs_dbe);
 			rfs4_file_rele(fp);
@@ -6758,23 +6765,24 @@
 		 * by this upgrade.
 		 */
 		fflags = 0;
-		if (amodes & OPEN4_SHARE_ACCESS_READ)
+		if (open_a & OPEN4_SHARE_ACCESS_READ)
 			fflags |= FREAD;
-		if (amodes & OPEN4_SHARE_ACCESS_WRITE)
+		if (open_a & OPEN4_SHARE_ACCESS_WRITE)
 			fflags |= FWRITE;
 		vn_open_upgrade(cs->vp, fflags);
 	}
-	sp->rs_opened = TRUE;
-
-	if (dmodes & OPEN4_SHARE_DENY_READ)
+	sp->rs_open_access |= access;
+	sp->rs_open_deny |= deny;
+
+	if (open_d & OPEN4_SHARE_DENY_READ)
 		fp->rf_deny_read++;
-	if (dmodes & OPEN4_SHARE_DENY_WRITE)
+	if (open_d & OPEN4_SHARE_DENY_WRITE)
 		fp->rf_deny_write++;
 	fp->rf_share_deny |= deny;
 
-	if (amodes & OPEN4_SHARE_ACCESS_READ)
+	if (open_a & OPEN4_SHARE_ACCESS_READ)
 		fp->rf_access_read++;
-	if (amodes & OPEN4_SHARE_ACCESS_WRITE)
+	if (open_a & OPEN4_SHARE_ACCESS_WRITE)
 		fp->rf_access_write++;
 	fp->rf_share_access |= access;
 
@@ -7659,14 +7667,14 @@
 	 * the new mode is a subset of the current modes we bitwise
 	 * AND them together and check that the result equals the new
 	 * mode. For example:
-	 * New mode, access == R and current mode, sp->rs_share_access  == RW
-	 * access & sp->rs_share_access == R == access, so the new access mode
-	 * is valid. Consider access == RW, sp->rs_share_access = R
-	 * access & sp->rs_share_access == R != access, so the new access mode
+	 * New mode, access == R and current mode, sp->rs_open_access  == RW
+	 * access & sp->rs_open_access == R == access, so the new access mode
+	 * is valid. Consider access == RW, sp->rs_open_access = R
+	 * access & sp->rs_open_access == R != access, so the new access mode
 	 * is invalid.
 	 */
-	if ((access & sp->rs_share_access) != access ||
-	    (deny & sp->rs_share_deny) != deny ||
+	if ((access & sp->rs_open_access) != access ||
+	    (deny & sp->rs_open_deny) != deny ||
 	    (access &
 	    (OPEN4_SHARE_ACCESS_READ | OPEN4_SHARE_ACCESS_WRITE)) == 0) {
 		*cs->statusp = resp->status = NFS4ERR_INVAL;
@@ -7683,6 +7691,14 @@
 	 */
 	(void) rfs4_unshare(sp);
 
+	status = rfs4_share(sp, access, deny);
+	if (status != NFS4_OK) {
+		*cs->statusp = resp->status = NFS4ERR_SERVERFAULT;
+		rfs4_update_open_sequence(sp->rs_owner);
+		rfs4_dbe_unlock(sp->rs_dbe);
+		goto end;
+	}
+
 	fp = sp->rs_finfo;
 	rfs4_dbe_lock(fp->rf_dbe);
 
@@ -7692,7 +7708,7 @@
 	 * and if it goes to zero turn off the deny read bit
 	 * on the file.
 	 */
-	if ((sp->rs_share_deny & OPEN4_SHARE_DENY_READ) &&
+	if ((sp->rs_open_deny & OPEN4_SHARE_DENY_READ) &&
 	    (deny & OPEN4_SHARE_DENY_READ) == 0) {
 		fp->rf_deny_read--;
 		if (fp->rf_deny_read == 0)
@@ -7705,7 +7721,7 @@
 	 * and if it goes to zero turn off the deny write bit
 	 * on the file.
 	 */
-	if ((sp->rs_share_deny & OPEN4_SHARE_DENY_WRITE) &&
+	if ((sp->rs_open_deny & OPEN4_SHARE_DENY_WRITE) &&
 	    (deny & OPEN4_SHARE_DENY_WRITE) == 0) {
 		fp->rf_deny_write--;
 		if (fp->rf_deny_write == 0)
@@ -7719,7 +7735,7 @@
 	 * on the file.  set fflags to FREAD for the call to
 	 * vn_open_downgrade().
 	 */
-	if ((sp->rs_share_access & OPEN4_SHARE_ACCESS_READ) &&
+	if ((sp->rs_open_access & OPEN4_SHARE_ACCESS_READ) &&
 	    (access & OPEN4_SHARE_ACCESS_READ) == 0) {
 		fp->rf_access_read--;
 		if (fp->rf_access_read == 0)
@@ -7734,7 +7750,7 @@
 	 * on the file.  set fflags to FWRITE for the call to
 	 * vn_open_downgrade().
 	 */
-	if ((sp->rs_share_access & OPEN4_SHARE_ACCESS_WRITE) &&
+	if ((sp->rs_open_access & OPEN4_SHARE_ACCESS_WRITE) &&
 	    (access & OPEN4_SHARE_ACCESS_WRITE) == 0) {
 		fp->rf_access_write--;
 		if (fp->rf_access_write == 0)
@@ -7747,25 +7763,18 @@
 
 	rfs4_dbe_unlock(fp->rf_dbe);
 
-	status = rfs4_share(sp, access, deny);
-	rfs4_dbe_unlock(sp->rs_dbe);
-
-	if (status != NFS4_OK) {
-		*cs->statusp = resp->status = NFS4ERR_SERVERFAULT;
-		rfs4_update_open_sequence(sp->rs_owner);
-		goto end;
-	}
+	/* now set the new open access and deny modes */
+	sp->rs_open_access = access;
+	sp->rs_open_deny = deny;
 
 	/*
 	 * we successfully downgraded the share lock, now we need to downgrade
-	 * the open.  it is possible that the downgrade was only for a deny
+	 * the open. it is possible that the downgrade was only for a deny
 	 * mode and we have nothing else to do.
 	 */
 	if ((fflags & (FREAD|FWRITE)) != 0)
 		vn_open_downgrade(cs->vp, fflags);
 
-	rfs4_dbe_lock(sp->rs_dbe);
-
 	/* Update the stateid */
 	next_stateid(&sp->rs_stateid);
 	resp->open_stateid = sp->rs_stateid.stateid;
@@ -8273,32 +8282,34 @@
 	if (sp->rs_owner->ro_client->rc_sysidt != LM_NOSYSID)
 		(void) rfs4_unshare(sp);
 
-	if (sp->rs_opened) {
+	if (sp->rs_open_access) {
+		rfs4_dbe_lock(fp->rf_dbe);
+
 		/*
 		 * Decrement the count for each access and deny bit that this
 		 * state has contributed to the file.
 		 * If the file counts go to zero
 		 * clear the appropriate bit in the appropriate mask.
 		 */
-		if (sp->rs_share_access & OPEN4_SHARE_ACCESS_READ) {
+		if (sp->rs_open_access & OPEN4_SHARE_ACCESS_READ) {
 			fp->rf_access_read--;
 			fflags |= FREAD;
 			if (fp->rf_access_read == 0)
 				fp->rf_share_access &= ~OPEN4_SHARE_ACCESS_READ;
 		}
-		if (sp->rs_share_access & OPEN4_SHARE_ACCESS_WRITE) {
+		if (sp->rs_open_access & OPEN4_SHARE_ACCESS_WRITE) {
 			fp->rf_access_write--;
 			fflags |= FWRITE;
 			if (fp->rf_access_write == 0)
 				fp->rf_share_access &=
 				    ~OPEN4_SHARE_ACCESS_WRITE;
 		}
-		if (sp->rs_share_deny & OPEN4_SHARE_DENY_READ) {
+		if (sp->rs_open_deny & OPEN4_SHARE_DENY_READ) {
 			fp->rf_deny_read--;
 			if (fp->rf_deny_read == 0)
 				fp->rf_share_deny &= ~OPEN4_SHARE_DENY_READ;
 		}
-		if (sp->rs_share_deny & OPEN4_SHARE_DENY_WRITE) {
+		if (sp->rs_open_deny & OPEN4_SHARE_DENY_WRITE) {
 			fp->rf_deny_write--;
 			if (fp->rf_deny_write == 0)
 				fp->rf_share_deny &= ~OPEN4_SHARE_DENY_WRITE;
@@ -8306,7 +8317,10 @@
 
 		(void) VOP_CLOSE(fp->rf_vp, fflags, 1, (offset_t)0, cr, NULL);
 
-		sp->rs_opened = FALSE;
+		rfs4_dbe_unlock(fp->rf_dbe);
+
+		sp->rs_open_access = 0;
+		sp->rs_open_deny = 0;
 	}
 }
 
--- a/usr/src/uts/common/nfs/nfs4.h	Wed Oct 28 10:36:39 2009 -0700
+++ b/usr/src/uts/common/nfs/nfs4.h	Wed Oct 28 13:39:45 2009 -0500
@@ -543,9 +543,10 @@
  * stateid - server provided stateid
  * owner - reference back to the openowner for this state
  * finfo - reference to the open file for this state
- * share_access - how did the openowner OPEN the file (access)
- * share_deny - how did the openowner OPEN the file (deny)
- * opened - has VOP_OPEN been done
+ * open_access - how did the openowner OPEN the file (access)
+ * open_deny - how did the openowner OPEN the file (deny)
+ * share_access - what share reservation is on the file (access)
+ * share_deny - what share reservation is on the file (deny)
  * closed - has this file been closed?
  * lostatelist - root of list of lo_state associated with this state/file
  * node - node for state struct list of states
@@ -555,9 +556,10 @@
 	stateid_t		rs_stateid;
 	rfs4_openowner_t	*rs_owner;
 	struct rfs4_file	*rs_finfo;
+	uint32_t		rs_open_access;
+	uint32_t		rs_open_deny;
 	uint32_t		rs_share_access;
 	uint32_t		rs_share_deny;
-	unsigned		rs_opened:1;
 	unsigned		rs_closed:1;
 	list_t			rs_lostatelist;
 	list_node_t		rs_node;