Subject: small nfs changes
To: None <tech-kern@netbsd.org>
From: Christos Zoulas <christos@zoulas.com>
List: tech-kern
Date: 07/06/2005 19:26:46
I'd like your opinion on the following changes:

1. use p = uio->uio_procp consistently and eliminate suspicious uses
   of curproc (where uio->uio_procp should be used?)
2. nfs_doio() does not use struct proc; remove it and the code to compute it.
3. use copyin_proc() and copyout_proc() instead of copyin() and copyout().
4. check return of copyout_proc(). and mark return from copyin_proc() XXX
5. Eliminate check p == curproc assertion check from nfs_write;
   nfs_read does not have it and we might be called in a different
   process context anyway (PR 20138).

Comments?

christos

Index: nfs_bio.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_bio.c,v
retrieving revision 1.128
diff -u -u -r1.128 nfs_bio.c
--- nfs_bio.c	26 Feb 2005 22:39:50 -0000	1.128
+++ nfs_bio.c	6 Jul 2005 23:20:01 -0000
@@ -85,7 +85,7 @@
 {
 	struct nfsnode *np = VTONFS(vp);
 	struct buf *bp = NULL, *rabp;
-	struct proc *p;
+	struct proc *p = uio->uio_procp;
 	struct nfsmount *nmp = VFSTONFS(vp->v_mount);
 	struct nfsdircache *ndp = NULL, *nndp = NULL;
 	caddr_t baddr, ep, edp;
@@ -102,7 +102,6 @@
 		return (0);
 	if (vp->v_type != VDIR && uio->uio_offset < 0)
 		return (EINVAL);
-	p = uio->uio_procp;
 #ifndef NFS_V2_ONLY
 	if ((nmp->nm_flag & NFSMNT_NFSV3) &&
 	    !(nmp->nm_iflag & NFSMNT_GOTFSINFO))
@@ -221,7 +220,7 @@
 			return (EINTR);
 		if ((bp->b_flags & B_DONE) == 0) {
 			bp->b_flags |= B_READ;
-			error = nfs_doio(bp, p);
+			error = nfs_doio(bp);
 			if (error) {
 				brelse(bp);
 				return (error);
@@ -266,7 +265,7 @@
 		if ((bp->b_flags & B_DONE) == 0) {
 		    bp->b_flags |= B_READ;
 		    bp->b_dcookie = ndp->dc_blkcookie;
-		    error = nfs_doio(bp, p);
+		    error = nfs_doio(bp);
 		    if (error) {
 			/*
 			 * Yuck! The directory has been modified on the
@@ -504,8 +503,6 @@
 #ifdef DIAGNOSTIC
 	if (uio->uio_rw != UIO_WRITE)
 		panic("nfs_write mode");
-	if (uio->uio_segflg == UIO_USERSPACE && uio->uio_procp != curproc)
-		panic("nfs_write proc");
 #endif
 	if (vp->v_type != VREG)
 		return (EIO);
@@ -923,6 +920,7 @@
 	struct vnode *vp = bp->b_vp;
 	struct nfsnode *np = VTONFS(vp);
 	struct nfsmount *nmp = VFSTONFS(vp->v_mount);
+	struct proc *p = uiop->uio_procp;
 	int error = 0;
 
 	uiop->uio_rw = UIO_READ;
@@ -947,7 +945,7 @@
 			len = uiop->uio_resid;
 			memset((char *)bp->b_data + diff, 0, len);
 		}
-		if (uiop->uio_procp && (vp->v_flag & VTEXT) &&
+		if (p && (vp->v_flag & VTEXT) &&
 		    (((nmp->nm_flag & NFSMNT_NQNFS) &&
 		      NQNFS_CKINVALID(vp, np, ND_READ) &&
 		      np->n_lrev != np->n_brev) ||
@@ -955,16 +953,16 @@
 		      timespeccmp(&np->n_mtime, &np->n_vattr->va_mtime, !=)))) {
 			uprintf("Process killed due to "
 				"text file modification\n");
-			psignal(uiop->uio_procp, SIGKILL);
+			psignal(p, SIGKILL);
 #if 0 /* XXX NJWLWP */
-			uiop->uio_procp->p_holdcnt++;
+			p->p_holdcnt++;
 #endif
 		}
 		break;
 	case VLNK:
 		KASSERT(uiop->uio_offset == (off_t)0);
 		nfsstats.readlink_bios++;
-		error = nfs_readlinkrpc(vp, uiop, curproc->p_ucred);
+		error = nfs_readlinkrpc(vp, uiop, p->p_ucred);
 		break;
 	case VDIR:
 		nfsstats.readdir_bios++;
@@ -1010,6 +1008,7 @@
 	boolean_t stalewriteverf = FALSE;
 	int i, npages = (bp->b_bcount + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	struct vm_page *pgs[npages];
+	struct proc *p = uiop->uio_procp;
 #ifndef NFS_V2_ONLY
 	boolean_t needcommit = TRUE; /* need only COMMIT RPC */
 #else
@@ -1102,7 +1101,7 @@
 			} else {
 				pushedrange = FALSE;
 			}
-			error = nfs_commit(vp, off, cnt, curproc);
+			error = nfs_commit(vp, off, cnt, p);
 			if (error == 0) {
 				if (pushedrange) {
 					nfs_merge_commit_ranges(vp);
@@ -1156,7 +1155,7 @@
 		if (np->n_pushhi - np->n_pushlo > nfs_commitsize) {
 			off = np->n_pushlo;
 			cnt = nfs_commitsize >> 1;
-			error = nfs_commit(vp, off, cnt, curproc);
+			error = nfs_commit(vp, off, cnt, p);
 			if (!error) {
 				nfs_add_committed_range(vp, off, cnt);
 				nfs_del_tobecommitted_range(vp, off, cnt);
@@ -1250,9 +1249,8 @@
  * synchronously or from an nfsiod.
  */
 int
-nfs_doio(bp, p)
+nfs_doio(bp)
 	struct buf *bp;
-	struct proc *p;
 {
 	int error;
 	struct uio uio;
Index: nfs_subs.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_subs.c,v
retrieving revision 1.149
diff -u -u -r1.149 nfs_subs.c
--- nfs_subs.c	29 May 2005 20:58:13 -0000	1.149
+++ nfs_subs.c	6 Jul 2005 23:20:01 -0000
@@ -834,7 +834,9 @@
 			if (uiop->uio_segflg == UIO_SYSSPACE)
 				memcpy(uiocp, mbufcp, xfer);
 			else
-				copyout(mbufcp, uiocp, xfer);
+				if ((error = copyout_proc(uiop->uio_procp,
+				    mbufcp, uiocp, xfer)) != 0)
+					return error;
 			left -= xfer;
 			len -= xfer;
 			mbufcp += xfer;
@@ -910,17 +912,19 @@
 				mlen = M_TRAILINGSPACE(mp);
 			}
 			xfer = (left > mlen) ? mlen : left;
+			cp = mtod(mp, caddr_t) + mp->m_len;
 #ifdef notdef
 			/* Not Yet.. */
 			if (uiop->uio_iov->iov_op != NULL)
-				(*(uiop->uio_iov->iov_op))
-				(uiocp, mtod(mp, caddr_t)+mp->m_len, xfer);
+				(*(uiop->uio_iov->iov_op))(uiocp, cp, xfer);
 			else
 #endif
 			if (uiop->uio_segflg == UIO_SYSSPACE)
-				memcpy(mtod(mp, caddr_t)+mp->m_len, uiocp, xfer);
+				(void)memcpy(cp, uiocp, xfer);
 			else
-				copyin(uiocp, mtod(mp, caddr_t)+mp->m_len, xfer);
+				/*XXX: Check error */
+				(void)copyin_proc(uiop->uio_procp, uiocp,
+				    cp, xfer);
 			mp->m_len += xfer;
 			left -= xfer;
 			uiocp += xfer;
@@ -939,7 +943,7 @@
 			mp->m_len = 0;
 			mp2->m_next = mp;
 		}
-		cp = mtod(mp, caddr_t)+mp->m_len;
+		cp = mtod(mp, caddr_t) + mp->m_len;
 		for (left = 0; left < rem; left++)
 			*cp++ = '\0';
 		mp->m_len += rem;
Index: nfs_syscalls.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_syscalls.c,v
retrieving revision 1.78
diff -u -u -r1.78 nfs_syscalls.c
--- nfs_syscalls.c	26 Feb 2005 22:39:50 -0000	1.78
+++ nfs_syscalls.c	6 Jul 2005 23:20:01 -0000
@@ -1086,7 +1086,7 @@
 				wakeup(&nmp->nm_bufq);
 			}
 			simple_unlock(&nmp->nm_slock);
-			(void) nfs_doio(bp, NULL);
+			(void)nfs_doio(bp);
 			simple_lock(&nmp->nm_slock);
 			/*
 			 * If there are more than one iod on this mount, then defect
Index: nfs_var.h
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_var.h,v
retrieving revision 1.50
diff -u -u -r1.50 nfs_var.h
--- nfs_var.h	29 May 2005 20:58:13 -0000	1.50
+++ nfs_var.h	6 Jul 2005 23:20:02 -0000
@@ -90,7 +90,7 @@
 int nfs_flushstalebuf __P((struct vnode *, struct ucred *, struct proc *, int));
 #define	NFS_FLUSHSTALEBUF_MYWRITE	1	/* assume writes are ours */
 int nfs_asyncio __P((struct buf *));
-int nfs_doio __P((struct buf *, struct proc *));
+int nfs_doio __P((struct buf *));
 
 /* nfs_boot.c */
 /* see nfsdiskless.h */
Index: nfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.222
diff -u -u -r1.222 nfs_vnops.c
--- nfs_vnops.c	29 May 2005 20:58:13 -0000	1.222
+++ nfs_vnops.c	6 Jul 2005 23:20:02 -0000
@@ -3269,25 +3269,18 @@
 {
 	struct vop_strategy_args *ap = v;
 	struct buf *bp = ap->a_bp;
-	struct proc *p;
 	int error = 0;
 
 	if ((bp->b_flags & (B_PHYS|B_ASYNC)) == (B_PHYS|B_ASYNC))
 		panic("nfs physio/async");
-	if (bp->b_flags & B_ASYNC)
-		p = NULL;
-	else
-		p = curproc;	/* XXX */
 
 	/*
 	 * If the op is asynchronous and an i/o daemon is waiting
 	 * queue the request, wake it up and wait for completion
 	 * otherwise just do it ourselves.
 	 */
-
-	if ((bp->b_flags & B_ASYNC) == 0 ||
-	    nfs_asyncio(bp))
-		error = nfs_doio(bp, p);
+	if ((bp->b_flags & B_ASYNC) == 0 || nfs_asyncio(bp))
+		error = nfs_doio(bp);
 	return (error);
 }