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--