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.
/**
- * __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;
}
- 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);
08-09-2012, 09:12 AM
Steven Whitehouse
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,
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