Date: Wed, 23 Feb 2000 18:27:06 -0800 (PST) From: Matthew Dillon <dillon@apollo.backplane.com> To: Ian Dowse <iedowse@maths.tcd.ie> Cc: nops@maths.tcd.ie, freebsd-current@FreeBSD.ORG, diablo-list@list.bart.nl Subject: Re: ffs_blkfree: freeing free block (was Re: Panic (pmap)) Message-ID: <200002240227.SAA33299@apollo.backplane.com> References: <200002232235.aa22335@salmon.maths.tcd.ie>
next in thread | previous in thread | raw e-mail | index | archive | help
: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?
-Matt
Matthew Dillon
<dillon@backplane.com>
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 02:25:53
@@ -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,26 @@
ffs_blkfree(ip, *blkp, fs->fs_bsize);
deallocated += fs->fs_bsize;
}
- if (allocib != NULL)
+ if (allocib != NULL) {
*allocib = 0;
+ } else if (unwindidx >= 0) {
+ error = bread(vp,
+ indirs[unwindidx].in_lbn, (int)fs->fs_bsize, NOCRED, &bp);
+ if (error) {
+ 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?200002240227.SAA33299>