Use ilog2() in fs/namespace.c
PATCH 1/1 Use ilog2() in fs/namespace.c
We can use ilog2() in fs/namespace.c to compute hash_bits and hash_mask at compile time, not runtime.
fs/namespace.c +3 -18
@@ -25,6 +25,7 @@#include <linux/security.h> #include <linux/mount.h> #include <linux/ramfs.h> +#include <linux/log2.h> #include <asm/uaccess.h> #include <asm/unistd.h> #include "pnode.h"
@@ -36,7 +37,8 @@ __cacheline_aligned_in_smp DEFINE_SPINLOCK(vfsmount_lock);static int event;static struct list_head *mount_hashtable__read_mostly ; -static int hash_mask__read_mostly , hash_bits__read_mostly ; +#define hash_bits ilog2(PAGE_SIZE / sizeof(struct list_head)) +#define hash_mask ((1UL << hash_bits) - 1)static struct kmem_cache *mnt_cache__read_mostly ;static struct rw_semaphore namespace_sem;
@@ -1828,24 +1830,7 @@ void __init mnt_init(void)if (!mount_hashtable)panic ("Failed to allocate mount hash table\n" ); -/* -* Find the power-of-two list-heads that can fit into the allocation.. -* We don't guarantee that "sizeof(struct list_head)" is necessarily -* a power-of-two. -*/ - nr_hash =PAGE_SIZE /sizeof (struct list_head ); - hash_bits =0 ; -do { - hash_bits++; - }while ((nr_hash >> hash_bits) !=0 ); - hash_bits--; - -/* -* Re-calculate the actual number of entries and the mask -* from the number of bits we can fit. -*/ nr_hash =1UL << hash_bits; - hash_mask = nr_hash -1 ;printk ("Mount-cache hash table entries: %d\n" , nr_hash);
Quoted 2 lines · On Fri, 14 Dec 2007 18:51:06 +0100 Eric Dumazet <[email protected]> wrote:
We can use
ilog2()infs/namespace.cto computehash_bitsandhash_maskat compile time, not runtime.
Well noted.
Quoted 1 line
[namespace.patch text/plain (1.4KB)]
argh. (save-as, read, copy-paste, s/^/> /g)
Quoted 34 lines
fs/namespace.c +3
-11@@ -25,6 +25,7 @@#include <linux/security.h> #include <linux/mount.h> #include <linux/ramfs.h> +#include <linux/log2.h> #include <asm/uaccess.h> #include <asm/unistd.h> #include "pnode.h" @@ -36,7 +37,8 @@ __cacheline_aligned_in_smp DEFINE_SPINLOCK(vfsmount_lock);static int event;static struct list_head *mount_hashtable__read_mostly ; -static int hash_mask__read_mostly , hash_bits__read_mostly ; +#define hash_bits ilog2(PAGE_SIZE / sizeof(struct list_head)) +#define hash_mask ((1UL << hash_bits) - 1)static struct kmem_cache *mnt_cache__read_mostly ;static struct rw_semaphore namespace_sem;@@ -1828,24 +1830,7 @@ void __init mnt_init(void)if (!mount_hashtable)panic ("Failed to allocate mount hash table\n" ); -/* -* Find the power-of-two list-heads that can fit into the allocation.. -* We don't guarantee that "sizeof(struct list_head)" is necessarily -* a power-of-two. -*/ - nr_hash =PAGE_SIZE /sizeof (struct list_head ); - hash_bits =0 ; -do { - hash_bits++; - }while ((nr_hash >> hash_bits) !=0 );
fs/namespace.c void __init mnt_init(void) · line 1835
Quoted 10 lines
fs/namespace.c +0
-7- hash_bits--; - -/* -* Re-calculate the actual number of entries and the mask -* from the number of bits we can fit. -*/ nr_hash =1UL << hash_bits; - hash_mask = nr_hash -1 ;printk ("Mount-cache hash table entries: %d\n" , nr_hash);
Those #defines you now have there are foul. Please, when there's a choice between doing it minimally and doing it right, let's do it right?
Look what we can now do:
fs/namespace.c renamed from fs/namespace.c~use-ilog2-in-fs-namespacec-fix +9 -17
@@ -31,14 +31,15 @@#include "pnode.h" #include "internal.h" +#define HASH_SHIFT ilog2(PAGE_SIZE / sizeof(struct list_head)) +#define HASH_SIZE (1UL << HASH_SHIFT) +/* spinlock for vfsmount related operations, inplace of dcache_lock */ __cacheline_aligned_in_smp DEFINE_SPINLOCK (vfsmount_lock );static int event;static struct list_head *mount_hashtable__read_mostly ; -#define hash_bits ilog2(PAGE_SIZE / sizeof(struct list_head)) -#define hash_mask ((1UL << hash_bits) - 1)static struct kmem_cache *mnt_cache__read_mostly ;static struct rw_semaphore namespace_sem;
@@ -50,8 +51,8 @@ static inline unsigned long hash(struct {unsigned long tmp = ((unsigned long )mnt /L1_CACHE_BYTES ); tmp += ((unsigned long )dentry /L1_CACHE_BYTES ); - tmp = tmp + (tmp >> hash_bits); -return tmp & hash_mask; + tmp = tmp + (tmp >>HASH_SHIFT ); +return tmp & (HASH_SIZE -1 ); }struct vfsmount *alloc_vfsmnt (const char *name)
@@ -1815,9 +1816,7 @@ static void __init init_mount_tree(void)void __init mnt_init (void ) { -struct list_head *d; -unsigned int nr_hash; -int i; +unsigned u;int err;init_rwsem (&namespace_sem);
@@ -1830,18 +1829,11 @@ void __init mnt_init(void)if (!mount_hashtable)panic ("Failed to allocate mount hash table\n" ); - nr_hash =1UL << hash_bits; +printk ("Mount-cache hash table entries: %lu\n" ,HASH_SIZE ); -printk ("Mount-cache hash table entries: %d\n" , nr_hash); +for (u =0 ; u <HASH_SIZE ; u++) +INIT_LIST_HEAD (&mount_hashtable[u]); -/* And initialize the newly allocated array */ - d = mount_hashtable; - i = nr_hash; -do { -INIT_LIST_HEAD (d); - d++; - i--; - }while (i); err =sysfs_init ();if (err) printk(KERN_WARNING "%s: sysfs_init error: %d\n",
_
Quoted 54 lines · On Fri, 21 Dec 2007 02:29:12 -0800 Andrew Morton <[email protected]>…
Quoted 2 lines · On Fri, 14 Dec 2007 18:51:06 +0100 Eric Dumazet <[email protected]> wrote:
We can use
ilog2()infs/namespace.cto computehash_bitsandhash_maskat compile time, not runtime.Well noted.
Quoted 1 line
[namespace.patch text/plain (1.4KB)]
argh. (save-as, read, copy-paste, s/^/> /g)
Quoted 43 lines
fs/namespace.c +3
-18@@ -25,6 +25,7 @@#include <linux/security.h> #include <linux/mount.h> #include <linux/ramfs.h> +#include <linux/log2.h> #include <asm/uaccess.h> #include <asm/unistd.h> #include "pnode.h" @@ -36,7 +37,8 @@ __cacheline_aligned_in_smp DEFINE_SPINLOCK(vfsmount_lock);static int event;static struct list_head *mount_hashtable__read_mostly ; -static int hash_mask__read_mostly , hash_bits__read_mostly ; +#define hash_bits ilog2(PAGE_SIZE / sizeof(struct list_head)) +#define hash_mask ((1UL << hash_bits) - 1)static struct kmem_cache *mnt_cache__read_mostly ;static struct rw_semaphore namespace_sem;@@ -1828,24 +1830,7 @@ void __init mnt_init(void)if (!mount_hashtable)panic ("Failed to allocate mount hash table\n" ); -/* -* Find the power-of-two list-heads that can fit into the allocation.. -* We don't guarantee that "sizeof(struct list_head)" is necessarily -* a power-of-two. -*/ - nr_hash =PAGE_SIZE /sizeof (struct list_head ); - hash_bits =0 ; -do { - hash_bits++; - }while ((nr_hash >> hash_bits) !=0 ); - hash_bits--; - -/* -* Re-calculate the actual number of entries and the mask -* from the number of bits we can fit. -*/ nr_hash =1UL << hash_bits; - hash_mask = nr_hash -1 ;
Quoted 5 lines
fs/namespace.c void __init mnt_init(void) · line 1835
Quoted 1 line
fs/namespace.c +0
-0printk ("Mount-cache hash table entries: %d\n" , nr_hash);Those #defines you now have there are foul. Please, when there's a choice between doing it minimally and doing it right, let's do it right?
Indeed !
Thanks Andrew for taking the time and fixing my lazyness :( Your version is much cleaner of course.
Quoted 69 lines
Look what we can now do:
fs/namespace.c renamed from fs/namespace.c~use-ilog2-in-fs-namespacec-fix +9
-17@@ -31,14 +31,15 @@#include "pnode.h" #include "internal.h" +#define HASH_SHIFT ilog2(PAGE_SIZE / sizeof(struct list_head)) +#define HASH_SIZE (1UL << HASH_SHIFT) +/* spinlock for vfsmount related operations, inplace of dcache_lock */ __cacheline_aligned_in_smp DEFINE_SPINLOCK (vfsmount_lock );static int event;static struct list_head *mount_hashtable__read_mostly ; -#define hash_bits ilog2(PAGE_SIZE / sizeof(struct list_head)) -#define hash_mask ((1UL << hash_bits) - 1)static struct kmem_cache *mnt_cache__read_mostly ;static struct rw_semaphore namespace_sem;@@ -50,8 +51,8 @@ static inline unsigned long hash(struct {unsigned long tmp = ((unsigned long )mnt /L1_CACHE_BYTES ); tmp += ((unsigned long )dentry /L1_CACHE_BYTES ); - tmp = tmp + (tmp >> hash_bits); -return tmp & hash_mask; + tmp = tmp + (tmp >>HASH_SHIFT ); +return tmp & (HASH_SIZE -1 ); }struct vfsmount *alloc_vfsmnt (const char *name)@@ -1815,9 +1816,7 @@ static void __init init_mount_tree(void)void __init mnt_init (void ) { -struct list_head *d; -unsigned int nr_hash; -int i; +unsigned u;int err;init_rwsem (&namespace_sem);@@ -1830,18 +1829,11 @@ void __init mnt_init(void)if (!mount_hashtable)panic ("Failed to allocate mount hash table\n" ); - nr_hash =1UL << hash_bits; +printk ("Mount-cache hash table entries: %lu\n" ,HASH_SIZE ); -printk ("Mount-cache hash table entries: %d\n" , nr_hash); +for (u =0 ; u <HASH_SIZE ; u++) +INIT_LIST_HEAD (&mount_hashtable[u]); -/* And initialize the newly allocated array */ - d = mount_hashtable; - i = nr_hash; -do { -INIT_LIST_HEAD (d); - d++; - i--; - }while (i); err =sysfs_init ();if (err) printk(KERN_WARNING "%s: sysfs_init error: %d\n",_