Date: Mon, 24 Feb 2003 15:44:24 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Terry Lambert <tlambert2@mindspring.com> Cc: David Syphers <dsyphers@uchicago.edu>, Kirk McKusick <mckusick@beastie.mckusick.com>, <freebsd-current@FreeBSD.ORG> Subject: Re: BOOT2_UFS=UFS1_ONLY works for today's current Message-ID: <20030224144742.L5105-100000@gamplex.bde.org> In-Reply-To: <3E5975D0.417C88A@mindspring.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 23 Feb 2003, Terry Lambert wrote:
> David Syphers wrote:
> > Okay, I've verified that the problem is due to rev. 1.39 of
> > /usr/src/sys/ufs/ffs/fs.h. Peter Wemm pointed out that the problem is not the
> > commit, but gcc's bad handling of 64-bit operations. Nonetheless, this commit
> > does break world for a lot of people... is there some official solution? The
> > make.conf line only works for UFS1 - if it's set to UFS2, buildworld still
> > fails. (Am I correct in assuming a 5.0-R install defaults to UFS2?)
>
> Yes. And 4.x upgrades, which are far more common, default to UFS.
>
> Personally, I think the changes should be #ifdef'ed the current
> version of GCC; when GCC rev's, hopefully its 64 bit operations
> handling will have improved.
This would wrong, since ufs2 depends on the changes to actually work
for file systems larger than about 1TB.
Blaming gcc's 64-bit operations seems to be wrong anyway. gcc actually
has good handling of (32, 32) -> 64 bit operations which are the only
types involved here in at least fs.h, boot2 still fits for me when it
is compiled the previous version of gcc. It has 19 bytes to spare:
%%%
text data bss dec hex filename
512 0 0 512 200 obj-UFS1_AND_UFS2/boot1.o
512 0 0 512 200 obj-UFS1_AND_UFS2/boot1.out
5228 25 2120 7373 1ccd obj-UFS1_AND_UFS2/boot2.o
5439 25 2196 7660 1dec obj-UFS1_AND_UFS2/boot2.out
91 0 0 91 5b obj-UFS1_AND_UFS2/sio.o
512 0 0 512 200 obj-UFS1_ONLY/boot1.o
512 0 0 512 200 obj-UFS1_ONLY/boot1.out
4999 25 1864 6888 1ae8 obj-UFS1_ONLY/boot2.o
5211 25 1940 7176 1c08 obj-UFS1_ONLY/boot2.out
91 0 0 91 5b obj-UFS1_ONLY/sio.o
512 0 0 512 200 obj-UFS2_ONLY/boot1.o
512 0 0 512 200 obj-UFS2_ONLY/boot1.out
5046 25 1992 7063 1b97 obj-UFS2_ONLY/boot2.o
5255 25 2068 7348 1cb4 obj-UFS2_ONLY/boot2.out
91 0 0 91 5b obj-UFS2_ONLY/sio.o
%%%
The fixes in fs.h are:
% Index: fs.h
% ===================================================================
% RCS file: /home/ncvs/src/sys/ufs/ffs/fs.h,v
% retrieving revision 1.38
% retrieving revision 1.39
% diff -u -1 -r1.38 -r1.39
% --- fs.h 10 Jan 2003 06:59:34 -0000 1.38
% +++ fs.h 22 Feb 2003 00:19:26 -0000 1.39
% @@ -33,3 +33,3 @@
% * @(#)fs.h 8.13 (Berkeley) 3/21/95
% - * $FreeBSD: src/sys/ufs/ffs/fs.h,v 1.38 2003/01/10 06:59:34 marcel Exp $
% + * $FreeBSD: src/sys/ufs/ffs/fs.h,v 1.39 2003/02/22 00:19:26 mckusick Exp $
% */
% @@ -498,3 +498,3 @@
% */
% -#define cgbase(fs, c) ((ufs2_daddr_t)((fs)->fs_fpg * (c)))
% +#define cgbase(fs, c) (((ufs2_daddr_t)(fs)->fs_fpg) * (c))
% #define cgdmin(fs, c) (cgstart(fs, c) + (fs)->fs_dblkno) /* 1st data */
This changes a (32, 32) -> 32 bit (possibly overflowing) operation to a
(32, 32) -> 64 bit (never overflowing) operation. The 1386 hardware
already does the wider operation so all gcc has to do is not discard the
high 32-bits, which it does reasonably well.
% @@ -543,5 +543,5 @@
% #define lfragtosize(fs, frag) /* calculates ((off_t)frag * fs->fs_fsize) */ \
% - ((off_t)(frag) << (fs)->fs_fshift)
% + (((off_t)(frag)) << (fs)->fs_fshift)
% #define lblktosize(fs, blk) /* calculates ((off_t)blk * fs->fs_bsize) */ \
% - ((off_t)(blk) << (fs)->fs_bshift)
% + (((off_t)(blk)) << (fs)->fs_bshift)
% /* Use this only when `blk' is known to be small, e.g., < NDADDR. */
These changes have no effect except to add style bugs (see below).
The casts were inserted in the correct place in rev.1.7 to fix similar
overflow bugs for _files_ larger than 2GB. Now we're fixing overflow
for block numbers larger than 2G.
% @@ -573,3 +573,3 @@
% (fs)->fs_cstotal.cs_nffree - \
% - ((off_t)((fs)->fs_dsize) * (percentreserved) / 100))
% + (((off_t)((fs)->fs_dsize)) * (percentreserved) / 100))
%
This change has no effect for many reasons:
- it just adds style bugs (see below).
- the macro is not used in the boot blocks.
- fs_dsize already happens to have the same type as off_t (both have type
int64_t). off_t is a bogus type to cast to anyway. We start with a
block count and multiply by a percentage. This is unrelated to file
sizes, but overflow happens to be prevented by the maxiumum percentage
(100) being smaller than the minimum block size (4096).
All of these changes add style bugs IMO. Except for the change to
cgbase(), they add redundant parentheses around "(cast)(foo)->bar",
but properly parenthesized macros are hard enough to read with only
non-redundant parentheses. The change to cgbase() moves parentheses
so that they are redundant instead of just wrong.
Bruce
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?20030224144742.L5105-100000>