Re: [Cluster-devel] [GFS2 PATCH 2/2] GFS2: Use resizable hash table for glocks
Hi,
Looks good. Acked-by: Steven Whitehouse swhit...@redhat.com
Steve.
On 09/07/15 19:25, Bob Peterson wrote:
This patch changes the glock hash table from a normal hash table to
a resizable hash table, which scales better. This also simplifies
a lot of code.
---
fs/gfs2/glock.c | 282 +--
fs/gfs2/incore.h | 4 +-
2 files changed, 111 insertions(+), 175 deletions(-)
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 25e0389..813de00 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -34,6 +34,7 @@
#include linux/percpu.h
#include linux/list_sort.h
#include linux/lockref.h
+#include linux/rhashtable.h
#include gfs2.h
#include incore.h
@@ -51,8 +52,12 @@
struct gfs2_glock_iter {
int hash; /* hash bucket index */
- unsigned nhash; /* Index within current bucket */
struct gfs2_sbd *sdp; /* incore superblock */
+};
+
+struct gfs2_glock_rht_iter {
+ struct gfs2_sbd *sdp; /* incore superblock */
+ struct rhashtable_iter hti; /* rhashtable iterator */
struct gfs2_glock *gl; /* current glock struct*/
loff_t last_pos;/* last position */
};
@@ -70,44 +75,20 @@ static DEFINE_SPINLOCK(lru_lock);
#define GFS2_GL_HASH_SHIFT 15
#define GFS2_GL_HASH_SIZE (1 GFS2_GL_HASH_SHIFT)
-#define GFS2_GL_HASH_MASK (GFS2_GL_HASH_SIZE - 1)
-
-static struct hlist_bl_head gl_hash_table[GFS2_GL_HASH_SIZE];
-static struct dentry *gfs2_root;
-
-/**
- * gl_hash() - Turn glock number into hash bucket number
- * @lock: The glock number
- *
- * Returns: The number of the corresponding hash bucket
- */
-
-static unsigned int gl_hash(const struct gfs2_sbd *sdp,
- const struct lm_lockname *name)
-{
- unsigned int h;
-
- h = jhash(name-ln_number, sizeof(u64), 0);
- h = jhash(name-ln_type, sizeof(unsigned int), h);
- h = jhash(sdp, sizeof(struct gfs2_sbd *), h);
- h = GFS2_GL_HASH_MASK;
-
- return h;
-}
-static inline void spin_lock_bucket(unsigned int hash)
-{
- hlist_bl_lock(gl_hash_table[hash]);
-}
+struct rhashtable_params ht_parms = {
+ .nelem_hint = GFS2_GL_HASH_SIZE * 3 / 4,
+ .key_len = sizeof(struct lm_lockname),
+ .key_offset = offsetof(struct gfs2_glock, gl_name),
+ .head_offset = offsetof(struct gfs2_glock, gl_node),
+ .hashfn = jhash,
+};
-static inline void spin_unlock_bucket(unsigned int hash)
-{
- hlist_bl_unlock(gl_hash_table[hash]);
-}
+static struct rhashtable gl_hash_table;
-static void gfs2_glock_dealloc(struct rcu_head *rcu)
+void gfs2_glock_free(struct gfs2_glock *gl)
{
- struct gfs2_glock *gl = container_of(rcu, struct gfs2_glock, gl_rcu);
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
if (gl-gl_ops-go_flags GLOF_ASPACE) {
kmem_cache_free(gfs2_glock_aspace_cachep, gl);
@@ -115,13 +96,6 @@ static void gfs2_glock_dealloc(struct rcu_head *rcu)
kfree(gl-gl_lksb.sb_lvbptr);
kmem_cache_free(gfs2_glock_cachep, gl);
}
-}
-
-void gfs2_glock_free(struct gfs2_glock *gl)
-{
- struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
-
- call_rcu(gl-gl_rcu, gfs2_glock_dealloc);
if (atomic_dec_and_test(sdp-sd_glock_disposal))
wake_up(sdp-sd_glock_wait);
}
@@ -202,9 +176,8 @@ void gfs2_glock_put(struct gfs2_glock *gl)
gfs2_glock_remove_from_lru(gl);
spin_unlock(gl-gl_lockref.lock);
- spin_lock_bucket(gl-gl_hash);
- hlist_bl_del_rcu(gl-gl_list);
- spin_unlock_bucket(gl-gl_hash);
+ if (gl-gl_node.next != NULL)
+ rhashtable_remove_fast(gl_hash_table, gl-gl_node, ht_parms);
GLOCK_BUG_ON(gl, !list_empty(gl-gl_holders));
GLOCK_BUG_ON(gl, mapping mapping-nrpages);
trace_gfs2_glock_put(gl);
@@ -212,30 +185,6 @@ void gfs2_glock_put(struct gfs2_glock *gl)
}
/**
- * search_bucket() - Find struct gfs2_glock by lock number
- * @bucket: the bucket to search
- * @name: The lock name
- *
- * Returns: NULL, or the struct gfs2_glock with the requested number
- */
-
-static struct gfs2_glock *search_bucket(unsigned int hash,
- const struct lm_lockname *name)
-{
- struct gfs2_glock *gl;
- struct hlist_bl_node *h;
-
- hlist_bl_for_each_entry_rcu(gl, h, gl_hash_table[hash], gl_list) {
- if (!lm_name_equal(gl-gl_name, name))
- continue;
- if (lockref_get_not_dead(gl-gl_lockref))
- return gl;
- }
-
- return NULL;
-}
-
-/**
* may_grant - check if its ok to grant a new lock
* @gl: The glock
* @gh: The lock request which we wish to grant
@@ -704,14 +653,14 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
Re: [Cluster-devel] [GFS2 PATCH 1/2] GFS2: Move glock superblock pointer to field gl_name
Hi,
On 09/07/15 19:25, Bob Peterson wrote:
What uniquely identifies a glock in the glock hash table is not
gl_name, but gl_name and its superblock pointer. This patch makes
the gl_name field correspond to a unique glock identifier. That will
allow us to simplify hashing with a future patch, since the hash
algorithm can then take the gl_name and hash its components in one
operation.
---
[snip]
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index a1ec7c2..4de7853 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -205,11 +205,13 @@ enum {
struct lm_lockname {
u64 ln_number;
unsigned int ln_type;
+ struct gfs2_sbd *ln_sbd;
};
This looks like its adding a hole on 64 bit arches... can we swap the
order of ln_type and ln_sbd, or even make the sbd the first element of
this? That way a memcmp of two of lm_locknames doesn't have to include
the hole in the comparison, which I assume is what you want here?
Steve.
Re: [Cluster-devel] [GFS2 PATCH 1/2][TRY #2] GFS2: Move glock superblock pointer to field gl_name
Hi,
- Original Message -
Hi,
On 09/07/15 19:25, Bob Peterson wrote:
What uniquely identifies a glock in the glock hash table is not
gl_name, but gl_name and its superblock pointer. This patch makes
the gl_name field correspond to a unique glock identifier. That will
allow us to simplify hashing with a future patch, since the hash
algorithm can then take the gl_name and hash its components in one
operation.
---
[snip]
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index a1ec7c2..4de7853 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -205,11 +205,13 @@ enum {
struct lm_lockname {
u64 ln_number;
unsigned int ln_type;
+ struct gfs2_sbd *ln_sbd;
};
This looks like its adding a hole on 64 bit arches... can we swap the
order of ln_type and ln_sbd, or even make the sbd the first element of
this? That way a memcmp of two of lm_locknames doesn't have to include
the hole in the comparison, which I assume is what you want here?
Steve.
Good catch. This replacement for patch #1 is identical except for
the placement of ln_sbd in the lm_lockname structure.
Regards,
Bob Peterson
Red Hat File Systems
---
commit 0ed2b820a4f3d44e447b3e724f5355cd2f7ee4ee
Author: Bob Peterson rpete...@redhat.com
Date: Mon Mar 16 11:52:05 2015 -0500
GFS2: Move glock superblock pointer to field gl_name
What uniquely identifies a glock in the glock hash table is not
gl_name, but gl_name and its superblock pointer. This patch makes
the gl_name field correspond to a unique glock identifier. That will
allow us to simplify hashing with a future patch, since the hash
algorithm can then take the gl_name and hash its components in one
operation.
Signed-off-by: Bob Peterson rpete...@redhat.com
---
fs/gfs2/glock.c | 32 +++-
fs/gfs2/glops.c | 38 --
fs/gfs2/incore.h | 9 +
fs/gfs2/lock_dlm.c | 10 +-
fs/gfs2/lops.c | 6 +++---
fs/gfs2/meta_io.c| 6 +++---
fs/gfs2/meta_io.h| 2 +-
fs/gfs2/quota.c | 22 +++---
fs/gfs2/rgrp.c | 2 +-
fs/gfs2/trace_gfs2.h | 18 +-
fs/gfs2/trans.c | 4 ++--
11 files changed, 75 insertions(+), 74 deletions(-)
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index a38e38f..25e0389 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -119,7 +119,7 @@ static void gfs2_glock_dealloc(struct rcu_head *rcu)
void gfs2_glock_free(struct gfs2_glock *gl)
{
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
call_rcu(gl-gl_rcu, gfs2_glock_dealloc);
if (atomic_dec_and_test(sdp-sd_glock_disposal))
@@ -192,7 +192,7 @@ static void gfs2_glock_remove_from_lru(struct gfs2_glock
*gl)
void gfs2_glock_put(struct gfs2_glock *gl)
{
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
struct address_space *mapping = gfs2_glock2aspace(gl);
if (lockref_put_or_lock(gl-gl_lockref))
@@ -220,7 +220,6 @@ void gfs2_glock_put(struct gfs2_glock *gl)
*/
static struct gfs2_glock *search_bucket(unsigned int hash,
- const struct gfs2_sbd *sdp,
const struct lm_lockname *name)
{
struct gfs2_glock *gl;
@@ -229,8 +228,6 @@ static struct gfs2_glock *search_bucket(unsigned int hash,
hlist_bl_for_each_entry_rcu(gl, h, gl_hash_table[hash], gl_list) {
if (!lm_name_equal(gl-gl_name, name))
continue;
- if (gl-gl_sbd != sdp)
- continue;
if (lockref_get_not_dead(gl-gl_lockref))
return gl;
}
@@ -506,7 +503,7 @@ __releases(gl-gl_spin)
__acquires(gl-gl_spin)
{
const struct gfs2_glock_operations *glops = gl-gl_ops;
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
unsigned int lck_flags = gh ? gh-gh_flags : 0;
int ret;
@@ -628,7 +625,7 @@ out_unlock:
static void delete_work_func(struct work_struct *work)
{
struct gfs2_glock *gl = container_of(work, struct gfs2_glock,
gl_delete);
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
struct gfs2_inode *ip;
struct inode *inode;
u64 no_addr = gl-gl_name.ln_number;
@@ -704,14 +701,16 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
struct gfs2_glock **glp)
{
struct super_block *s = sdp-sd_vfs;
- struct lm_lockname name = { .ln_number = number, .ln_type =
glops-go_type };
+ struct lm_lockname name = { .ln_number = number,
+ .ln_type = glops-go_type,
+ .ln_sbd = sdp };
struct gfs2_glock *gl, *tmp;
unsigned int hash = gl_hash(sdp, name);
struct address_space *mapping;
Re: [Cluster-devel] [GFS2 PATCH 1/2][TRY #2] GFS2: Move glock superblock pointer to field gl_name
Hi,
On 13/07/15 14:39, Bob Peterson wrote:
Hi,
- Original Message -
Hi,
On 09/07/15 19:25, Bob Peterson wrote:
What uniquely identifies a glock in the glock hash table is not
gl_name, but gl_name and its superblock pointer. This patch makes
the gl_name field correspond to a unique glock identifier. That will
allow us to simplify hashing with a future patch, since the hash
algorithm can then take the gl_name and hash its components in one
operation.
---
[snip]
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index a1ec7c2..4de7853 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -205,11 +205,13 @@ enum {
struct lm_lockname {
u64 ln_number;
unsigned int ln_type;
+ struct gfs2_sbd *ln_sbd;
};
This looks like its adding a hole on 64 bit arches... can we swap the
order of ln_type and ln_sbd, or even make the sbd the first element of
this? That way a memcmp of two of lm_locknames doesn't have to include
the hole in the comparison, which I assume is what you want here?
Steve.
Good catch. This replacement for patch #1 is identical except for
the placement of ln_sbd in the lm_lockname structure.
Regards,
Bob Peterson
Red Hat File Systems
---
That looks good now. Acked-by: Steven Whitehouse swhit...@redhat.com
Steve.
commit 0ed2b820a4f3d44e447b3e724f5355cd2f7ee4ee
Author: Bob Peterson rpete...@redhat.com
Date: Mon Mar 16 11:52:05 2015 -0500
GFS2: Move glock superblock pointer to field gl_name
What uniquely identifies a glock in the glock hash table is not
gl_name, but gl_name and its superblock pointer. This patch makes
the gl_name field correspond to a unique glock identifier. That will
allow us to simplify hashing with a future patch, since the hash
algorithm can then take the gl_name and hash its components in one
operation.
Signed-off-by: Bob Peterson rpete...@redhat.com
---
fs/gfs2/glock.c | 32 +++-
fs/gfs2/glops.c | 38 --
fs/gfs2/incore.h | 9 +
fs/gfs2/lock_dlm.c | 10 +-
fs/gfs2/lops.c | 6 +++---
fs/gfs2/meta_io.c| 6 +++---
fs/gfs2/meta_io.h| 2 +-
fs/gfs2/quota.c | 22 +++---
fs/gfs2/rgrp.c | 2 +-
fs/gfs2/trace_gfs2.h | 18 +-
fs/gfs2/trans.c | 4 ++--
11 files changed, 75 insertions(+), 74 deletions(-)
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index a38e38f..25e0389 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -119,7 +119,7 @@ static void gfs2_glock_dealloc(struct rcu_head *rcu)
void gfs2_glock_free(struct gfs2_glock *gl)
{
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
call_rcu(gl-gl_rcu, gfs2_glock_dealloc);
if (atomic_dec_and_test(sdp-sd_glock_disposal))
@@ -192,7 +192,7 @@ static void gfs2_glock_remove_from_lru(struct gfs2_glock
*gl)
void gfs2_glock_put(struct gfs2_glock *gl)
{
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
struct address_space *mapping = gfs2_glock2aspace(gl);
if (lockref_put_or_lock(gl-gl_lockref))
@@ -220,7 +220,6 @@ void gfs2_glock_put(struct gfs2_glock *gl)
*/
static struct gfs2_glock *search_bucket(unsigned int hash,
- const struct gfs2_sbd *sdp,
const struct lm_lockname *name)
{
struct gfs2_glock *gl;
@@ -229,8 +228,6 @@ static struct gfs2_glock *search_bucket(unsigned int hash,
hlist_bl_for_each_entry_rcu(gl, h, gl_hash_table[hash], gl_list) {
if (!lm_name_equal(gl-gl_name, name))
continue;
- if (gl-gl_sbd != sdp)
- continue;
if (lockref_get_not_dead(gl-gl_lockref))
return gl;
}
@@ -506,7 +503,7 @@ __releases(gl-gl_spin)
__acquires(gl-gl_spin)
{
const struct gfs2_glock_operations *glops = gl-gl_ops;
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
unsigned int lck_flags = gh ? gh-gh_flags : 0;
int ret;
@@ -628,7 +625,7 @@ out_unlock:
static void delete_work_func(struct work_struct *work)
{
struct gfs2_glock *gl = container_of(work, struct gfs2_glock,
gl_delete);
- struct gfs2_sbd *sdp = gl-gl_sbd;
+ struct gfs2_sbd *sdp = gl-gl_name.ln_sbd;
struct gfs2_inode *ip;
struct inode *inode;
u64 no_addr = gl-gl_name.ln_number;
@@ -704,14 +701,16 @@ int gfs2_glock_get(struct gfs2_sbd *sdp, u64 number,
struct gfs2_glock **glp)
{
struct super_block *s = sdp-sd_vfs;
- struct lm_lockname name = { .ln_number = number, .ln_type =
glops-go_type };
+ struct lm_lockname name = { .ln_number = number,
+ .ln_type = glops-go_type,
+ .ln_sbd = sdp
[Cluster-devel] [PATCH linux-gfs2] GFS2: ht_parms can be static
Signed-off-by: Fengguang Wu fengguang...@intel.com
---
glock.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 813de00..1b0dff4 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -76,7 +76,7 @@ static DEFINE_SPINLOCK(lru_lock);
#define GFS2_GL_HASH_SHIFT 15
#define GFS2_GL_HASH_SIZE (1 GFS2_GL_HASH_SHIFT)
-struct rhashtable_params ht_parms = {
+static struct rhashtable_params ht_parms = {
.nelem_hint = GFS2_GL_HASH_SIZE * 3 / 4,
.key_len = sizeof(struct lm_lockname),
.key_offset = offsetof(struct gfs2_glock, gl_name),
