Date: Mon, 4 Feb 2002 08:30:03 -0800 (PST) From: John Polstra <jdp@polstra.com> To: current@freebsd.org Cc: bright@mu.org Subject: Re: Panics in ffs_clusteracct with todays -current Message-ID: <200202041630.g14GU3P01712@vashon.polstra.com> In-Reply-To: <20020204080336.A95852@elvis.mu.org> References: <XFMail.20020203205249.jdp@polstra.com> <20020205000829.A22758-100000@gamplex.bde.org> <20020204080336.A95852@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In article <20020204080336.A95852@elvis.mu.org>,
Alfred Perlstein <bright@mu.org> wrote:
> * Bruce Evans <bde@zeta.org.au> [020204 05:26] wrote:
> > This was broken by a recent change to the type of NBBY.
> >
> > `start' is apparently negative (it would be for blkno == 0). Small
> > negative values of `start' used to give an index of 0 in freemapp (not
> > even small negative, since integer division bogusly rounds negative
> > values towards 0). They now give an index of about 0x1fffffff on
> > 32-bit machines.
> >
> > I also got panics in vm. The vm code apparently trapped while trying
> > to map the preposterous address generated by the above.
> >
> > This patch just backs out the change to NBBY.
>
> Wouldn't this make more sense:
>
> Index: ffs/ffs_alloc.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_alloc.c,v
> retrieving revision 1.86
> diff -u -r1.86 ffs_alloc.c
> --- ffs/ffs_alloc.c 2 Feb 2002 01:42:44 -0000 1.86
> +++ ffs/ffs_alloc.c 4 Feb 2002 16:08:34 -0000
> @@ -1790,7 +1790,7 @@
> end = start + fs->fs_contigsumsize;
> if (end >= cgp->cg_nclusterblks)
> end = cgp->cg_nclusterblks;
> - mapp = &freemapp[start / NBBY];
> + mapp = &freemapp[start < 0 ? 0 : start / NBBY];
> map = *mapp++;
> bit = 1 << (start % NBBY);
> for (i = start; i < end; i++) {
I think your change makes sense here, but I also think the change
to NBBY should be backed out. In general it is very dangerous to
change constants to explicitly unsigned. There's no reason why NBBY
shouldn't be used in signed contexts, but making it unsigned promotes
all of the other ints in containing expressions to unsigned as well
(on the i386). NBBY is used in lots of code, including 3rd party
applications. It has a long tradition, and now isn't the time to
change its type. There's no reason for it to be explicitly unsigned.
The constant 8 is every bit as positive as the constant 8U. :-)
John
--
John Polstra
John D. Polstra & Co., Inc. Seattle, Washington USA
"Disappointment is a good sign of basic intelligence." -- Chögyam Trungpa
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?200202041630.g14GU3P01712>