Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
On Tue, Jul 26, 2005 at 01:48:46PM -0400, Trond Myklebust wrote:
> I really don't like the choice of name. If you feel you must change the
> name, then make it something like nfs_blocksize_align(). That describes
> its function, instead of the implementation details.
Yes, rounddown_pow_of_two() belongs in kernel.h next to
roundup_pow_of_two(). And maybe it should get a shorter name.
Anyway, I also don't like "nfs_blocksize_align". So let's simply keep
the old name. Renaming can be done later if really needed.
Rene
[PATCH 3/3] Simplify nfs_block_bits()
Signed-off-by: Rene Scharfe <[EMAIL PROTECTED]>
---
fs/nfs/inode.c | 12 ++--
1 files changed, 2 insertions(+), 10 deletions(-)
ddad5eadf4c2907842bf9baa2610e0a35ea14137
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -189,16 +189,8 @@ nfs_umount_begin(struct super_block *sb)
static inline unsigned long
nfs_block_bits(unsigned long bsize)
{
- /* 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;
+ /* round down to the nearest power of two */
+ return bsize ? (1UL << (fls(bsize) - 1)) : 0;
}
/*
-
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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
Trond Myklebust wrote: må den 25.07.2005 Klokka 17:56 (+0200) skreiv Rene Scharfe: What's your opinion of the following patch, which explicitly states the intent of nfs_block_bits()? It still needs patch 1 and 2. I really don't like the choice of name. If you feel you must change the name, then make it something like nfs_blocksize_align(). That describes its function, instead of the implementation details. I would agree. Was there really a driving requirement to change the name? Thanx... ps - 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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
må den 25.07.2005 Klokka 17:56 (+0200) skreiv Rene Scharfe: > What's your opinion of the following patch, which explicitly states the > intent of nfs_block_bits()? It still needs patch 1 and 2. I really don't like the choice of name. If you feel you must change the name, then make it something like nfs_blocksize_align(). That describes its function, instead of the implementation details. Cheers, Trond - 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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
må den 25.07.2005 Klokka 17:56 (+0200) skreiv Rene Scharfe: What's your opinion of the following patch, which explicitly states the intent of nfs_block_bits()? It still needs patch 1 and 2. I really don't like the choice of name. If you feel you must change the name, then make it something like nfs_blocksize_align(). That describes its function, instead of the implementation details. Cheers, Trond - 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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
Trond Myklebust wrote: må den 25.07.2005 Klokka 17:56 (+0200) skreiv Rene Scharfe: What's your opinion of the following patch, which explicitly states the intent of nfs_block_bits()? It still needs patch 1 and 2. I really don't like the choice of name. If you feel you must change the name, then make it something like nfs_blocksize_align(). That describes its function, instead of the implementation details. I would agree. Was there really a driving requirement to change the name? Thanx... ps - 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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
On Tue, Jul 26, 2005 at 01:48:46PM -0400, Trond Myklebust wrote:
I really don't like the choice of name. If you feel you must change the
name, then make it something like nfs_blocksize_align(). That describes
its function, instead of the implementation details.
Yes, rounddown_pow_of_two() belongs in kernel.h next to
roundup_pow_of_two(). And maybe it should get a shorter name.
Anyway, I also don't like nfs_blocksize_align. So let's simply keep
the old name. Renaming can be done later if really needed.
Rene
[PATCH 3/3] Simplify nfs_block_bits()
Signed-off-by: Rene Scharfe [EMAIL PROTECTED]
---
fs/nfs/inode.c | 12 ++--
1 files changed, 2 insertions(+), 10 deletions(-)
ddad5eadf4c2907842bf9baa2610e0a35ea14137
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -189,16 +189,8 @@ nfs_umount_begin(struct super_block *sb)
static inline unsigned long
nfs_block_bits(unsigned long bsize)
{
- /* 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;
+ /* round down to the nearest power of two */
+ return bsize ? (1UL (fls(bsize) - 1)) : 0;
}
/*
-
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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
su den 24.07.2005 Klokka 19:09 (-0400) skreiv Trond Myklebust: > su den 24.07.2005 Klokka 16:36 (+0200) skreiv Rene Scharfe: > > [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two() > > > > Function nfs_block_bits() an open-coded version of (the non-existing) > > rounddown_pow_of_two(). That means that for non-power-of-two target > > sizes it returns half the size needed for a block to fully contain > > the target. I guess this is wrong. :-) The patch uses the built-in > > roundup_pow_of_two() instead. > > What non-power-of-two target? Anything _not_ aligned to a power of two > boundary is a 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. Cheers, Trond - 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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
su den 24.07.2005 Klokka 16:36 (+0200) skreiv Rene Scharfe: > [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two() > > Function nfs_block_bits() an open-coded version of (the non-existing) > rounddown_pow_of_two(). That means that for non-power-of-two target > sizes it returns half the size needed for a block to fully contain > the target. I guess this is wrong. :-) The patch uses the built-in > roundup_pow_of_two() instead. What non-power-of-two target? Anything _not_ aligned to a power of two boundary is a BUG! Cheers, Trond - 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/
[PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
[PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
Function nfs_block_bits() an open-coded version of (the non-existing)
rounddown_pow_of_two(). That means that for non-power-of-two target
sizes it returns half the size needed for a block to fully contain
the target. I guess this is wrong. :-) The patch uses the built-in
roundup_pow_of_two() instead.
Signed-off-by: Rene Scharfe <[EMAIL PROTECTED]>
---
fs/nfs/inode.c | 22 +++---
1 files changed, 3 insertions(+), 19 deletions(-)
4130722d1eeb5eb22c38df9f09dfa6be554bc72c
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -185,22 +185,6 @@ nfs_umount_begin(struct super_block *sb)
rpc_killall_tasks(rpc);
}
-
-static inline unsigned long
-nfs_block_bits(unsigned long bsize)
-{
- /* 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;
-}
-
/*
* Calculate the number of 512byte blocks used.
*/
@@ -222,7 +206,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 roundup_pow_of_two(bsize);
}
/*
@@ -319,10 +303,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 = roundup_pow_of_two(server->wsize);
sb->s_blocksize_bits = fls(sb->s_blocksize - 1);
}
- server->wtmult = nfs_block_bits(fsinfo.wtmult);
+ server->wtmult = roundup_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/
[PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
[PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
Function nfs_block_bits() an open-coded version of (the non-existing)
rounddown_pow_of_two(). That means that for non-power-of-two target
sizes it returns half the size needed for a block to fully contain
the target. I guess this is wrong. :-) The patch uses the built-in
roundup_pow_of_two() instead.
Signed-off-by: Rene Scharfe [EMAIL PROTECTED]
---
fs/nfs/inode.c | 22 +++---
1 files changed, 3 insertions(+), 19 deletions(-)
4130722d1eeb5eb22c38df9f09dfa6be554bc72c
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -185,22 +185,6 @@ nfs_umount_begin(struct super_block *sb)
rpc_killall_tasks(rpc);
}
-
-static inline unsigned long
-nfs_block_bits(unsigned long bsize)
-{
- /* 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;
-}
-
/*
* Calculate the number of 512byte blocks used.
*/
@@ -222,7 +206,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 roundup_pow_of_two(bsize);
}
/*
@@ -319,10 +303,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 = roundup_pow_of_two(server-wsize);
sb-s_blocksize_bits = fls(sb-s_blocksize - 1);
}
- server-wtmult = nfs_block_bits(fsinfo.wtmult);
+ server-wtmult = roundup_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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
su den 24.07.2005 Klokka 16:36 (+0200) skreiv Rene Scharfe: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two() Function nfs_block_bits() an open-coded version of (the non-existing) rounddown_pow_of_two(). That means that for non-power-of-two target sizes it returns half the size needed for a block to fully contain the target. I guess this is wrong. :-) The patch uses the built-in roundup_pow_of_two() instead. What non-power-of-two target? Anything _not_ aligned to a power of two boundary is a BUG! Cheers, Trond - 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/
Re: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two()
su den 24.07.2005 Klokka 19:09 (-0400) skreiv Trond Myklebust: su den 24.07.2005 Klokka 16:36 (+0200) skreiv Rene Scharfe: [PATCH NFS 3/3] Replace nfs_block_bits() with roundup_pow_of_two() Function nfs_block_bits() an open-coded version of (the non-existing) rounddown_pow_of_two(). That means that for non-power-of-two target sizes it returns half the size needed for a block to fully contain the target. I guess this is wrong. :-) The patch uses the built-in roundup_pow_of_two() instead. What non-power-of-two target? Anything _not_ aligned to a power of two boundary is a 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. Cheers, Trond - 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/
