Re: CVS commit: src/sys

[Jaromír Doleček <jaromir.dolecek%gmail.com@localhost>]

> On Fri, Nov 04, 2016 at 04:44:10PM +0100, J. Hannken-Illjes wrote:
>> - This change results in "panic: ffs_blkfree_common: freeing free block"
>>   if I put a file system under stress (*1).
>>
>> - I suppose not zeroing the blocks to be freed before freeing them
>>   makes the life of fsck harder.
>>
>> - Running "brelse(bp, BC_INVAL)" doesn't look OK.

The brelse(bp, BC_INVAL) call was there before as well, but the
condition changed and is wrong.

I can repeat the problem with your script and the packaged fsx (thanks
Thomas). Whipped up a patch to what looked wrong there, and it no
longer panics for me. Patch is attached. I'll test further and commit
it tomorrow.

Jaromir

Index: ffs_inode.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_inode.c,v
retrieving revision 1.118
diff -u -p -r1.118 ffs_inode.c
--- ffs_inode.c	28 Oct 2016 20:38:12 -0000	1.118
+++ ffs_inode.c	6 Nov 2016 23:09:15 -0000
@@ -675,18 +675,18 @@ ffs_indirtrunc(struct inode *ip, daddr_t
 	 * Recursively free blocks on the now last partial indirect block.
 	 */
 	if (level > SINGLE && lastbn >= 0) {
-		nb = RBAP(ip, last);
+		last = lastbn % factor;
+		nb = RBAP(ip, i);
 		if (nb != 0) {
 			error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb),
-					       lastbn % factor, level - 1,
-					       countp);
+					       last, level - 1, countp);
 			if (error)
 				goto out;
 		}
 	}
 
 out:
-	if (RBAP(ip, 0) == 0) {
+	if (lastbn < 0 && error == 0) {
 		/* all freed, release without writing back */
 		brelse(bp, BC_INVAL);
 	} else {