Date: Wed, 18 Dec 2002 15:12:14 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Kirk McKusick <mckusick@beastie.mckusick.com> Cc: Julian Elischer <julian@elischer.org>, Kris Kennaway <kris@obsecurity.org>, Robert Watson <rwatson@tislabs.com>, <fs@FreeBSD.ORG> Subject: Re:panic: ffs_vfree: range: dev = ad4s1c, ino = -1690809896, fs = /mnt2 Message-ID: <20021218145929.S23372-100000@gamplex.bde.org> In-Reply-To: <200212180055.gBI0tB59018624@beastie.mckusick.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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 <bde@zeta.org.au>
> To: Julian Elischer <julian@elischer.org>
> cc: Kirk McKusick <mckusick@beastie.mckusick.com>,
> Kris Kennaway <kris@obsecurity.org>, Robert Watson <rwatson@tislabs.com>,
> <fs@FreeBSD.ORG>
> Subject: Re: panic: ffs_vfree: range: dev = ad4s1c, ino = -1690809896, fs =
> /mnt2
> In-Reply-To: <Pine.BSF.4.21.0212052133000.38887-100000@InterJet.elischer.org>
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021218145929.S23372-100000>