bin/56606: df cannot handle 2TB NFS volumes
Bruce Evans
bde at zeta.org.au
Tue Sep 9 10:20:15 PDT 2003
The following reply was made to PR bin/56606; it has been noted by GNATS.
From: Bruce Evans <bde at zeta.org.au>
To: Hendrik Scholz <hendrik at scholz.net>
Cc: FreeBSD-gnats-submit at freebsd.org, freebsd-bugs at freebsd.org,
peter at 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 <sys/cdefs.h>
% -__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
More information about the freebsd-bugs
mailing list