From owner-freebsd-bugs@FreeBSD.ORG Tue Sep 9 10:20:15 2003 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 98D4516A4BF for ; Tue, 9 Sep 2003 10:20:15 -0700 (PDT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 06D3A43FAF for ; Tue, 9 Sep 2003 10:20:15 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.9/8.12.9) with ESMTP id h89HKEUp067711 for ; Tue, 9 Sep 2003 10:20:14 -0700 (PDT) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.9/8.12.9/Submit) id h89HKEEe067710; Tue, 9 Sep 2003 10:20:14 -0700 (PDT) Date: Tue, 9 Sep 2003 10:20:14 -0700 (PDT) Message-Id: <200309091720.h89HKEEe067710@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Bruce Evans Subject: Re: bin/56606: df cannot handle 2TB NFS volumes X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Bruce Evans List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Sep 2003 17:20:15 -0000 The following reply was made to PR bin/56606; it has been noted by GNATS. From: Bruce Evans To: Hendrik Scholz Cc: FreeBSD-gnats-submit@freebsd.org, freebsd-bugs@freebsd.org, peter@freebsd.org Subject: Re: bin/56606: df cannot handle 2TB NFS volumes Date: Wed, 10 Sep 2003 03:15:07 +1000 (EST) On Mon, 8 Sep 2003, Hendrik Scholz wrote: > >Description: > Mounting a 1.8TB NFS volume shows negative values: > $ \df /mnt/foo > Filesystem 1K-blocks Used Avail Capacity Mounted on > x.x.x.x:/spool1 -222502944 -463076980 240574036 208% /mnt/foo > $ mount | grep foo > x.x.x.x:/spool1 on /mnt/foo (nfs) > > It should read (copied from Linux box): > x.x.x.x:/spool1 1924980704 1684406656 240574048 88% /news/spool1 I think this was supposed to have been fixed in nfs_vfsops.c nfs_vfsops.c, but the fix was buggy and was turned into verbose nonsense that happens to have no effect in rev.1.135. I sent the following to the author of the fix 3 weeks ago but received no reply. It contains a hopefully correct fix. This has not been tested with multi-TB filesystems but hasn't caused any problems with ones in the 4-13 GB range. [Old mail] % Index: nfs_vfsops.c % =================================================================== % RCS file: /home/ncvs/src/sys/nfsclient/nfs_vfsops.c,v % retrieving revision 1.134 % retrieving revision 1.135 % diff -u -2 -r1.134 -r1.135 % --- nfs_vfsops.c 29 Apr 2003 13:36:04 -0000 1.134 % +++ nfs_vfsops.c 19 May 2003 22:35:00 -0000 1.135 % @@ -38,5 +38,5 @@ % % #include % -__FBSDID("$FreeBSD: src/sys/nfsclient/nfs_vfsops.c,v 1.134 2003/04/29 13:36:04 kan Exp $"); % +__FBSDID("$FreeBSD: src/sys/nfsclient/nfs_vfsops.c,v 1.135 2003/05/19 22:35:00 peter Exp $"); % % #include "opt_bootp.h" % @@ -278,13 +278,16 @@ % sbp->f_bsize = bsize; % tquad = fxdr_hyper(&sfp->sf_tbytes); % - if ((tquad / bsize) > LONG_MAX) % + if (((long)(tquad / bsize) > LONG_MAX) || % + ((long)(tquad / bsize) < LONG_MIN)) % continue; % sbp->f_blocks = tquad / bsize; % tquad = fxdr_hyper(&sfp->sf_fbytes); % - if ((tquad / bsize) > LONG_MAX) % + if (((long)(tquad / bsize) > LONG_MAX) || % + ((long)(tquad / bsize) < LONG_MIN)) % continue; % sbp->f_bfree = tquad / bsize; % tquad = fxdr_hyper(&sfp->sf_abytes); % - if ((tquad / bsize) > LONG_MAX) % + if (((long)(tquad / bsize) > LONG_MAX) || % + ((long)(tquad / bsize) < LONG_MIN)) % continue; % sbp->f_bavail = tquad / bsize; Values of type long have the remarkable property of always being between LONG_MIN and LONG_MAX, so this change is just a verbose way of backing out rev.1.33. Attempted fix: %%% Index: nfs_vfsops.c =================================================================== RCS file: /home/ncvs/src/sys/nfsclient/nfs_vfsops.c,v retrieving revision 1.136 diff -u -2 -r1.136 nfs_vfsops.c --- nfs_vfsops.c 12 Jun 2003 20:48:38 -0000 1.136 +++ nfs_vfsops.c 16 Aug 2003 04:01:37 -0000 @@ -239,5 +239,5 @@ struct mbuf *mreq, *mrep, *md, *mb; struct nfsnode *np; - u_quad_t tquad; + quad_t tquad; int bsize; @@ -270,19 +270,19 @@ for (bsize = NFS_FABLKSIZE; ; bsize *= 2) { sbp->f_bsize = bsize; - tquad = fxdr_hyper(&sfp->sf_tbytes); - if (((long)(tquad / bsize) > LONG_MAX) || - ((long)(tquad / bsize) < LONG_MIN)) + tquad = (quad_t)fxdr_hyper(&sfp->sf_tbytes) / bsize; + if (bsize <= INT_MAX / 2 && + (tquad > LONG_MAX || tquad < LONG_MIN)) continue; - sbp->f_blocks = tquad / bsize; - tquad = fxdr_hyper(&sfp->sf_fbytes); - if (((long)(tquad / bsize) > LONG_MAX) || - ((long)(tquad / bsize) < LONG_MIN)) + sbp->f_blocks = tquad; + tquad = (quad_t)fxdr_hyper(&sfp->sf_fbytes) / bsize; + if (bsize <= INT_MAX / 2 && + (tquad > LONG_MAX || tquad < LONG_MIN)) continue; - sbp->f_bfree = tquad / bsize; - tquad = fxdr_hyper(&sfp->sf_abytes); - if (((long)(tquad / bsize) > LONG_MAX) || - ((long)(tquad / bsize) < LONG_MIN)) + sbp->f_bfree = tquad; + tquad = (quad_t)fxdr_hyper(&sfp->sf_abytes) / bsize; + if (bsize <= INT_MAX / 2 && + (tquad > LONG_MAX || tquad < LONG_MIN)) continue; - sbp->f_bavail = tquad / bsize; + sbp->f_bavail = tquad; sbp->f_files = (fxdr_unsigned(int32_t, sfp->sf_tfiles.nfsuquad[1]) & 0x7fffffff); %%% This has not been tested at runtime. Rev.1.33 apparently didn't work essentially because of overflow (-1 became UQUAD_MAX, and dividing that by bsize made a mess). The patch assumes that values larger than QUAD_MAX really mean negative values. The patch also ensures that doubling bsize never overflows. bsize could be limited to a less preposterous size than INT_MAX. The patch also fixes some style bugs (excessive parentheses). It's interesting that gcc doesn't warn about this. It now seems to warn only about null comparisons of chars with their limits. I think it recently expanded this warning from one involving only one-sided limits of chars. ISTR looking at the code for this more than 10 years ago. It could have warned about all sorts of null comparisons but intentially only gave warnings for a couple of cases to limit the warnings. Hmm, there are more sloppy conversions here: % sbp->f_files = (fxdr_unsigned(int32_t, % sfp->sf_tfiles.nfsuquad[1]) & 0x7fffffff); % sbp->f_ffree = (fxdr_unsigned(int32_t, % sfp->sf_ffiles.nfsuquad[1]) & 0x7fffffff); Looks like it blindly truncates to a 31-bit (positive) int. f_ffree could easily be >= 2^31. Truncating to 2^31-1 or a documented weird (negative) value meaning "overflow" would be better. [End of old mail] Bruce