Date: Mon, 2 May 2011 15:43:52 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: Bruce Evans <brde@optusnet.com.au> Cc: rmacklem@FreeBSD.org, kib@FreeBSD.org, fs@FreeBSD.org Subject: Re: newnfs client and statfs Message-ID: <413547662.892660.1304365432183.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20110503013724.I2001@besplex.bde.org>
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.
>
Agreed. The "tmp" was left over from when I had "if (tmp > INT64_MAX)",
which I realized I didn't need. I can just change it to:
sbp->f_bavail = sfp->sf_abytes / NFS_FABLKSIZE;
> 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.
>
Well, since the RFCs don't say that, I think it shouldn't be assumed.
(I could assume that having the 63rd but set just means a server doesn't
know the exact answer and chooses to say "lots are free", but the truth
is, neither of us know.)
I've asked the question over on freebsd-fs@ and I'll wait to see what
everyone thinks w.r.t. RFC conformance vs hiding negative values in
the fields. If the collective agrees with you, I don't mind the code
assuming that 63rd bit set means negative.
> % + 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:
>
same as above, since the RFC says they're unsigned, I think that's what
the client should assume.
rick
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?413547662.892660.1304365432183.JavaMail.root>