On Sun, Jul 24, 2005 at 07:24:23PM -0400, Trond Myklebust wrote:
> su den 24.07.2005 Klokka 19:09 (-0400) skreiv Trond Myklebust:
> > What non-power-of-two target? Anything _not_ aligned to a power of two
> > boundary is a BUG!

So we could simply replace the loop in nfs_block_bits() with call to
BUG()? :->

> Furthermore, rounding UP in order to "correct" this non-alignment would
> definitely be a bug.
> 
> If users choose to override the default rsize/wsize, that is almost
> always in order to limit the UDP fragmentation per read/write request on
> lossy networks. By rounding up, you are doubling the number of fragments
> that the user requested instead of respecting the limit.

OK.  Either way, this function can be cleaned up.  Apparently the Plan 9
filesystem guys thought it rounds up.

What's your opinion of the following patch, which explicitly states the
intent of nfs_block_bits()?  It still needs patch 1 and 2.

Thanks,
Rene



[PATCH 3/4] Simplify and rename nfs_block_bits() to rounddown_pow_of_two()

nfs_block_bits() doesn't have a lot to do with bits anymore.  It can
also be implemented simpler and clearer with fls().

Signed-off-by: Rene Scharfe <[EMAIL PROTECTED]>
---

 fs/nfs/inode.c |   21 +++++----------------
 1 files changed, 5 insertions(+), 16 deletions(-)

1388b63224334877b1b154e38ad9ee17f1726bca
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -185,20 +185,9 @@ nfs_umount_begin(struct super_block *sb)
                rpc_killall_tasks(rpc);
 }
 
-
-static inline unsigned long
-nfs_block_bits(unsigned long bsize)
+static inline unsigned long rounddown_pow_of_two(unsigned long x)
 {
-       /* make sure blocksize is a power of two */
-       if (bsize & (bsize - 1)) {
-               unsigned char   nrbits;
-
-               for (nrbits = 31; nrbits && !(bsize & (1 << nrbits)); nrbits--)
-                       ;
-               bsize = 1 << nrbits;
-       }
-
-       return bsize;
+       return x ? (1UL << (fls(x) - 1)) : 0;
 }
 
 /*
@@ -222,7 +211,7 @@ nfs_block_size(unsigned long bsize)
        else if (bsize >= NFS_MAX_FILE_IO_BUFFER_SIZE)
                bsize = NFS_MAX_FILE_IO_BUFFER_SIZE;
 
-       return nfs_block_bits(bsize);
+       return rounddown_pow_of_two(bsize);
 }
 
 /*
@@ -319,10 +308,10 @@ nfs_sb_init(struct super_block *sb, rpc_
        }
 
        if (sb->s_blocksize == 0) {
-               sb->s_blocksize = nfs_block_bits(server->wsize);
+               sb->s_blocksize = rounddown_pow_of_two(server->wsize);
                sb->s_blocksize_bits = fls(sb->s_blocksize - 1);
        }
-       server->wtmult = nfs_block_bits(fsinfo.wtmult);
+       server->wtmult = rounddown_pow_of_two(fsinfo.wtmult);
 
        server->dtsize = nfs_block_size(fsinfo.dtpref);
        if (server->dtsize > PAGE_CACHE_SIZE)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to