Date: Wed, 23 Feb 2000 22:55:06 -0800 From: Alfred Perlstein <bright@wintelcom.net> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: Ian Dowse <iedowse@maths.tcd.ie>, mckusick@freebsd.org, current@freebsd.org Subject: Re: ffs_blkfree: freeing free block (was Re: Panic (pmap)) Message-ID: <20000223225505.A21720@fw.wintelcom.net> In-Reply-To: <200002240556.VAA34592@apollo.backplane.com>; from dillon@apollo.backplane.com on Wed, Feb 23, 2000 at 09:56:35PM -0800 References: <200002232235.aa22335@salmon.maths.tcd.ie> <200002240227.SAA33299@apollo.backplane.com> <200002240556.VAA34592@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Er, shouldn't you guys let Kirk know, not only is he "MAINTAINER", but
pretty much mostly "CREATOR" :)
Kirk, another issue with running out of space in FFS seems to have
come up:
* Matthew Dillon <dillon@apollo.backplane.com> [000223 18:57] wrote:
> :Ian Dowse <iedowse@maths.tcd.ie> wrote:
> :I think I've found it. Here's an easy way to repeat the problem to
> :start with:
> :
> : mount_mfs -T fd1440 none /mnt
> : cd /mnt
> : dd if=/dev/zero bs=4k of=test seek=1036 count=1
> : dd if=/dev/zero bs=4k of=test1 count=1
> : dd if=/dev/zero bs=4k of=test2
> : rm test1
> : dd if=/dev/zero bs=4k of=test seek=8000 count=1
> : echo > test
> :
> :It looks as if the problem is in ffs_balloc(), and occurs as follows:
> : - ffs_balloc() is asked to allocate a doubly indirect block.
> : - The first-level indirection block already exists
> : - The second-level indirection block does not exist, but is
> : successfully allocated.
> : - This block is linked into the first-level indirection block by
> : the line:
> :
> : bap[indirs[i - 1].in_off] = nb;
> :
> : - Allocation of the data block fails.
> : - All allocated blocks are then released, but there is still
> : a link in the first-level indirection block to what is
> : now a free block.
> :
> :The fix should be relatively straightforward - either the code should
> :avoid linking new indirection blocks until all allocations succeed,
> :or it should back out the changes on failure.
> :
> :Ian
>
> Great detective work! The 'allocib' variable is used to unwind the
> 'first' new indirect block allocation, but it is *only* initialized
> for the Level 0 block. So the unwinding only works when the Level 0
> indirect block had to be allocated. When the Level 0 indirect block
> is valid and a deeper level indirect block had to be allocated,
> allocib is not initialized and the association is not unwound even
> though the deeper indirect block is freed when the data block allocation
> fails.
>
> Unfortunately, we can't simply initialize 'allocib' here, because it
> would point into the middle of a buffer. The fix is thus not entirely
> trivial.
>
> This is what I think we need to do, but THIS PATCH IS NOT TESTED AND
> COULD BE COMPLETELY WRONG. Any filesystem gurus out there who can look
> at this and tell me if it's correct?
[patch snipped, Matt just re-issued a fixed version]
* Matthew Dillon <dillon@apollo.backplane.com> [000223 22:26] wrote:
>
> Oops, here's a new version of my patch. This one survives Ian's test
> script. I was improperly blowing away an 'error' field.
>
> -Matt
>
> Index: ffs_balloc.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_balloc.c,v
> retrieving revision 1.25
> diff -u -r1.25 ffs_balloc.c
> --- ffs_balloc.c 2000/01/11 08:27:00 1.25
> +++ ffs_balloc.c 2000/02/24 05:44:59
> @@ -77,6 +77,7 @@
> ufs_daddr_t newb, *bap, pref;
> int deallocated, osize, nsize, num, i, error;
> ufs_daddr_t *allocib, *blkp, *allocblk, allociblk[NIADDR + 1];
> + int unwindidx = -1;
> struct proc *p = curproc; /* XXX */
>
> vp = ap->a_vp;
> @@ -272,6 +273,8 @@
> }
> }
> bap[indirs[i - 1].in_off] = nb;
> + if (allocib == NULL && unwindidx < 0)
> + unwindidx = i - 1;
> /*
> * If required, write synchronously, otherwise use
> * delayed write.
> @@ -349,8 +352,28 @@
> ffs_blkfree(ip, *blkp, fs->fs_bsize);
> deallocated += fs->fs_bsize;
> }
> - if (allocib != NULL)
> + if (allocib != NULL) {
> *allocib = 0;
> + } else if (unwindidx >= 0) {
> + int r;
> +
> + r = bread(vp, indirs[unwindidx].in_lbn,
> + (int)fs->fs_bsize, NOCRED, &bp);
> + if (r) {
> + panic("Could not unwind indirect block, error %d", error);
> + brelse(bp);
> + } else {
> + bap = (ufs_daddr_t *)bp->b_data;
> + bap[indirs[unwindidx].in_off] = 0;
> + if (flags & B_SYNC) {
> + bwrite(bp);
> + } else {
> + if (bp->b_bufsize == fs->fs_bsize)
> + bp->b_flags |= B_CLUSTEROK;
> + bdwrite(bp);
> + }
> + }
> + }
> if (deallocated) {
> #ifdef QUOTA
> /*
>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000223225505.A21720>