Subject: Re: softdep panic (PR/16670)
To: None <chuq@chuq.com>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 10/14/2002 18:25:51
--NextPart-20021014182448-0034100
Content-Type: Text/Plain; charset=us-ascii

> hi,
> 
> the problem with this fix is that it opens a window where another thread
> could see uninitialized pages in the region of the file that uiomove()
> failed to write to.  however, unwinding the softdep state at this point
> is no doubt more hassle than it's worth, so what we need to do is just
> make sure that the kernel initializes the pages if uiomove() didn't
> do it.  just zeroing the range that uiomove() would have filled is fine.
> ideally we could use a fault-tolerant kzero() function (similar to kcopy()),
> but that doesn't exist.  in practice we'll never fault here because we just
> allocated the pages right before this, so memset() is good enough for now.
> 
> -Chuck

i see..
then how about attached one?
thanks.

YAMAMOTO Takashi

--NextPart-20021014182448-0034100
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="softdep2.diff"

? softdep2.diff
Index: ufs_readwrite.c
===================================================================
RCS file: /cvs/NetBSD/syssrc/sys/ufs/ufs/ufs_readwrite.c,v
retrieving revision 1.42
diff -u -p -r1.42 ufs_readwrite.c
--- ufs_readwrite.c	2002/03/25 02:23:56	1.42
+++ ufs_readwrite.c	2002/10/14 09:21:14
@@ -309,6 +309,9 @@ WRITE(void *v)
 
 	ubc_alloc_flags = UBC_WRITE;
 	while (uio->uio_resid > 0) {
+		boolean_t extending; /* if we're extending a whole block */
+		off_t newoff;
+
 		oldoff = uio->uio_offset;
 		blkoffset = blkoff(fs, uio->uio_offset);
 		bytelen = MIN(fs->fs_bsize - blkoffset, uio->uio_resid);
@@ -320,9 +323,10 @@ WRITE(void *v)
 		 * since the new blocks will be inaccessible until the write
 		 * is complete.
 		 */
+		extending = uio->uio_offset >= preallocoff &&
+		    uio->uio_offset < endallocoff;
 
-		if (uio->uio_offset < preallocoff ||
-		    uio->uio_offset >= endallocoff) {
+		if (!extending) {
 			error = ufs_balloc_range(vp, uio->uio_offset, bytelen,
 			    cred, aflag);
 			if (error) {
@@ -347,18 +351,31 @@ WRITE(void *v)
 		win = ubc_alloc(&vp->v_uobj, uio->uio_offset, &bytelen,
 		    ubc_alloc_flags);
 		error = uiomove(win, bytelen, uio);
-		ubc_release(win, 0);
-		if (error) {
-			break;
+		if (error && extending) {
+			/*
+			 * if we haven't initialized the pages yet,
+			 * do it now.  it's safe to use memset here
+			 * because we just mapped the pages above.
+			 */
+			memset(win, 0, bytelen);
 		}
+		ubc_release(win, 0);
 
 		/*
 		 * update UVM's notion of the size now that we've
 		 * copied the data into the vnode's pages.
+		 *
+		 * we should update the size even when uiomove failed.
+		 * otherwise ffs_truncate can't flush soft update states.
 		 */
 
-		if (vp->v_size < uio->uio_offset) {
-			uvm_vnp_setsize(vp, uio->uio_offset);
+		newoff = oldoff + bytelen;
+		if (vp->v_size < newoff) {
+			uvm_vnp_setsize(vp, newoff);
+		}
+
+		if (error) {
+			break;
 		}
 
 		/*

--NextPart-20021014182448-0034100--