Date: Tue, 3 May 2011 02:09:19 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: rmacklem@FreeBSD.org, kib@FreeBSD.org, fs@FreeBSD.org Subject: Re: newnfs client and statfs Message-ID: <20110503013724.I2001@besplex.bde.org> In-Reply-To: <733531363.835298.1304281643548.JavaMail.root@erie.cs.uoguelph.ca> References: <733531363.835298.1304281643548.JavaMail.root@erie.cs.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 1 May 2011, Rick Macklem wrote:
> Ok, I realized the code in the last post was pretty bogus:-) My only
> excuse was that I typed it as I was running out the door...
>
> So, I played with it a bit and the attached patch seems to work for
> i386. For the fields that are uint64_t in struct statfs, it just
> divides/assigns. For the int64_t field that takes the divided value
> (f_bavail) I did the division/assignment to a uint64_t tmp and then
> assigned that to f_bavail. (Since any value that fits in uint64_t is
> a positive value for int64_t after being divided by 2 or more, it will
> always be positive.) For the other int64_t one, I just check for "> INT64_MAX"
> and set it to INT64_MAX for that case, so it doesn't go negative.
Sorry, I don't like this. Going through tmp makes no difference since
all values are reduced below INT64_MAX by dividing by just 2. "Negative"
values are still converted to garbage positive values.
> Anyhow, the updated patch is attached and maybe kib@ can test it?
% --- fs/nfsclient/nfs_clport.c.sav 2011-04-30 20:16:39.000000000 -0400
% +++ fs/nfsclient/nfs_clport.c 2011-05-01 16:11:18.000000000 -0400
% @@ -838,20 +838,19 @@ void
% nfscl_loadsbinfo(struct nfsmount *nmp, struct nfsstatfs *sfp, void *statfs)
% {
% struct statfs *sbp = (struct statfs *)statfs;
% - nfsquad_t tquad;
% + uint64_t tmp;
%
% if (nmp->nm_flag & (NFSMNT_NFSV3 | NFSMNT_NFSV4)) {
% sbp->f_bsize = NFS_FABLKSIZE;
% - tquad.qval = sfp->sf_tbytes;
% - sbp->f_blocks = (long)(tquad.qval / ((u_quad_t)NFS_FABLKSIZE));
% - tquad.qval = sfp->sf_fbytes;
% - sbp->f_bfree = (long)(tquad.qval / ((u_quad_t)NFS_FABLKSIZE));
% - tquad.qval = sfp->sf_abytes;
% - sbp->f_bavail = (long)(tquad.qval / ((u_quad_t)NFS_FABLKSIZE));
% - tquad.qval = sfp->sf_tfiles;
% - sbp->f_files = (tquad.lval[0] & 0x7fffffff);
% - tquad.qval = sfp->sf_ffiles;
% - sbp->f_ffree = (tquad.lval[0] & 0x7fffffff);
% + sbp->f_blocks = sfp->sf_tbytes / NFS_FABLKSIZE;
% + sbp->f_bfree = sfp->sf_fbytes / NFS_FABLKSIZE;
OK.
% + tmp = sfp->sf_abytes / NFS_FABLKSIZE;
% + sbp->f_bavail = tmp;
The division made it less than 2**55, and kept it nonnegative. Going through
tmp doesn't change this.
But I still want to use my code to support negative values:
sbp->f_bavail = (int64_t)sfp->sf_abytes / NFS_FABLKSIZE;
If the 63rd bit is set, it must mean that the server is an
non-broken^non-conforming FreeBSD one trying to send a negative value,
since file systems with 2 >= 2**63 bytes available are physical impossible
Even if the file system is virtual and growable so that it has no real
limits, it should probably limit itself to much less than 2**63 to avoid
testing whether clients can handle such large values.
% + sbp->f_files = sfp->sf_tfiles;
% + if (sfp->sf_ffiles > INT64_MAX)
% + sbp->f_ffree = INT64_MAX;
% + else
% + sbp->f_ffree = sfp->sf_ffiles;
This gives correct-as-possible clamping for large unsigned values, but
gives a garbage large positive value for "negative" values. Again, negative
values are physically impossible, so if the 63rd bit is set then it must
mean that the server is a FreeBSD one trying to send a negative value.
So I prefer to use my (untested in this case code to support negative
values:
Sloppy version: just assign and depend on 2's complement magic that isn't
guaranteed to be there, and on the type sizes being the same:
sbp->f_ffree = sfp->sf_ffiles;
More careful version: first make sure that the 2's complement magic is there:
sbp->f_ffree = (int64_t)sfp->sf_ffiles;
% } else if ((nmp->nm_flag & NFSMNT_NFSV4) == 0) {
% sbp->f_bsize = (int32_t)sfp->sf_bsize;
% sbp->f_blocks = (int32_t)sfp->sf_blocks;
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110503013724.I2001>