nfs fstat st_blocks overreporting

nfs fstat st_blocks overreporting

[Frank Cusack]

While you know I disagree that s_blocksize should be wtmult (ie, it
is wtmult?wtmult:512 and I think it should be MAX(rsize,wsize)), in
any event the blocks used reporting is incorrect in that it assumes
a 512 byte blocksize.

I don't know why blockbits is an unsigned char instead of an int; I
just copied it from a similar block of code in fs/nfs/inode.c.

--- 26t7.1/fs/nfs/inode.c	2003-10-13 06:59:34.000000000 -0700
+++ 26t7.2/fs/nfs/inode.c	2003-10-13 07:50:34.000000000 -0700
@@ -207,13 +207,17 @@ nfs_block_bits(unsigned long bsize, unsi
 }
 
 /*
- * Calculate the number of 512byte blocks used.
+ * Calculate the number of 2^blockbits -byte blocks used.
  */
 static inline unsigned long
-nfs_calc_block_size(u64 tsize)
+nfs_calc_blocks_used(u64 tsize, unsigned char blockbits)
 {
-	loff_t used = (tsize + 511) >> 9;
-	return (used > ULONG_MAX) ? ULONG_MAX : used;
+	unsigned long blockres;
+	loff_t used;
+
+	blockres = (1 << blockbits) - 1;
+	used = (tsize + blockres) >> blockbits;
+	return (used > ULONG_MAX) ? ULONG_MAX : used;
 }
 
 /*
@@ -762,9 +766,9 @@ nfs_fhget(struct super_block *sb, struct
 		inode->i_gid = fattr->gid;
 		if (fattr->valid & (NFS_ATTR_FATTR_V3 | NFS_ATTR_FATTR_V4)) {
 			/*
-			 * report the blocks in 512byte units
+			 * report the blocks in s_blocksize units
 			 */
-			inode->i_blocks = nfs_calc_block_size(fattr->du.nfs3.used);
+			inode->i_blocks = nfs_calc_blocks_used(fattr->du.nfs3.used, inode->i_sb->s_blocksize_bits);
 			inode->i_blksize = inode->i_sb->s_blocksize;
 		} else {
 			inode->i_blocks = fattr->du.nfs2.blocks;
@@ -1181,9 +1185,10 @@ __nfs_refresh_inode(struct inode *inode,
 
 	if (fattr->valid & (NFS_ATTR_FATTR_V3 | NFS_ATTR_FATTR_V4)) {
 		/*
-		 * report the blocks in 512byte units
+		 * report the blocks in s_blocksize units
 		 */
-		inode->i_blocks = nfs_calc_block_size(fattr->du.nfs3.used);
+		inode->i_blocks = nfs_calc_blocks_used(fattr->du.nfs3.used,
+					inode->i_sb->s_blocksize_bits);
 		inode->i_blksize = inode->i_sb->s_blocksize;
  	} else {
  		inode->i_blocks = fattr->du.nfs2.blocks;

nfs fstat st_blocks overreporting

[Trond Myklebust]

>>>>> " " == Frank Cusack <fcusack@fcusack.com> writes:

     > While you know I disagree that s_blocksize should be wtmult
     > (ie, it is wtmult?wtmult:512 and I think it should be
     > MAX(rsize,wsize)), in any event the blocks used reporting is
     > incorrect in that it assumes a 512 byte blocksize.

Frank,

     man 2 stat

  i_blocks a.k.a. stat->st_blocks is _defined_ to be in 512byte
units. It bears no relation to the st_blksize. If you don't like that,
spec then by all means appeal to the POSIX committee that wrote
it. (You probably wouldn't be the first to do so)

  Pending the outcome of such an appeal, though, the patch must be
rejected...

Cheers,
  Trond

Re: nfs fstat st_blocks overreporting

[Linus Torvalds]

On Mon, 13 Oct 2003, Frank Cusack wrote:
>
> While you know I disagree that s_blocksize should be wtmult (ie, it
> is wtmult?wtmult:512 and I think it should be MAX(rsize,wsize)), in
> any event the blocks used reporting is incorrect in that it assumes
> a 512 byte blocksize.

I agree with you that st_blocksize should very probably be different (in 
_no_ case has it got anything to do with just "wtmult", since the thing 
is used for reading too). However, the st_blocks calculations has 
_nothing_ to do with st_blocksize.

st_blocksize is "this is the preferred blocking for IO".

st_blocks is "this is how many 512-byte blocks the file has".

The names may be similar, but they have nothing in common. They are in
totally different namespaces - one is in bytes, the other one in units of
"traditional UNIX block size", aka sector, aka 512 bytes. Trying to mix 
the two is doomed.

		Linus

Re: nfs fstat st_blocks overreporting

[Frank Cusack]

On Mon, Oct 13, 2003 at 11:29:26AM -0400, Trond Myklebust wrote:
> >>>>> " " == Frank Cusack <fcusack@fcusack.com> writes:
> 
>      > While you know I disagree that s_blocksize should be wtmult
>      > (ie, it is wtmult?wtmult:512 and I think it should be
>      > MAX(rsize,wsize)), in any event the blocks used reporting is
>      > incorrect in that it assumes a 512 byte blocksize.
> 
> Frank,
> 
>      man 2 stat
> 
>   i_blocks a.k.a. stat->st_blocks is _defined_ to be in 512byte
> units. It bears no relation to the st_blksize. If you don't like that,
> spec then by all means appeal to the POSIX committee that wrote
> it. (You probably wouldn't be the first to do so)
> 
>   Pending the outcome of such an appeal, though, the patch must be
> rejected...

I'm an idiot.

/fc