From owner-freebsd-fs@FreeBSD.ORG Wed May 4 08:19:34 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 7D2301065670 for ; Wed, 4 May 2011 08:19:34 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 132D28FC08 for ; Wed, 4 May 2011 08:19:33 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p448IJ2t070852 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 4 May 2011 11:18:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id p448IJRA087944; Wed, 4 May 2011 11:18:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id p448IJSP087943; Wed, 4 May 2011 11:18:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Wed, 4 May 2011 11:18:19 +0300 From: Kostik Belousov To: Rick Macklem Message-ID: <20110504081819.GM48734@deviant.kiev.zoral.com.ua> References: <20110503174200.V1050@besplex.bde.org> <2143699515.968680.1304468141505.JavaMail.root@erie.cs.uoguelph.ca> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ttU9AGiyjxxpUEJf" Content-Disposition: inline In-Reply-To: <2143699515.968680.1304468141505.JavaMail.root@erie.cs.uoguelph.ca> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: 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: Wed, 04 May 2011 08:19:34 -0000 --ttU9AGiyjxxpUEJf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, May 03, 2011 at 08:15:41PM -0400, Rick Macklem wrote: > > On Mon, 2 May 2011, Rick Macklem wrote: > >=20 > > > I have attached a version of the patch that I intend to commit > > > unless it doesn't work for Kostik's test case. Kostik, could > > > you please test this one. > > > > > > Yes, Bruce, I realize you won't like it, but I > > > have put some comments in it > > > to try and clarify why it is coded the way it is. > > > (The arithmetic seems to work the way I would expect it to for > > > i386, which is the only arch I have for testing.) > >=20 > > Sigh. > >=20 > > % --- fs/nfsclient/nfs_clport.c.sav 2011-04-30 20:16:39.000000000 > > -0400 > > % +++ fs/nfsclient/nfs_clport.c 2011-05-02 19:32:31.000000000 -0400 > > % @@ -838,21 +838,33 @@ void > > % nfscl_loadsbinfo(struct nfsmount *nmp, struct nfsstatfs *sfp, void > > *statfs) > > % { > > % struct statfs *sbp =3D (struct statfs *)statfs; > > % - nfsquad_t tquad; > > % > > % if (nmp->nm_flag & (NFSMNT_NFSV3 | NFSMNT_NFSV4)) { > > % sbp->f_bsize =3D NFS_FABLKSIZE; > > % - tquad.qval =3D sfp->sf_tbytes; > > % - sbp->f_blocks =3D (long)(tquad.qval / ((u_quad_t)NFS_FABLKSIZE)); > > % - tquad.qval =3D sfp->sf_fbytes; > > % - sbp->f_bfree =3D (long)(tquad.qval / ((u_quad_t)NFS_FABLKSIZE)); > > % - tquad.qval =3D sfp->sf_abytes; > > % - sbp->f_bavail =3D (long)(tquad.qval / ((u_quad_t)NFS_FABLKSIZE)); > > % - tquad.qval =3D sfp->sf_tfiles; > > % - sbp->f_files =3D (tquad.lval[0] & 0x7fffffff); > > % - tquad.qval =3D sfp->sf_ffiles; > > % - sbp->f_ffree =3D (tquad.lval[0] & 0x7fffffff); > > % + sbp->f_blocks =3D sfp->sf_tbytes / NFS_FABLKSIZE; > > % + sbp->f_bfree =3D sfp->sf_fbytes / NFS_FABLKSIZE; > > % + /* > > % + * Although sf_abytes is uint64_t and f_bavail is int64_t, > > % + * the value after dividing by NFS_FABLKSIZE is small > > % + * enough that it will fit in 63bits, so it is ok to > > % + * assign it to f_bavail without fear that it will become > > % + * negative. > > % + */ > > % + sbp->f_bavail =3D sfp->sf_abytes / NFS_FABLKSIZE; > > % + sbp->f_files =3D sfp->sf_tfiles; > > % + /* Since f_ffree is int64_t, clip it to 63bits. */ > > % + if (sfp->sf_ffiles > (uint64_t)INT64_MAX) > >=20 > > This cast has no effect. INT64_MAX has type int64_t. sf_ffiles has > > uint64_t. The default binary promotions cause both types to be > > promoted > > to the minimally larger common type. This type is uint64_t. Thus > > INT64_MAX is converted automatically to the correct type. > >=20 > Yea, I didn't tthink the cast mattered and it didn't affect the outcome > for my little userland test program, so I'll take it out. (I was trying > the "play it safe", but if you say it doesn't matter, I believe you.) >=20 > > % + sbp->f_ffree =3D INT64_MAX; > > % + else > > % + sbp->f_ffree =3D sfp->sf_ffiles; > > % } else if ((nmp->nm_flag & NFSMNT_NFSV4) =3D=3D 0) { > > % + /* > > % + * The type casts to (int32_t) ensure that this code is > > % + * compatible with the old NFS client, in that it will > > % + * sign extend a value with bit31 set. This may or may > > % + * not be correct for NFSv2, but since it is a legacy > > % + * environment, I'd rather retain backwards compatibility. > > % + */ > > % sbp->f_bsize =3D (int32_t)sfp->sf_bsize; > > % sbp->f_blocks =3D (int32_t)sfp->sf_blocks; > > % sbp->f_bfree =3D (int32_t)sfp->sf_bfree; > >=20 > > It won't sign extend, but will propagate bit31 as an unsigned bit. For > > example, sfp->sf_blocks =3D 0x80000000 becomes sbp->f_blocks =3D > > 0xFFFFFFFF80000000, which is massively different. Again, omitting the > > cast gives the correct result if the wire insists on its values being > > unsigned. > >=20 > Ok, I'll change the comment. >=20 > > The result is only backwards compatible with relatively recent FreeBSD > > nfs clients. All FreeBSD clients are completely broken if bit31 is > > set, and compatibility with this brokenness is not useful (but as I > > pointed out in another reply, we would never have seen the broken case > > when the old clients weren't old, since it takes a server file system > > size of about 32TB for bit 31 to be set). > Well, the last legitimate use of the FreeBSD NFSv2 client was a diskless > root fs stored on a non-FreeBSD NFS server (because pxeboot didn't know t= he > correct file handle size). Since this is now fixed, there really isn't > any use for the NFSv2 client, as far as I know. Given that and the fact > that no one is complaining about it being broken, I feel it should just > be left alone. (Or remain "bug compatible" with the regular NFS client, > if you prefer.) >=20 > I'm afraid I have other things to work on and just don't see changing > NFSv2 (a 1985 protocol superceded by NFSv3 in 1994) a priority, rick. Rick, so any final version of the final patch to (re-)test ? --ttU9AGiyjxxpUEJf Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAk3BC8oACgkQC3+MBN1Mb4jluQCfdYUpvlQVu1lY+zV/KsWyr97Q QCcAnisrVqXE3UdiXE8KiGKoigmYk4zR =ld0I -----END PGP SIGNATURE----- --ttU9AGiyjxxpUEJf--