From owner-freebsd-current Mon Feb 4 8:30:22 2002 Delivered-To: freebsd-current@freebsd.org Received: from wall.polstra.com (wall-gw.polstra.com [206.213.73.130]) by hub.freebsd.org (Postfix) with ESMTP id 42C4437B427 for ; Mon, 4 Feb 2002 08:30:11 -0800 (PST) Received: from vashon.polstra.com (vashon.polstra.com [206.213.73.13]) by wall.polstra.com (8.11.3/8.11.3) with ESMTP id g14GU5M46816; Mon, 4 Feb 2002 08:30:05 -0800 (PST) (envelope-from jdp@wall.polstra.com) Received: (from jdp@localhost) by vashon.polstra.com (8.11.6/8.11.0) id g14GU3P01712; Mon, 4 Feb 2002 08:30:03 -0800 (PST) (envelope-from jdp) Date: Mon, 4 Feb 2002 08:30:03 -0800 (PST) Message-Id: <200202041630.g14GU3P01712@vashon.polstra.com> To: current@freebsd.org From: John Polstra Cc: bright@mu.org Subject: Re: Panics in ffs_clusteracct with todays -current In-Reply-To: <20020204080336.A95852@elvis.mu.org> References: <20020205000829.A22758-100000@gamplex.bde.org> <20020204080336.A95852@elvis.mu.org> Organization: Polstra & Co., Seattle, WA Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG In article <20020204080336.A95852@elvis.mu.org>, Alfred Perlstein wrote: > * Bruce Evans [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