[Cluster-devel] [PATCH 0/6] GFS2: Misc minor optimizations

[Cluster-devel] [PATCH 0/6] GFS2: Misc minor optimizations

[rpeterso]

I've taken Steve Whitehouse's advice and broken the previous set of
changes into six individual patches. In some cases, the patches are
not so much for optimization as they are for readability and to make
GFS2 easier to understand. The patches are as follows:

[PATCH 1/6] GFS2: change function gfs2_direct_IO to use a normal gfs2_glock_dq
[PATCH 2/6] GFS2: inline __gfs2_glock_schedule_for_reclaim
[PATCH 3/6] GFS2: Combine functions gfs2_glock_wait and wait_on_holder
[PATCH 4/6] GFS2: Combine functions gfs2_glock_dq_wait and wait_on_demote
[PATCH 5/6] GFS2: Eliminate redundant calls to may_grant
[PATCH 6/6] GFS2: Eliminate unnecessary check for state > 3 in bitfit

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH 1/6] GFS2: change function gfs2_direct_IO to use a normal gfs2_glock_dq

[rpeterso]

From: Bob Peterson <rpeterso@redhat.com>

This patch changes function gfs2_direct_IO so that it uses a normal
call to gfs2_glock_dq rather than a call to a multiple-dq of one item.
---
 fs/gfs2/aops.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 00eaa83..01c4975 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -1024,7 +1024,7 @@ static ssize_t gfs2_direct_IO(int rw, struct kiocb *iocb,
 				  offset, nr_segs, gfs2_get_block_direct,
 				  NULL, NULL, 0);
 out:
-	gfs2_glock_dq_m(1, &gh);
+	gfs2_glock_dq(&gh);
 	gfs2_holder_uninit(&gh);
 	return rv;
 }
-- 
1.7.7.6

[Cluster-devel] [PATCH 1/6] GFS2: change function gfs2_direct_IO to use a normal gfs2_glock_dq

[Steven Whitehouse]

Hi,

All six pushed to the -nmw tree. Thanks,

Steve.

On Thu, 2012-08-09 at 12:48 -0500, rpeterso at redhat.com wrote:
> From: Bob Peterson <rpeterso@redhat.com>
> 
> This patch changes function gfs2_direct_IO so that it uses a normal
> call to gfs2_glock_dq rather than a call to a multiple-dq of one item.
> ---
>  fs/gfs2/aops.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index 00eaa83..01c4975 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -1024,7 +1024,7 @@ static ssize_t gfs2_direct_IO(int rw, struct kiocb *iocb,
>  				  offset, nr_segs, gfs2_get_block_direct,
>  				  NULL, NULL, 0);
>  out:
> -	gfs2_glock_dq_m(1, &gh);
> +	gfs2_glock_dq(&gh);
>  	gfs2_holder_uninit(&gh);
>  	return rv;
>  }

[Cluster-devel] [PATCH 2/6] GFS2: inline __gfs2_glock_schedule_for_reclaim

[rpeterso]

From: Bob Peterson <rpeterso@redhat.com>

Since function gfs2_glock_schedule_for_reclaim is only two
significant lines, we can eliminate it, simplifying the code
and making it more readable.
---
 fs/gfs2/glock.c |   19 +++----------------
 1 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 1ed81f4..67f3e42 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -186,20 +186,6 @@ static void gfs2_glock_remove_from_lru(struct gfs2_glock *gl)
 }
 
 /**
- * __gfs2_glock_schedule_for_reclaim - Add a glock to the reclaim list
- * @gl: the glock
- *
- * If the glock is demotable, then we add it (or move it) to the end
- * of the glock LRU list.
- */
-
-static void __gfs2_glock_schedule_for_reclaim(struct gfs2_glock *gl)
-{
-	if (demote_ok(gl))
-		gfs2_glock_add_to_lru(gl);
-}
-
-/**
  * gfs2_glock_put_nolock() - Decrement reference count on glock
  * @gl: The glock to put
  *
@@ -1121,8 +1107,9 @@ void gfs2_glock_dq(struct gfs2_holder *gh)
 		    !test_bit(GLF_DEMOTE, &gl->gl_flags))
 			fast_path = 1;
 	}
-	if (!test_bit(GLF_LFLUSH, &gl->gl_flags))
-		__gfs2_glock_schedule_for_reclaim(gl);
+	if (!test_bit(GLF_LFLUSH, &gl->gl_flags) && demote_ok(gl))
+		gfs2_glock_add_to_lru(gl);
+
 	trace_gfs2_glock_queue(gh, 0);
 	spin_unlock(&gl->gl_spin);
 	if (likely(fast_path))
-- 
1.7.7.6

[Cluster-devel] [PATCH 3/6] GFS2: Combine functions gfs2_glock_wait and wait_on_holder

[rpeterso]

From: Bob Peterson <rpeterso@redhat.com>

Function gfs2_glock_wait only called function wait_on_holder and
returned its return code, so they were combined for readability.
---
 fs/gfs2/glock.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 67f3e42..5c87909 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -869,7 +869,14 @@ static int gfs2_glock_demote_wait(void *word)
 	return 0;
 }
 
-static void wait_on_holder(struct gfs2_holder *gh)
+/**
+ * gfs2_glock_wait - wait on a glock acquisition
+ * @gh: the glock holder
+ *
+ * Returns: 0 on success
+ */
+
+int gfs2_glock_wait(struct gfs2_holder *gh)
 {
 	unsigned long time1 = jiffies;
 
@@ -880,6 +887,7 @@ static void wait_on_holder(struct gfs2_holder *gh)
 		gh->gh_gl->gl_hold_time = min(gh->gh_gl->gl_hold_time +
 					      GL_GLOCK_HOLD_INCR,
 					      GL_GLOCK_MAX_HOLD);
+	return gh->gh_error;
 }
 
 static void wait_on_demote(struct gfs2_glock *gl)
@@ -915,19 +923,6 @@ static void handle_callback(struct gfs2_glock *gl, unsigned int state,
 	trace_gfs2_demote_rq(gl);
 }
 
-/**
- * gfs2_glock_wait - wait on a glock acquisition
- * @gh: the glock holder
- *
- * Returns: 0 on success
- */
-
-int gfs2_glock_wait(struct gfs2_holder *gh)
-{
-	wait_on_holder(gh);
-	return gh->gh_error;
-}
-
 void gfs2_print_dbg(struct seq_file *seq, const char *fmt, ...)
 {
 	struct va_format vaf;
-- 
1.7.7.6

[Cluster-devel] [PATCH 4/6] GFS2: Combine functions gfs2_glock_dq_wait and wait_on_demote

[rpeterso]

From: Bob Peterson <rpeterso@redhat.com>

Function gfs2_glock_dq_wait called two-line function wait_on_demote,
so they were combined.
---
 fs/gfs2/glock.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 5c87909..fca6a87 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -890,12 +890,6 @@ int gfs2_glock_wait(struct gfs2_holder *gh)
 	return gh->gh_error;
 }
 
-static void wait_on_demote(struct gfs2_glock *gl)
-{
-	might_sleep();
-	wait_on_bit(&gl->gl_flags, GLF_DEMOTE, gfs2_glock_demote_wait, TASK_UNINTERRUPTIBLE);
-}
-
 /**
  * handle_callback - process a demote request
  * @gl: the glock
@@ -1123,7 +1117,8 @@ void gfs2_glock_dq_wait(struct gfs2_holder *gh)
 {
 	struct gfs2_glock *gl = gh->gh_gl;
 	gfs2_glock_dq(gh);
-	wait_on_demote(gl);
+	might_sleep();
+	wait_on_bit(&gl->gl_flags, GLF_DEMOTE, gfs2_glock_demote_wait, TASK_UNINTERRUPTIBLE);
 }
 
 /**
-- 
1.7.7.6

[Cluster-devel] [PATCH 5/6] GFS2: Eliminate redundant calls to may_grant

[rpeterso]

From: Bob Peterson <rpeterso@redhat.com>

Function add_to_queue was checking may_grant for the passed-in
holder for every iteration of its gh2 loop. Now it only checks it
once at the beginning to see if a try lock is futile.
---
 fs/gfs2/glock.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index fca6a87..e6c2fd5 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -954,7 +954,7 @@ __acquires(&gl->gl_spin)
 	struct gfs2_sbd *sdp = gl->gl_sbd;
 	struct list_head *insert_pt = NULL;
 	struct gfs2_holder *gh2;
-	int try_lock = 0;
+	int try_futile = 0;
 
 	BUG_ON(gh->gh_owner_pid == NULL);
 	if (test_and_set_bit(HIF_WAIT, &gh->gh_iflags))
@@ -962,7 +962,7 @@ __acquires(&gl->gl_spin)
 
 	if (gh->gh_flags & (LM_FLAG_TRY | LM_FLAG_TRY_1CB)) {
 		if (test_bit(GLF_LOCK, &gl->gl_flags))
-			try_lock = 1;
+			try_futile = !may_grant(gl, gh);
 		if (test_bit(GLF_INVALIDATE_IN_PROGRESS, &gl->gl_flags))
 			goto fail;
 	}
@@ -971,9 +971,8 @@ __acquires(&gl->gl_spin)
 		if (unlikely(gh2->gh_owner_pid == gh->gh_owner_pid &&
 		    (gh->gh_gl->gl_ops->go_type != LM_TYPE_FLOCK)))
 			goto trap_recursive;
-		if (try_lock &&
-		    !(gh2->gh_flags & (LM_FLAG_TRY | LM_FLAG_TRY_1CB)) &&
-		    !may_grant(gl, gh)) {
+		if (try_futile &&
+		    !(gh2->gh_flags & (LM_FLAG_TRY | LM_FLAG_TRY_1CB))) {
 fail:
 			gh->gh_error = GLR_TRYFAILED;
 			gfs2_holder_wake(gh);
-- 
1.7.7.6

[Cluster-devel] [PATCH 6/6] GFS2: Eliminate unnecessary check for state > 3 in bitfit

[rpeterso]

From: Bob Peterson <rpeterso@redhat.com>

Function gfs2_bitfit was checking for state > 3, but that's
impossible since it is only called from rgblk_search, which receives
only GFS2_BLKST_ constants.
---
 fs/gfs2/rgrp.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index c267118..47d2346 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -228,8 +228,6 @@ static u32 gfs2_bitfit(const u8 *buf, const unsigned int len,
 	u64 mask = 0x5555555555555555ULL;
 	u32 bit;
 
-	BUG_ON(state > 3);
-
 	/* Mask off bits we don't care about at the start of the search */
 	mask <<= spoint;
 	tmp = gfs2_bit_search(ptr, mask, state);
-- 
1.7.7.6