Subject: Re: softdep panic (PR/16670)
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Chuck Silvers <chuq@chuq.com>
List: tech-kern
Date: 10/13/2002 22:04:19
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


On Mon, Oct 14, 2002 at 05:51:20AM +0900, YAMAMOTO Takashi wrote:
> hi.
> 
> is attached patch ok?
> it updates the vnode's v_size even if uiomove returns an error.
> otherwise, ffs_sync(genfs_gop_write) doesn't flush dependencies for
> the last block of the file.
> for more details of the problem, please see PR/16670.
> thanks.
> 
> YAMAMOTO Takashi

> 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/13 20:38:22
> @@ -309,6 +309,8 @@ WRITE(void *v)
>  
>  	ubc_alloc_flags = UBC_WRITE;
>  	while (uio->uio_resid > 0) {
> +		off_t newsize;
> +
>  		oldoff = uio->uio_offset;
>  		blkoffset = blkoff(fs, uio->uio_offset);
>  		bytelen = MIN(fs->fs_bsize - blkoffset, uio->uio_resid);
> @@ -348,17 +350,19 @@ WRITE(void *v)
>  		    ubc_alloc_flags);
>  		error = uiomove(win, bytelen, uio);
>  		ubc_release(win, 0);
> -		if (error) {
> -			break;
> -		}
>  
>  		/*
>  		 * update UVM's notion of the size now that we've
>  		 * copied the data into the vnode's pages.
>  		 */
>  
> -		if (vp->v_size < uio->uio_offset) {
> -			uvm_vnp_setsize(vp, uio->uio_offset);
> +		newsize = oldoff + bytelen;
> +		if (vp->v_size < newsize) {
> +			uvm_vnp_setsize(vp, newsize);
> +		}
> +
> +		if (error) {
> +			break;
>  		}
>  
>  		/*