PERFORCE change 122480 for review

[John Baldwin]

On Thursday 28 June 2007 08:14:43 am Roman Divacky wrote:
> http://perforce.freebsd.org/chv.cgi?CH=122480
> 
> Change 122480 by rdivacky at rdivacky_witten on 2007/06/28 12:14:14
> 
> 	vrele AFTER checking the vp for being on non-MPSAFE FS +  some indentation
> 	problems that caused bad scoping
> 	
> 	Noticed by: rwatson (again ;) )

Umm, no.  You need Giant for the vrele().  The proper fix is to use the 
VFS_*_GIANT() macros and use int variables to know if you need to unlock 
Giant or not.

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



-- 
John Baldwin