From owner-freebsd-fs@FreeBSD.ORG Mon May 2 16:09:26 2011 Return-Path: Delivered-To: fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3A218106566C; Mon, 2 May 2011 16:09:26 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id B68508FC1B; Mon, 2 May 2011 16:09:24 +0000 (UTC) Received: from c122-106-155-58.carlnfd1.nsw.optusnet.com.au (c122-106-155-58.carlnfd1.nsw.optusnet.com.au [122.106.155.58]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id p42G9J5P005602 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 3 May 2011 02:09:21 +1000 Date: Tue, 3 May 2011 02:09:19 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Rick Macklem In-Reply-To: <733531363.835298.1304281643548.JavaMail.root@erie.cs.uoguelph.ca> Message-ID: <20110503013724.I2001@besplex.bde.org> References: <733531363.835298.1304281643548.JavaMail.root@erie.cs.uoguelph.ca> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: rmacklem@FreeBSD.org, kib@FreeBSD.org, fs@FreeBSD.org Subject: Re: newnfs client and statfs X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 May 2011 16:09:26 -0000 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