Re: BOOT2_UFS=UFS1_ONLY works for today's current
On Sun, 23 Feb 2003, Terry Lambert wrote: > Bruce Evans wrote: > > > 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. > > If it's not going to compile correctly, it's not going to work. That is a feature :-). > What we are bitching about here is that someone wants to use > 64 bit operations that GCC can't handle. Waving our hands isn't > going to make the problem go away, and adding code that doesn't > compile correctly with the current compiler won't, either. I doubt that it is a problem with 64-bitness. Handling 64 bits just takes more code, and the macro is apparently used a lot (it is used deeply nested so it is not obvious how often it is expanded). I suspect the problem is more from increased register pressure that happens to afect the current compiler more. > > 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. > > So it is agreed that this is good? I hope so. The result doesn't always fit in 32 bits when the file system size exceeds about 1TB. Discarding nonzero high bits probably causes corrupt file systems. > OK... so what should be done? Just the cgbase() change? Does > this fix the compiler revision dependent problem in such a way > that the code does not need to be conditionalized on the compiler > revision to avoid being broken? Kirk committed a quick fix. The bug is unlikely to matter in boot code. Root file systems shouldn't be larger than 1TB. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
Bruce Evans wrote: > > 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. If it's not going to compile correctly, it's not going to work. What we are bitching about here is that someone wants to use 64 bit operations that GCC can't handle. Waving our hands isn't going to make the problem go away, and adding code that doesn't compile correctly with the current compiler won't, either. > 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: So we're agreed: it's a GCC issue, which is resolved by downgrading the compiler, or by upgrading it, after the new version is fixed (which is the direction I suggested going). > % -#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. So it is agreed that this is good? [ ... rest of patches == "style bugs" ... ] > 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. OK... so what should be done? Just the cgbase() change? Does this fix the compiler revision dependent problem in such a way that the code does not need to be conditionalized on the compiler revision to avoid being broken? -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
From: David Syphers <[EMAIL PROTECTED]> To: Kirk McKusick <[EMAIL PROTECTED]> Subject: Re: BOOT2_UFS=UFS1_ONLY works for today's current Date: Sun, 23 Feb 2003 14:49:52 -0600 Cc: [EMAIL PROTECTED] On Sunday 23 February 2003 11:10 am, Richard Arends wrote: > On Sun, 23 Feb 2003, David Syphers wrote: > > I added BOOT2_UFS=UFS2_ONLY to my make.conf, and my buildworld still > > dies in boot2. I'm trying to upgrade from a Feb. 19 -current > > (because it's crashing all the time, and I need to enable debugging > > stuff). Is there a fix, or would other information be helpful? > > Same problem over here. I reverted back the last commit on > /usr/src/sys/ufs/ffs/fs.h in my source tree and that "fixed" the > build. Of course, this is a workaround !! 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?) -David -- http://www.seektruth.org Astronomy and Astrophysics Center The University of Chicago I have committed the following "fix" which reverts to using the previous broken version of cgbase in ufsread.c. It will work fine provided that your filesystem is smaller than 1.5Tb. Kirk McKusick Index: ufsread.c === RCS file: /usr/ncvs/src/sys/boot/common/ufsread.c,v retrieving revision 1.9 diff -c -r1.9 ufsread.c *** ufsread.c 2002/12/14 19:39:44 1.9 --- ufsread.c 2003/02/24 04:44:50 *** *** 28,33 --- 28,35 #include #include + #undef cgbase + #define cgbase(fs, c) ((ufs2_daddr_t)((fs)->fs_fpg * (c))) /* * We use 4k `virtual' blocks for filesystem data, whatever the actual To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
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: %%% textdata 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 25212073731ccd obj-UFS1_AND_UFS2/boot2.o 5439 25219676601dec 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 25186468881ae8 obj-UFS1_ONLY/boot2.o 5211 25194071761c08 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 25199270631b97 obj-UFS2_ONLY/boot2.o 5255 25206873481cb4 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 - 1.38 % +++ fs.h 22 Feb 2003 00:19:26 - 1.39 % @@ -33,3 +33,3 @@ % * @(#)fs.h8.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 [EMAIL PROTECTED] with "unsubscribe freeb
Re: BOOT2_UFS=UFS1_ONLY works for today's current
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. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
On Sun, Feb 23, 2003 at 02:49:52PM -0600, David Syphers wrote: > fails. (Am I correct in assuming a 5.0-R install defaults to UFS2?) You are not correct. 5.0-R, and infact 5-CURRENT still default to ufs1. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
On Sunday 23 February 2003 11:10 am, Richard Arends wrote: > On Sun, 23 Feb 2003, David Syphers wrote: > > I added BOOT2_UFS=UFS2_ONLY to my make.conf, and my buildworld still dies > > in boot2. I'm trying to upgrade from a Feb. 19 -current (because it's > > crashing all the time, and I need to enable debugging stuff). Is there a > > fix, or would other information be helpful? > > Same problem over here. I reverted back the last commit on > /usr/src/sys/ufs/ffs/fs.h in my source tree and that "fixed" the build. Of > course, this is a workaround !! 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?) -David -- http://www.seektruth.org Astronomy and Astrophysics Center The University of Chicago To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
On Sun, 23 Feb 2003, David Syphers wrote: David, > I added BOOT2_UFS=UFS2_ONLY to my make.conf, and my buildworld still dies in > boot2. I'm trying to upgrade from a Feb. 19 -current (because it's crashing > all the time, and I need to enable debugging stuff). Is there a fix, or would > other information be helpful? Same problem over here. I reverted back the last commit on /usr/src/sys/ufs/ffs/fs.h in my source tree and that "fixed" the build. Of course, this is a workaround !! Regards, Richard. Paul Vixie in an interview with Sendmail.net: Now that the Internet has the full spectrum of humanity as users, the technology is showing its weakness: it was designed to be used by friendly, smart people. Spammers, as an example of a class, are neither friendly nor smart. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
Thank you for your info., Giorgos. BOOT2_UFS=UFS1_ONLY in /etc/make.conf made my buildworld OK. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
On Saturday 22 February 2003 08:55 pm, Giorgos Keramidas wrote: > Just in case anyone tries to build today's current and sees > it fail because of boot2, you can always set BOOT2_UFS=UFS1_ONLY > or BOOT2_UFS=UFS2_ONLY in your make.conf and rebuild. I added BOOT2_UFS=UFS2_ONLY to my make.conf, and my buildworld still dies in boot2. I'm trying to upgrade from a Feb. 19 -current (because it's crashing all the time, and I need to enable debugging stuff). Is there a fix, or would other information be helpful? -David -- http://www.seektruth.org Astronomy and Astrophysics Center The University of Chicago To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: BOOT2_UFS=UFS1_ONLY works for today's current
keramida> Just in case anyone tries to build today's current and sees keramida> it fail because of boot2, you can always set BOOT2_UFS=UFS1_ONLY keramida> or BOOT2_UFS=UFS2_ONLY in your make.conf and rebuild. It should work, but it can't be used for a release distribution:) -- - Makoto `MAR' Matsushita To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
BOOT2_UFS=UFS1_ONLY works for today's current
Just in case anyone tries to build today's current and sees it fail because of boot2, you can always set BOOT2_UFS=UFS1_ONLY or BOOT2_UFS=UFS2_ONLY in your make.conf and rebuild. Note that you should have at least one alternative boot method (floppy or CDROM) if you happen to accidentally use UFS1_ONLY on a ufs version-2 system or vice versa. - Giorgos To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
