From owner-freebsd-fs Tue Dec 17 20:11:57 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 394C937B401 for ; Tue, 17 Dec 2002 20:11:54 -0800 (PST) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id C0D7F43E4A for ; Tue, 17 Dec 2002 20:11:52 -0800 (PST) (envelope-from bde@zeta.org.au) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id PAA06314; Wed, 18 Dec 2002 15:11:10 +1100 Date: Wed, 18 Dec 2002 15:12:14 +1100 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Kirk McKusick Cc: Julian Elischer , Kris Kennaway , Robert Watson , Subject: Re:panic: ffs_vfree: range: dev = ad4s1c, ino = -1690809896, fs = /mnt2 In-Reply-To: <200212180055.gBI0tB59018624@beastie.mckusick.com> Message-ID: <20021218145929.S23372-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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. Thanks. Is the original problem now understood? Bruce > 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