On Thu, 2020-09-10 at 20:06 +0000, Frank van der Linden wrote:
> nfs_statfs rounds up the numbers of blocks as computed
> from the numbers the server return values.
>
> This works out well if the client block size, which is
> the same as wrsize, is smaller than or equal to the actual
> filesystem block size on the server.
>
> But, for NFS4, the size is usually larger (1M), meaning
> that statfs reports more free space than actually is
> available. This confuses, for example, fstest generic/103.
>
> Given a choice between reporting too much or too little
> space, the latter is the safer option, so don't round
> up the number of blocks. This also simplifies the code.
>
> Signed-off-by: Frank van der Linden <fllinden@xxxxxxxxxx>
> ---
> fs/nfs/super.c | 25 ++++---------------------
> 1 file changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 7a70287f21a2..45d368a5cc95 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -217,8 +217,6 @@ EXPORT_SYMBOL_GPL(nfs_client_for_each_server);
> int nfs_statfs(struct dentry *dentry, struct kstatfs *buf)
> {
> struct nfs_server *server = NFS_SB(dentry->d_sb);
> - unsigned char blockbits;
> - unsigned long blockres;
> struct nfs_fh *fh = NFS_FH(d_inode(dentry));
> struct nfs_fsstat res;
> int error = -ENOMEM;
> @@ -241,26 +239,11 @@ int nfs_statfs(struct dentry *dentry, struct
> kstatfs *buf)
>
> buf->f_type = NFS_SUPER_MAGIC;
>
> - /*
> - * Current versions of glibc do not correctly handle the
> - * case where f_frsize != f_bsize. Eventually we want to
> - * report the value of wtmult in this field.
> - */
> - buf->f_frsize = dentry->d_sb->s_blocksize;
NACK. This comment and explicit setting is there to document why we're
not propagating the true value of the filesystem block size. Please do
not remove it.
> -
> - /*
> - * On most *nix systems, f_blocks, f_bfree, and f_bavail
> - * are reported in units of f_frsize. Linux hasn't had
> - * an f_frsize field in its statfs struct until recently,
> - * thus historically Linux's sys_statfs reports these
> - * fields in units of f_bsize.
> - */
> buf->f_bsize = dentry->d_sb->s_blocksize;
> - blockbits = dentry->d_sb->s_blocksize_bits;
> - blockres = (1 << blockbits) - 1;
> - buf->f_blocks = (res.tbytes + blockres) >> blockbits;
> - buf->f_bfree = (res.fbytes + blockres) >> blockbits;
> - buf->f_bavail = (res.abytes + blockres) >> blockbits;
> +
> + buf->f_blocks = res.tbytes / buf->f_bsize;
> + buf->f_bfree = res.fbytes / buf->f_bsize;
> + buf->f_bavail = res.abytes / buf->f_bsize;
>
> buf->f_files = res.tfiles;
> buf->f_ffree = res.afiles;
As far as I can tell, all we're doing here is changing rounding up to
rounding down, since dentry->d_sb->s_blocksize should always equal to
(1 << dentry->d_sb->s_blocksize_bits).
Otherwise, switching from a shift to division seems like it is just
making the calculation less efficient. Why?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
