Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 12 Jul 2007 11:03:32 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Roman Divacky <rdivacky@freebsd.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 122854 for review
Message-ID:  <200707121103.32541.jhb@freebsd.org>
In-Reply-To: <200707041342.l64Dgm6H071641@repoman.freebsd.org>
References:  <200707041342.l64Dgm6H071641@repoman.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 04 July 2007 09:42:48 am Roman Divacky wrote:
> http://perforce.freebsd.org/chv.cgi?CH=122854
> 
> Change 122854 by rdivacky@rdivacky_witten on 2007/07/04 13:42:23
> 
> 	vrele() before Giant unlock. Use VFS_UNLOCK_GIANT macro. Reuse "vfslocked"
> 	variable where possible, elsewhere introduce "vfslocked1" varibale.
> 	
> 	Suggested by: jhb

dvfslocked would be preferable to vfslocked1, and more consistent with other 
places in the kernel with multiple vfslocked variables.

> Affected files ...
> 
> .. //depot/projects/soc2007/rdivacky/linux_at/sys/kern/vfs_syscalls.c#43 
edit
> 
> Differences ...
> 
> ==== //depot/projects/soc2007/rdivacky/linux_at/sys/kern/vfs_syscalls.c#43 
(text+ko) ====
> 
> @@ -1164,9 +1164,9 @@
>  	fdclose(fdp, fp, indx, td);
>  	fdrop(fp, td);
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -1272,9 +1272,9 @@
>  		return (error);
>  restart:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	error = kern_get_at(td, fd, &dvp);
>  	if (error && !kern_absolute_path(path, pathseg))
> @@ -1354,9 +1354,9 @@
>  		}
>  	}
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		int vfslocked1 = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked1);
>  	}
>  	NDFREE(&nd, NDF_ONLY_PNBUF);
>  	vput(nd.ni_dvp);
> @@ -1419,9 +1419,9 @@
>  	AUDIT_ARG(mode, mode);
>  restart:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	error = kern_get_at(td, fd, &dvp);
>  	if (error && !kern_absolute_path(path, pathseg))
> @@ -1477,9 +1477,9 @@
>  out:
>  #endif
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		int vfslocked1 = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked1);
>  	}
>  	vput(nd.ni_dvp);
>  	vn_finished_write(mp);
> @@ -1602,9 +1602,9 @@
>  	error = kern_get_at(td, fd2, &ldvp);
>  	if (error && !kern_absolute_path(path2, segflg)) {
>  		if (pdvp) {
> -			if (VFS_NEEDSGIANT(pdvp->v_mount))
> -				mtx_unlock(&Giant);
> +			vfslocked = VFS_NEEDSGIANT(pdvp->v_mount); 
>  			vrele(pdvp);
> +			VFS_UNLOCK_GIANT(vfslocked);
>  		}
>  		return (error);
>  	}
> @@ -1662,14 +1662,14 @@
>  
>  out:
>  	if (pdvp) {
> -		if (VFS_NEEDSGIANT(pdvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(pdvp->v_mount); 
>  		vrele(pdvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	if (ldvp) {
> -		if (VFS_NEEDSGIANT(ldvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(ldvp->v_mount); 
>  		vrele(ldvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -1737,9 +1737,9 @@
>  	AUDIT_ARG(text, syspath);
>  restart:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	error = kern_get_at(td, fd, &dvp);
>  	if (error && !kern_absolute_path(path2, segflg))
> @@ -1793,9 +1793,9 @@
>  	VFS_UNLOCK_GIANT(vfslocked);
>  out:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	if (segflg != UIO_SYSSPACE)
>  		uma_zfree(namei_zone, syspath);
> @@ -1911,9 +1911,9 @@
>  
>  restart:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	error = kern_get_at(td, fd, &dvp);
>  	if (error && !kern_absolute_path(path, pathseg))
> @@ -1972,9 +1972,9 @@
>  	NDFREE(&nd, NDF_ONLY_PNBUF);
>  	vput(nd.ni_dvp);
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		int vfslocked1 = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked1);
>  	}
>  	if (vp == nd.ni_dvp)
>  		vrele(vp);
> @@ -2228,9 +2228,9 @@
>  	td->td_ucred = cred;
>  	crfree(tmpcred);
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -2447,9 +2447,9 @@
>  		*sbp = sb;
>  out:	
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -2514,9 +2514,9 @@
>  		*sbp = sb;
>  out:	
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -2743,9 +2743,9 @@
>  	td->td_retval[0] = count - auio.uio_resid;
>  out:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -2993,9 +2993,9 @@
>  	VFS_UNLOCK_GIANT(vfslocked);
>  out:	
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -3045,9 +3045,9 @@
>  	vrele(nd.ni_vp);
>  	VFS_UNLOCK_GIANT(vfslocked);
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  
>  	return (error);
> @@ -3192,9 +3192,9 @@
>  	VFS_UNLOCK_GIANT(vfslocked);
>  out:	
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -3253,9 +3253,9 @@
>  	VFS_UNLOCK_GIANT(vfslocked);
>  out:	
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -3443,9 +3443,9 @@
>  	VFS_UNLOCK_GIANT(vfslocked);
>  out:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	return (error);
>  }
> @@ -3835,9 +3835,9 @@
>  	error = kern_get_at(td, newfd, &todvp);
>  	if (error && !kern_absolute_path(new, pathseg)) {
>  		if (frdvp) {
> -			if (VFS_NEEDSGIANT(frdvp->v_mount))
> -				mtx_unlock(&Giant);
> +			fvfslocked = VFS_NEEDSGIANT(frdvp->v_mount); 
>  			vrele(frdvp);
> +			VFS_UNLOCK_GIANT(fvfslocked);
>  		}
>  		return (error);
>  	}
> @@ -3947,14 +3947,14 @@
>  		return (0);
>  out2:
>  	if (frdvp) {
> -		if (VFS_NEEDSGIANT(frdvp->v_mount))
> -			mtx_unlock(&Giant);
> +		fvfslocked = VFS_NEEDSGIANT(frdvp->v_mount); 
>  		vrele(frdvp);
> +		VFS_UNLOCK_GIANT(fvfslocked);
>  	}
>  	if (todvp) {
> -		if (VFS_NEEDSGIANT(todvp->v_mount))
> -			mtx_unlock(&Giant);
> +		tvfslocked = VFS_NEEDSGIANT(todvp->v_mount); 
>  		vrele(todvp);
> +		VFS_UNLOCK_GIANT(tvfslocked);
>  	}
>  	return (error);
>  }
> @@ -4013,9 +4013,9 @@
>  	AUDIT_ARG(mode, mode);
>  restart:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	error = kern_get_at(td, fd, &dvp);
>  	if (error && !kern_absolute_path(path, segflg))
> @@ -4076,9 +4076,9 @@
>  out:
>  #endif
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		int vfslocked1 = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked1);
>  	}
>  	NDFREE(&nd, NDF_ONLY_PNBUF);
>  	vput(nd.ni_dvp);
> @@ -4125,9 +4125,9 @@
>  
>  restart:
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		vfslocked = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked);
>  	}
>  	error = kern_get_at(td, fd, &dvp);
>  	if (error && !kern_absolute_path(path, pathseg))
> @@ -4183,9 +4183,9 @@
>  	NDFREE(&nd, NDF_ONLY_PNBUF);
>  	vput(vp);
>  	if (dvp) {
> -		if (VFS_NEEDSGIANT(dvp->v_mount))
> -			mtx_unlock(&Giant);
> +		int vfslocked1 = VFS_NEEDSGIANT(dvp->v_mount); 
>  		vrele(dvp);
> +		VFS_UNLOCK_GIANT(vfslocked1);
>  	}
>  	if (nd.ni_dvp == vp)
>  		vrele(nd.ni_dvp);
> 



-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200707121103.32541.jhb>