Re: [Cluster-devel] [GFS2 PATCH 1/2][TRY #2] GFS2: Move glock superblock pointer to field gl_name
- Original Message - Bob, how hard would it be to move the rhashtable into struct gfs2_sbd instead? Thanks, Andreas Hi Andreas, I've suggested moving the glock hash table into the superblock a couple times in the past, when it was still a normal hash table, but I've always been shot down. The advantage was improved scalability and not having to filter out the glocks you don't want from the ones you don't when you go to do things like dumping glocks. There's one central point for all glocks everywhere. With rhashtable, scalability isn't much of an issue anymore. The disadvantage is having multiple hash tables to manage when you are trying to clear things out when you're unloading the module, for example. I've always felt the advantages outweighed the disadvantages, but others did not agree. Regards, Bob Peterson Red Hat File Systems
Re: [Cluster-devel] [GFS2 PATCH 1/2][TRY #2] GFS2: Move glock superblock pointer to field gl_name
Hi, On 22/08/15 23:00, Andreas Gruenbacher wrote: Bob, how hard would it be to move the rhashtable into struct gfs2_sbd instead? Thanks, Andreas What would be the advantage? Steve.
Re: [Cluster-devel] [GFS2 PATCH 1/2][TRY #2] GFS2: Move glock superblock pointer to field gl_name
Bob, how hard would it be to move the rhashtable into struct gfs2_sbd instead? Thanks, Andreas
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
