From owner-freebsd-fs Tue Dec 17 16:55:35 2002 Delivered-To: freebsd-fs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8ED7637B401 for ; Tue, 17 Dec 2002 16:55:32 -0800 (PST) Received: from beastie.mckusick.com (beastie.mckusick.com [209.31.233.184]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0480443EC2 for ; Tue, 17 Dec 2002 16:55:32 -0800 (PST) (envelope-from mckusick@beastie.mckusick.com) Received: from beastie.mckusick.com (localhost [127.0.0.1]) by beastie.mckusick.com (8.12.3/8.12.3) with ESMTP id gBI0tB59018624; Tue, 17 Dec 2002 16:55:12 -0800 (PST) (envelope-from mckusick@beastie.mckusick.com) Message-Id: <200212180055.gBI0tB59018624@beastie.mckusick.com> To: Bruce Evans Subject: Re:panic: ffs_vfree: range: dev = ad4s1c, ino = -1690809896, fs = /mnt2 Cc: Julian Elischer , Kris Kennaway , Robert Watson , fs@FreeBSD.ORG In-Reply-To: Your message of "Fri, 06 Dec 2002 20:53:55 +1100." <20021206201556.B8366-100000@gamplex.bde.org> Date: Tue, 17 Dec 2002 16:55:11 -0800 From: Kirk McKusick Sender: owner-freebsd-fs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org Now that the 6.0 trunk has defrosted, I have made your suggested change. Kirk McKusick =-=-=-=-= Date: Fri, 6 Dec 2002 20:53:55 +1100 (EST) From: Bruce Evans To: Julian Elischer cc: Kirk McKusick , Kris Kennaway , Robert Watson , Subject: Re: panic: ffs_vfree: range: dev = ad4s1c, ino = -1690809896, fs = /mnt2 In-Reply-To: X-ASK-Info: Whitelist match On Thu, 5 Dec 2002, Julian Elischer wrote: > This is one of the reasons that I think such fields should all be > defined as unsigned to start with.. This is actually an example of why naive conversion to use unsigned is usually wrong. The bug seems to have nothing to do with signedness, except for a bug in the debugging message. > > Well it is not at all clear how the dirpref routine came up with > > such an out of whack inode preference (2604157400 when the filesystem > > has only 3538944 inodes), but the following fix should catch it and > > make it harmless. I have submitted the patch to release engineering. > > > > Kirk McKusick > > > > =-=-=-=-= > > > > Index: ffs_alloc.c > > =================================================================== > > RCS file: /usr/ncvs/src/sys/ufs/ffs/ffs_alloc.c,v > > retrieving revision 1.102 > > diff -c -r1.102 ffs_alloc.c > > *** ffs_alloc.c 2002/09/19 03:55:30 1.102 > > --- ffs_alloc.c 2002/12/06 01:15:50 > > *************** > > *** 841,847 **** > > ipref = ffs_dirpref(pip); > > else > > ipref = pip->i_number; > > ! if (ipref >= fs->fs_ncg * fs->fs_ipg) > > ipref = 0; > > cg = ino_to_cg(fs, ipref); > > /* > > --- 841,847 ---- > > ipref = ffs_dirpref(pip); > > else > > ipref = pip->i_number; > > ! if ((unsigned)ipref >= fs->fs_ncg * fs->fs_ipg) > > ipref = 0; > > cg = ino_to_cg(fs, ipref); > > /* > > This change has no effect on any supported system, since `ipref' is already an unsigned type (ino_t == uint32_t), which is in fact identical with unsigned on all supported systems. The analysis is slightly more complicated on exotic systems. Many things are broken on systems with ints smaller than 32 bits, and the change increases the breakage by clipping the LHS (if the filesystem has more than UINT_MAX inodes). The change causes at most slightly different signed-versus unsigned comparision warnings on systems with ints larger than 32 bits. The RHS multiplies 2 int32_t's and the multiplication may in theory overflow, but newfs presumably won't create filesystems for which it overflows (ones with more than 2^31-1 inodes), so there should be no signedness problems in practice. Related cosmetic bugs: (1) `unsigned' is not correctly misspelled u_int. (2) the original panic message has a wrong function name and a wrong printf format for printing ino_t's. Te following patch fixes (2) and a nearby non-misspelling of `unsigned' (the only other one in ffs_alloc.c). %%% Index: ffs_alloc.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_alloc.c,v retrieving revision 1.103 diff -u -2 -r1.103 ffs_alloc.c --- ffs_alloc.c 6 Dec 2002 02:08:46 -0000 1.103 +++ ffs_alloc.c 6 Dec 2002 09:13:26 -0000 @@ -1649,5 +1683,5 @@ */ static int -ffs_isfreeblock(struct fs *fs, unsigned char *cp, ufs1_daddr_t h) +ffs_isfreeblock(struct fs *fs, u_char *cp, ufs1_daddr_t h) { @@ -1897,6 +1931,6 @@ } if ((u_int)ino >= fs->fs_ipg * fs->fs_ncg) - panic("ffs_vfree: range: dev = %s, ino = %d, fs = %s", - devtoname(dev), ino, fs->fs_fsmnt); + panic("ffs_freefile: range: dev = %s, ino = %lu, fs = %s", + devtoname(dev), (u_long)ino, fs->fs_fsmnt); if ((error = bread(devvp, cgbno, (int)fs->fs_cgsize, NOCRED, &bp))) { brelse(bp); @@ -1916,5 +1950,5 @@ (u_long)ino + cg * fs->fs_ipg, fs->fs_fsmnt); if (fs->fs_ronly == 0) - panic("ffs_vfree: freeing free inode"); + panic("ffs_freefile: freeing free inode"); } clrbit(inosused, ino); %%% Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-fs" in the body of the message