FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Cluster Development

 
 
LinkBack Thread Tools
 
Old 08-08-2012, 08:52 PM
Bob Peterson
 
Default GFS2: Misc minor optimizations

Hi,

This patch implements some minor optimizations of the glock and rgrp code:
In most cases, I just combined tiny one or two-line functions into their
only caller. Some of these optimizations may gain us little to no real
improvement, but having fewer functions also makes it easier to read and
understand. In order of appearance:

1. In function gfs2_direct_IO, the glock was dequeued as a group of 1
glock. This was changed to a normal dequeue for readability.
2. Two-line function __gfs2_glock_schedule_for_reclaim was coded into
its only caller, gfs2_glock_dq.
3. Function gfs2_glock_wait only called function wait_on_holder and
returned its return code, so they were combined.
4. Function gfs2_glock_dq_wait called two-line function wait_on_demote,
so they were combined.
5. 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.
6. 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.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
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;
}
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 1ed81f4..8146c6f 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
*
@@ -883,7 +869,7 @@ static int gfs2_glock_demote_wait(void *word)
return 0;
}

-static void wait_on_holder(struct gfs2_holder *gh)
+int gfs2_glock_wait(struct gfs2_holder *gh)
{
unsigned long time1 = jiffies;

@@ -894,12 +880,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);
-}
-
-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);
+ return gh->gh_error;
}

/**
@@ -929,19 +910,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;
@@ -979,7 +947,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))
@@ -987,7 +955,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;
}
@@ -996,9 +964,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);
@@ -1121,8 +1088,8 @@ 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))
@@ -1141,7 +1108,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);
}

/**
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);
 
Old 08-09-2012, 09:12 AM
Steven Whitehouse
 
Default GFS2: Misc minor optimizations

Hi,

On Wed, 2012-08-08 at 16:52 -0400, Bob Peterson wrote:
> Hi,
>
> This patch implements some minor optimizations of the glock and rgrp code:
> In most cases, I just combined tiny one or two-line functions into their
> only caller. Some of these optimizations may gain us little to no real
> improvement, but having fewer functions also makes it easier to read and
> understand. In order of appearance:
>
When there are a number of unrelated things it would be better to break
into separate patches. It at least gives us a better chance if we need
to bisect in the future.

> 1. In function gfs2_direct_IO, the glock was dequeued as a group of 1
> glock. This was changed to a normal dequeue for readability.
Looks good

> 2. Two-line function __gfs2_glock_schedule_for_reclaim was coded into
> its only caller, gfs2_glock_dq.
Ok, although that is unlikely to make any real difference since the
compiler will do that anyway.

> 3. Function gfs2_glock_wait only called function wait_on_holder and
> returned its return code, so they were combined.
Looks good

> 4. Function gfs2_glock_dq_wait called two-line function wait_on_demote,
> so they were combined.
As per #2, the compiler will do this for us anyway, so no real gain
there.

> 5. 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.
Sounds like a good plan, but this should really be split out from the
rest of this patch.

> 6. 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.
>
Looks good - this has become redundant now that we can easily visually
check the passed constants,

Steve.

> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> 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;
> }
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index 1ed81f4..8146c6f 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
> *
> @@ -883,7 +869,7 @@ static int gfs2_glock_demote_wait(void *word)
> return 0;
> }
>
> -static void wait_on_holder(struct gfs2_holder *gh)
> +int gfs2_glock_wait(struct gfs2_holder *gh)
> {
> unsigned long time1 = jiffies;
>
> @@ -894,12 +880,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);
> -}
> -
> -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);
> + return gh->gh_error;
> }
>
> /**
> @@ -929,19 +910,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;
> @@ -979,7 +947,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))
> @@ -987,7 +955,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;
> }
> @@ -996,9 +964,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);
> @@ -1121,8 +1088,8 @@ 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))
> @@ -1141,7 +1108,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);
> }
>
> /**
> 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);
>
 
Old 08-09-2012, 05:48 PM
 
Default GFS2: Misc minor optimizations

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
 

Thread Tools




All times are GMT. The time now is 12:09 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org