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 02-05-2010, 04:45 AM
Dave Chinner
 
Default gfs2: introduce AIL lock

THe log lock is currently used to protect the AIL lists and
the movements of buffers into and out of them. The lists
are self contained and no log specific items outside the
lists are accessed when starting or emptying the AIL lists.

Hence the operation of the AIL does not require the protection
of the log lock so split them out into a new AIL specific lock
to reduce the amount of traffic on the log lock. This will
also reduce the amount of serialisation that occurs when
the gfs2_logd pushes on the AIL to move it forward.

This reduces the impact of log pushing on sequential write
throughput. On no-op scheduler on a disk that can do 85MB/s,
this increases the write rate from 65MB/s with the ordering
fixes to 75MB/s.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/gfs2/glops.c | 10 ++++++++--
fs/gfs2/incore.h | 1 +
fs/gfs2/log.c | 32 +++++++++++++++++---------------
fs/gfs2/log.h | 22 ++++++++++++++++++++++
fs/gfs2/lops.c | 5 ++++-
fs/gfs2/ops_fstype.c | 1 +
6 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 78554ac..65048f9 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -57,20 +57,26 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl)
BUG_ON(current->journal_info);
current->journal_info = &tr;

- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);
while (!list_empty(head)) {
bd = list_entry(head->next, struct gfs2_bufdata,
bd_ail_gl_list);
bh = bd->bd_bh;
gfs2_remove_from_ail(bd);
+ gfs2_ail_unlock(sdp);
+
bd->bd_bh = NULL;
bh->b_private = NULL;
bd->bd_blkno = bh->b_blocknr;
+ gfs2_log_lock(sdp);
gfs2_assert_withdraw(sdp, !buffer_busy(bh));
gfs2_trans_add_revoke(sdp, bd);
+ gfs2_log_unlock(sdp);
+
+ gfs2_ail_lock(sdp);
}
gfs2_assert_withdraw(sdp, !atomic_read(&gl->gl_ail_count));
- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);

gfs2_trans_end(sdp);
gfs2_log_flush(sdp, NULL);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 4792200..00d610f 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -644,6 +644,7 @@ struct gfs2_sbd {
unsigned int sd_log_flush_head;
u64 sd_log_flush_wrapped;

+ spinlock_t sd_ail_lock;
struct list_head sd_ail1_list;
struct list_head sd_ail2_list;
u64 sd_ail_sync_gen;
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index a9797be..f2ee615 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -89,8 +89,8 @@ void gfs2_remove_from_ail(struct gfs2_bufdata *bd)
*/

static void gfs2_ail1_start_one(struct gfs2_sbd *sdp, struct gfs2_ail *ai)
-__releases(&sdp->sd_log_lock)
-__acquires(&sdp->sd_log_lock)
+__releases(&sdp->sd_ail_lock)
+__acquires(&sdp->sd_ail_lock)
{
struct gfs2_bufdata *bd, *s;
struct buffer_head *bh;
@@ -118,7 +118,7 @@ __acquires(&sdp->sd_log_lock)
list_move(&bd->bd_ail_st_list, &ai->ai_ail1_list);

get_bh(bh);
- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);
lock_buffer(bh);
if (test_clear_buffer_dirty(bh)) {
bh->b_end_io = end_buffer_write_sync;
@@ -128,7 +128,7 @@ __acquires(&sdp->sd_log_lock)
unlock_buffer(bh);
brelse(bh);
}
- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);

retry = 1;
break;
@@ -178,10 +178,10 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp, int flags)
struct gfs2_ail *first_ai, *ai, *tmp;
int done = 0;

- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);
head = &sdp->sd_ail1_list;
if (list_empty(head)) {
- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);
return;
}
sync_gen = sdp->sd_ail_sync_gen++;
@@ -189,7 +189,7 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp, int flags)
first = head->prev;
first_ai = list_entry(first, struct gfs2_ail, ai_list);
first_ai->ai_sync_gen = sync_gen;
- gfs2_ail1_start_one(sdp, first_ai); /* This may drop log lock */
+ gfs2_ail1_start_one(sdp, first_ai); /* This may drop ail lock */

if (flags & DIO_ALL)
first = NULL;
@@ -204,13 +204,13 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp, int flags)
if (ai->ai_sync_gen >= sync_gen)
continue;
ai->ai_sync_gen = sync_gen;
- gfs2_ail1_start_one(sdp, ai); /* This may drop log lock */
+ gfs2_ail1_start_one(sdp, ai); /* This may drop ail lock */
done = 0;
break;
}
}

- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);
}

static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)
@@ -218,7 +218,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)
struct gfs2_ail *ai, *s;
int ret;

- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);

list_for_each_entry_safe_reverse(ai, s, &sdp->sd_ail1_list, ai_list) {
if (gfs2_ail1_empty_one(sdp, ai, flags))
@@ -229,7 +229,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)

ret = list_empty(&sdp->sd_ail1_list);

- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);

return ret;
}
@@ -262,7 +262,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int new_tail)
int wrap = (new_tail < old_tail);
int a, b, rm;

- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);

list_for_each_entry_safe(ai, safe, &sdp->sd_ail2_list, ai_list) {
a = (old_tail <= ai->ai_first);
@@ -278,7 +278,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int new_tail)
kfree(ai);
}

- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);
}

/**
@@ -437,7 +437,7 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
struct gfs2_ail *ai;
unsigned int tail;

- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);

if (list_empty(&sdp->sd_ail1_list)) {
tail = sdp->sd_log_head;
@@ -446,7 +446,7 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
tail = ai->ai_first;
}

- gfs2_log_unlock(sdp);
+ gfs2_ail_unlock(sdp);

return tail;
}
@@ -775,10 +775,12 @@ void __gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl)
sdp->sd_log_commited_databuf = 0;
sdp->sd_log_commited_revoke = 0;

+ gfs2_ail_lock(sdp);
if (!list_empty(&ai->ai_ail1_list)) {
list_add(&ai->ai_list, &sdp->sd_ail1_list);
ai = NULL;
}
+ gfs2_ail_unlock(sdp);
gfs2_log_unlock(sdp);
trace_gfs2_log_flush(sdp, 0);
up_write(&sdp->sd_log_flush_lock);
diff --git a/fs/gfs2/log.h b/fs/gfs2/log.h
index 7c64510..62c110e 100644
--- a/fs/gfs2/log.h
+++ b/fs/gfs2/log.h
@@ -38,6 +38,28 @@ __releases(&sdp->sd_log_lock)
spin_unlock(&sdp->sd_log_lock);
}

+/**
+ * gfs2_ail_lock - acquire the right to mess with the AIL
+ * @sdp: the filesystem
+ *
+ */
+static inline void gfs2_ail_lock(struct gfs2_sbd *sdp)
+__acquires(&sdp->sd_ail_lock)
+{
+ spin_lock(&sdp->sd_ail_lock);
+}
+
+/**
+ * gfs2_ail_unlock - release the right to mess with the AIL
+ * @sdp: the filesystem
+ *
+ */
+static inline void gfs2_ail_unlock(struct gfs2_sbd *sdp)
+__releases(&sdp->sd_ail_lock)
+{
+ spin_unlock(&sdp->sd_ail_lock);
+}
+
static inline void gfs2_log_pointers_init(struct gfs2_sbd *sdp,
unsigned int value)
{
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 0fe2f3c..342d65e 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -80,7 +80,7 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
mark_buffer_dirty(bh);
clear_buffer_pinned(bh);

- gfs2_log_lock(sdp);
+ gfs2_ail_lock(sdp);
if (bd->bd_ail) {
list_del(&bd->bd_ail_st_list);
brelse(bh);
@@ -91,6 +91,9 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
}
bd->bd_ail = ai;
list_add(&bd->bd_ail_st_list, &ai->ai_ail1_list);
+ gfs2_ail_unlock(sdp);
+
+ gfs2_log_lock(sdp);
clear_bit(GLF_LFLUSH, &bd->bd_gl->gl_flags);
trace_gfs2_pin(bd, 0);
gfs2_log_unlock(sdp);
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index edfee24..14ecb2b 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -108,6 +108,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)
INIT_LIST_HEAD(&sdp->sd_log_le_ordered);

mutex_init(&sdp->sd_log_reserve_mutex);
+ spin_lock_init(&sdp->sd_ail_lock);
INIT_LIST_HEAD(&sdp->sd_ail1_list);
INIT_LIST_HEAD(&sdp->sd_ail2_list);

--
1.6.5
 
Old 02-05-2010, 10:11 AM
Steven Whitehouse
 
Default gfs2: introduce AIL lock

Hi,

On Fri, 2010-02-05 at 16:45 +1100, Dave Chinner wrote:
> THe log lock is currently used to protect the AIL lists and
> the movements of buffers into and out of them. The lists
> are self contained and no log specific items outside the
> lists are accessed when starting or emptying the AIL lists.
>
> Hence the operation of the AIL does not require the protection
> of the log lock so split them out into a new AIL specific lock
> to reduce the amount of traffic on the log lock. This will
> also reduce the amount of serialisation that occurs when
> the gfs2_logd pushes on the AIL to move it forward.
>
> This reduces the impact of log pushing on sequential write
> throughput. On no-op scheduler on a disk that can do 85MB/s,
> this increases the write rate from 65MB/s with the ordering
> fixes to 75MB/s.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

This looks good, but a couple of comments:

> ---
> fs/gfs2/glops.c | 10 ++++++++--
> fs/gfs2/incore.h | 1 +
> fs/gfs2/log.c | 32 +++++++++++++++++---------------
> fs/gfs2/log.h | 22 ++++++++++++++++++++++
> fs/gfs2/lops.c | 5 ++++-
> fs/gfs2/ops_fstype.c | 1 +
> 6 files changed, 53 insertions(+), 18 deletions(-)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 78554ac..65048f9 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -57,20 +57,26 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl)
> BUG_ON(current->journal_info);
> current->journal_info = &tr;
>
> - gfs2_log_lock(sdp);
> + gfs2_ail_lock(sdp);
^^^^ this abstraction of a spinlock is left over from the old
gfs1 code. I'd prefer when adding new locks just to use spinlock(&....)
directly, rather than abstracting it out like this. That way we don't
have to think about what kind of lock it is.

[snip]
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 0fe2f3c..342d65e 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -80,7 +80,7 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
> mark_buffer_dirty(bh);
> clear_buffer_pinned(bh);
>
> - gfs2_log_lock(sdp);
> + gfs2_ail_lock(sdp);
> if (bd->bd_ail) {
> list_del(&bd->bd_ail_st_list);
> brelse(bh);
> @@ -91,6 +91,9 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
> }
> bd->bd_ail = ai;
> list_add(&bd->bd_ail_st_list, &ai->ai_ail1_list);
> + gfs2_ail_unlock(sdp);
> +
> + gfs2_log_lock(sdp);
> clear_bit(GLF_LFLUSH, &bd->bd_gl->gl_flags);
> trace_gfs2_pin(bd, 0);
> gfs2_log_unlock(sdp);
I don't think the gfs2_log_lock() is actually required at this point.
the LFLUSH bit is protected by the sd_log_flush_lock rwsem
and the tracing doesn't need the log lock either,

Steve.
 
Old 02-06-2010, 01:34 AM
Dave Chinner
 
Default gfs2: introduce AIL lock

On Fri, Feb 05, 2010 at 11:11:48AM +0000, Steven Whitehouse wrote:
> Hi,
>
> On Fri, 2010-02-05 at 16:45 +1100, Dave Chinner wrote:
> > THe log lock is currently used to protect the AIL lists and
> > the movements of buffers into and out of them. The lists
> > are self contained and no log specific items outside the
> > lists are accessed when starting or emptying the AIL lists.
> >
> > Hence the operation of the AIL does not require the protection
> > of the log lock so split them out into a new AIL specific lock
> > to reduce the amount of traffic on the log lock. This will
> > also reduce the amount of serialisation that occurs when
> > the gfs2_logd pushes on the AIL to move it forward.
> >
> > This reduces the impact of log pushing on sequential write
> > throughput. On no-op scheduler on a disk that can do 85MB/s,
> > this increases the write rate from 65MB/s with the ordering
> > fixes to 75MB/s.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
>
> This looks good, but a couple of comments:
>
> > diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> > index 78554ac..65048f9 100644
> > --- a/fs/gfs2/glops.c
> > +++ b/fs/gfs2/glops.c
> > @@ -57,20 +57,26 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl)
> > BUG_ON(current->journal_info);
> > current->journal_info = &tr;
> >
> > - gfs2_log_lock(sdp);
> > + gfs2_ail_lock(sdp);
> ^^^^ this abstraction of a spinlock is left over from the old
> gfs1 code. I'd prefer when adding new locks just to use spinlock(&....)
> directly, rather than abstracting it out like this. That way we don't
> have to think about what kind of lock it is.

Cool. I wondered about that - I was going to just ignore the
wrappers and put direct calls in, but I thought it might be better
to just start by following the existing convention. I'll respin this
without the wrappers.



> [snip]
> > @@ -91,6 +91,9 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
> > }
> > bd->bd_ail = ai;
> > list_add(&bd->bd_ail_st_list, &ai->ai_ail1_list);
> > + gfs2_ail_unlock(sdp);
> > +
> > + gfs2_log_lock(sdp);
> > clear_bit(GLF_LFLUSH, &bd->bd_gl->gl_flags);
> > trace_gfs2_pin(bd, 0);
> > gfs2_log_unlock(sdp);
> I don't think the gfs2_log_lock() is actually required at this point.
> the LFLUSH bit is protected by the sd_log_flush_lock rwsem
> and the tracing doesn't need the log lock either,

Ok, I wasn't sure how that bit was protected, so I left it with the
same protection as it had before. I'll kill the log lock from there
entirely.

Cheers,

Dave.
--
Dave Chinner
dchinner@redhat.com
 
Old 03-11-2011, 11:23 AM
Steven Whitehouse
 
Default GFS2: introduce AIL lock

Hi,

I'm not sure how this patch got missed out, but for some reason I didn't
pick it up originally. This is a port of the original patch to the
latest upstream kernel, plus a couple of tweeks that I suggested in the
original review.

Dave, if you've no objections, then I'd like to push this into my -nmw
git tree for the next merge.

Steve.

>From d6a079e82efd5fcbb1c7295f22e123c2cc748018 Mon Sep 17 00:00:00 2001
From: Dave Chinner <dchinner@redhat.com>
Date: Fri, 11 Mar 2011 11:52:25 +0000
Subject: [PATCH] GFS2: introduce AIL lock

The log lock is currently used to protect the AIL lists and
the movements of buffers into and out of them. The lists
are self contained and no log specific items outside the
lists are accessed when starting or emptying the AIL lists.

Hence the operation of the AIL does not require the protection
of the log lock so split them out into a new AIL specific lock
to reduce the amount of traffic on the log lock. This will
also reduce the amount of serialisation that occurs when
the gfs2_logd pushes on the AIL to move it forward.

This reduces the impact of log pushing on sequential write
throughput.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index ac5fac9..3754e3c 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -56,20 +56,26 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl)
BUG_ON(current->journal_info);
current->journal_info = &tr;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);
while (!list_empty(head)) {
bd = list_entry(head->next, struct gfs2_bufdata,
bd_ail_gl_list);
bh = bd->bd_bh;
gfs2_remove_from_ail(bd);
+ spin_unlock(&sdp->sd_ail_lock);
+
bd->bd_bh = NULL;
bh->b_private = NULL;
bd->bd_blkno = bh->b_blocknr;
+ gfs2_log_lock(sdp);
gfs2_assert_withdraw(sdp, !buffer_busy(bh));
gfs2_trans_add_revoke(sdp, bd);
+ gfs2_log_unlock(sdp);
+
+ spin_lock(&sdp->sd_ail_lock);
}
gfs2_assert_withdraw(sdp, !atomic_read(&gl->gl_ail_count));
- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);

gfs2_trans_end(sdp);
gfs2_log_flush(sdp, NULL);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 59aaaa0..870a89d 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -651,6 +651,7 @@ struct gfs2_sbd {
unsigned int sd_log_flush_head;
u64 sd_log_flush_wrapped;

+ spinlock_t sd_ail_lock;
struct list_head sd_ail1_list;
struct list_head sd_ail2_list;
u64 sd_ail_sync_gen;
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index eb01f35..4e3c044 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -88,8 +88,8 @@ void gfs2_remove_from_ail(struct gfs2_bufdata *bd)
*/

static void gfs2_ail1_start_one(struct gfs2_sbd *sdp, struct gfs2_ail *ai)
-__releases(&sdp->sd_log_lock)
-__acquires(&sdp->sd_log_lock)
+__releases(&sdp->sd_ail_lock)
+__acquires(&sdp->sd_ail_lock)
{
struct gfs2_bufdata *bd, *s;
struct buffer_head *bh;
@@ -117,7 +117,7 @@ __acquires(&sdp->sd_log_lock)
list_move(&bd->bd_ail_st_list, &ai->ai_ail1_list);

get_bh(bh);
- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
lock_buffer(bh);
if (test_clear_buffer_dirty(bh)) {
bh->b_end_io = end_buffer_write_sync;
@@ -126,7 +126,7 @@ __acquires(&sdp->sd_log_lock)
unlock_buffer(bh);
brelse(bh);
}
- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

retry = 1;
break;
@@ -175,10 +175,10 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
struct gfs2_ail *ai;
int done = 0;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);
head = &sdp->sd_ail1_list;
if (list_empty(head)) {
- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
return;
}
sync_gen = sdp->sd_ail_sync_gen++;
@@ -189,13 +189,13 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
if (ai->ai_sync_gen >= sync_gen)
continue;
ai->ai_sync_gen = sync_gen;
- gfs2_ail1_start_one(sdp, ai); /* This may drop log lock */
+ gfs2_ail1_start_one(sdp, ai); /* This may drop ail lock */
done = 0;
break;
}
}

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
}

static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)
@@ -203,7 +203,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)
struct gfs2_ail *ai, *s;
int ret;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

list_for_each_entry_safe_reverse(ai, s, &sdp->sd_ail1_list, ai_list) {
if (gfs2_ail1_empty_one(sdp, ai, flags))
@@ -214,7 +214,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)

ret = list_empty(&sdp->sd_ail1_list);

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);

return ret;
}
@@ -247,7 +247,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int new_tail)
int wrap = (new_tail < old_tail);
int a, b, rm;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

list_for_each_entry_safe(ai, safe, &sdp->sd_ail2_list, ai_list) {
a = (old_tail <= ai->ai_first);
@@ -263,7 +263,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int new_tail)
kfree(ai);
}

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
}

/**
@@ -421,7 +421,7 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
struct gfs2_ail *ai;
unsigned int tail;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

if (list_empty(&sdp->sd_ail1_list)) {
tail = sdp->sd_log_head;
@@ -430,7 +430,7 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
tail = ai->ai_first;
}

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);

return tail;
}
@@ -743,10 +743,12 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl)
sdp->sd_log_commited_databuf = 0;
sdp->sd_log_commited_revoke = 0;

+ spin_lock(&sdp->sd_ail_lock);
if (!list_empty(&ai->ai_ail1_list)) {
list_add(&ai->ai_list, &sdp->sd_ail1_list);
ai = NULL;
}
+ spin_unlock(&sdp->sd_ail_lock);
gfs2_log_unlock(sdp);
trace_gfs2_log_flush(sdp, 0);
up_write(&sdp->sd_log_flush_lock);
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 11a73ef..4295a6a 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -80,7 +80,7 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
mark_buffer_dirty(bh);
clear_buffer_pinned(bh);

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);
if (bd->bd_ail) {
list_del(&bd->bd_ail_st_list);
brelse(bh);
@@ -91,10 +91,11 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
}
bd->bd_ail = ai;
list_add(&bd->bd_ail_st_list, &ai->ai_ail1_list);
+ spin_unlock(&sdp->sd_ail_lock);
+
if (test_and_clear_bit(GLF_LFLUSH, &bd->bd_gl->gl_flags))
gfs2_glock_schedule_for_reclaim(bd->bd_gl);
trace_gfs2_pin(bd, 0);
- gfs2_log_unlock(sdp);
unlock_buffer(bh);
atomic_dec(&sdp->sd_log_pinned);
}
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 67654d0..42ef243 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -99,6 +99,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)

init_waitqueue_head(&sdp->sd_log_waitq);
init_waitqueue_head(&sdp->sd_logd_waitq);
+ spin_lock_init(&sdp->sd_ail_lock);
INIT_LIST_HEAD(&sdp->sd_ail1_list);
INIT_LIST_HEAD(&sdp->sd_ail2_list);

--
1.7.4
 
Old 03-13-2011, 09:44 PM
Dave Chinner
 
Default GFS2: introduce AIL lock

On Fri, Mar 11, 2011 at 12:23:42PM +0000, Steven Whitehouse wrote:
> Hi,
>
> I'm not sure how this patch got missed out, but for some reason I didn't
> pick it up originally. This is a port of the original patch to the
> latest upstream kernel, plus a couple of tweeks that I suggested in the
> original review.

Ah, I thought you took it in with the other patches at the time, so
I didn't follow up. Oh well, at lesat it didn't quite fall through
the crack.

> Dave, if you've no objections, then I'd like to push this into my -nmw
> git tree for the next merge.

No objections. I just did a quick check of the patch again and I
can't see anything obviously wrong with it, so queue it up

Cheers,

Dave.
--
Dave Chinner
dchinner@redhat.com
 
Old 03-14-2011, 12:12 PM
Steven Whitehouse
 
Default GFS2: introduce AIL lock

Hi,

On Mon, 2011-03-14 at 09:44 +1100, Dave Chinner wrote:
> On Fri, Mar 11, 2011 at 12:23:42PM +0000, Steven Whitehouse wrote:
> > Hi,
> >
> > I'm not sure how this patch got missed out, but for some reason I didn't
> > pick it up originally. This is a port of the original patch to the
> > latest upstream kernel, plus a couple of tweeks that I suggested in the
> > original review.
>
> Ah, I thought you took it in with the other patches at the time, so
> I didn't follow up. Oh well, at lesat it didn't quite fall through
> the crack.
>
Me too I discovered it when I was reviewing your comments while
investigating performance issues.

> > Dave, if you've no objections, then I'd like to push this into my -nmw
> > git tree for the next merge.
>
> No objections. I just did a quick check of the patch again and I
> can't see anything obviously wrong with it, so queue it up
>
> Cheers,
>
> Dave.
Ok, done. Also I'm intending to add the below to cover a few loose ends
which I found on Friday. I think it should be good to go after then,


Steve.


>From c618e87a5fd02aaad006c12d5a80a231dfa39250 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Mon, 14 Mar 2011 12:40:29 +0000
Subject: [PATCH] GFS2: Update to AIL list locking

The previous patch missed a couple of places where the AIL list
needed locking, so this fixes up those places, plus a comment
is corrected too.

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
Cc: Dave Chinner <dchinner@redhat.com>

diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 4e3c044..e7ed31f 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -67,7 +67,7 @@ unsigned int gfs2_struct2blk(struct gfs2_sbd *sdp, unsigned int nstruct,
* @mapping: The associated mapping (maybe NULL)
* @bd: The gfs2_bufdata to remove
*
- * The log lock _must_ be held when calling this function
+ * The ail lock _must_ be held when calling this function
*
*/

diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 4295a6a..e919abf 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -51,8 +51,10 @@ static void gfs2_pin(struct gfs2_sbd *sdp, struct buffer_head *bh)
/* If this buffer is in the AIL and it has already been written
* to in-place disk block, remove it from the AIL.
*/
+ spin_lock(&sdp->sd_ail_lock);
if (bd->bd_ail)
list_move(&bd->bd_ail_st_list, &bd->bd_ail->ai_ail2_list);
+ spin_unlock(&sdp->sd_ail_lock);
get_bh(bh);
atomic_inc(&sdp->sd_log_pinned);
trace_gfs2_pin(bd, 1);
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 939739c..01d97f4 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -326,6 +326,7 @@ void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int
brelse(bh);
}
if (bd) {
+ spin_lock(&sdp->sd_ail_lock);
if (bd->bd_ail) {
gfs2_remove_from_ail(bd);
bh->b_private = NULL;
@@ -333,6 +334,7 @@ void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int
bd->bd_blkno = bh->b_blocknr;
gfs2_trans_add_revoke(sdp, bd);
}
+ spin_unlock(&sdp->sd_ail_lock);
}
clear_buffer_dirty(bh);
clear_buffer_uptodate(bh);
--
1.7.4
 
Old 03-15-2011, 08:11 AM
Steven Whitehouse
 
Default GFS2: introduce AIL lock

From: Dave Chinner <dchinner@redhat.com>

The log lock is currently used to protect the AIL lists and
the movements of buffers into and out of them. The lists
are self contained and no log specific items outside the
lists are accessed when starting or emptying the AIL lists.

Hence the operation of the AIL does not require the protection
of the log lock so split them out into a new AIL specific lock
to reduce the amount of traffic on the log lock. This will
also reduce the amount of serialisation that occurs when
the gfs2_logd pushes on the AIL to move it forward.

This reduces the impact of log pushing on sequential write
throughput.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index ac5fac9..3754e3c 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -56,20 +56,26 @@ static void gfs2_ail_empty_gl(struct gfs2_glock *gl)
BUG_ON(current->journal_info);
current->journal_info = &tr;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);
while (!list_empty(head)) {
bd = list_entry(head->next, struct gfs2_bufdata,
bd_ail_gl_list);
bh = bd->bd_bh;
gfs2_remove_from_ail(bd);
+ spin_unlock(&sdp->sd_ail_lock);
+
bd->bd_bh = NULL;
bh->b_private = NULL;
bd->bd_blkno = bh->b_blocknr;
+ gfs2_log_lock(sdp);
gfs2_assert_withdraw(sdp, !buffer_busy(bh));
gfs2_trans_add_revoke(sdp, bd);
+ gfs2_log_unlock(sdp);
+
+ spin_lock(&sdp->sd_ail_lock);
}
gfs2_assert_withdraw(sdp, !atomic_read(&gl->gl_ail_count));
- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);

gfs2_trans_end(sdp);
gfs2_log_flush(sdp, NULL);
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 59aaaa0..870a89d 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -651,6 +651,7 @@ struct gfs2_sbd {
unsigned int sd_log_flush_head;
u64 sd_log_flush_wrapped;

+ spinlock_t sd_ail_lock;
struct list_head sd_ail1_list;
struct list_head sd_ail2_list;
u64 sd_ail_sync_gen;
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index eb01f35..4e3c044 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -88,8 +88,8 @@ void gfs2_remove_from_ail(struct gfs2_bufdata *bd)
*/

static void gfs2_ail1_start_one(struct gfs2_sbd *sdp, struct gfs2_ail *ai)
-__releases(&sdp->sd_log_lock)
-__acquires(&sdp->sd_log_lock)
+__releases(&sdp->sd_ail_lock)
+__acquires(&sdp->sd_ail_lock)
{
struct gfs2_bufdata *bd, *s;
struct buffer_head *bh;
@@ -117,7 +117,7 @@ __acquires(&sdp->sd_log_lock)
list_move(&bd->bd_ail_st_list, &ai->ai_ail1_list);

get_bh(bh);
- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
lock_buffer(bh);
if (test_clear_buffer_dirty(bh)) {
bh->b_end_io = end_buffer_write_sync;
@@ -126,7 +126,7 @@ __acquires(&sdp->sd_log_lock)
unlock_buffer(bh);
brelse(bh);
}
- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

retry = 1;
break;
@@ -175,10 +175,10 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
struct gfs2_ail *ai;
int done = 0;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);
head = &sdp->sd_ail1_list;
if (list_empty(head)) {
- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
return;
}
sync_gen = sdp->sd_ail_sync_gen++;
@@ -189,13 +189,13 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
if (ai->ai_sync_gen >= sync_gen)
continue;
ai->ai_sync_gen = sync_gen;
- gfs2_ail1_start_one(sdp, ai); /* This may drop log lock */
+ gfs2_ail1_start_one(sdp, ai); /* This may drop ail lock */
done = 0;
break;
}
}

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
}

static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)
@@ -203,7 +203,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)
struct gfs2_ail *ai, *s;
int ret;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

list_for_each_entry_safe_reverse(ai, s, &sdp->sd_ail1_list, ai_list) {
if (gfs2_ail1_empty_one(sdp, ai, flags))
@@ -214,7 +214,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int flags)

ret = list_empty(&sdp->sd_ail1_list);

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);

return ret;
}
@@ -247,7 +247,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int new_tail)
int wrap = (new_tail < old_tail);
int a, b, rm;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

list_for_each_entry_safe(ai, safe, &sdp->sd_ail2_list, ai_list) {
a = (old_tail <= ai->ai_first);
@@ -263,7 +263,7 @@ static void ail2_empty(struct gfs2_sbd *sdp, unsigned int new_tail)
kfree(ai);
}

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);
}

/**
@@ -421,7 +421,7 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
struct gfs2_ail *ai;
unsigned int tail;

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);

if (list_empty(&sdp->sd_ail1_list)) {
tail = sdp->sd_log_head;
@@ -430,7 +430,7 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
tail = ai->ai_first;
}

- gfs2_log_unlock(sdp);
+ spin_unlock(&sdp->sd_ail_lock);

return tail;
}
@@ -743,10 +743,12 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl)
sdp->sd_log_commited_databuf = 0;
sdp->sd_log_commited_revoke = 0;

+ spin_lock(&sdp->sd_ail_lock);
if (!list_empty(&ai->ai_ail1_list)) {
list_add(&ai->ai_list, &sdp->sd_ail1_list);
ai = NULL;
}
+ spin_unlock(&sdp->sd_ail_lock);
gfs2_log_unlock(sdp);
trace_gfs2_log_flush(sdp, 0);
up_write(&sdp->sd_log_flush_lock);
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 11a73ef..4295a6a 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -80,7 +80,7 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
mark_buffer_dirty(bh);
clear_buffer_pinned(bh);

- gfs2_log_lock(sdp);
+ spin_lock(&sdp->sd_ail_lock);
if (bd->bd_ail) {
list_del(&bd->bd_ail_st_list);
brelse(bh);
@@ -91,10 +91,11 @@ static void gfs2_unpin(struct gfs2_sbd *sdp, struct buffer_head *bh,
}
bd->bd_ail = ai;
list_add(&bd->bd_ail_st_list, &ai->ai_ail1_list);
+ spin_unlock(&sdp->sd_ail_lock);
+
if (test_and_clear_bit(GLF_LFLUSH, &bd->bd_gl->gl_flags))
gfs2_glock_schedule_for_reclaim(bd->bd_gl);
trace_gfs2_pin(bd, 0);
- gfs2_log_unlock(sdp);
unlock_buffer(bh);
atomic_dec(&sdp->sd_log_pinned);
}
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index 67654d0..42ef243 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -99,6 +99,7 @@ static struct gfs2_sbd *init_sbd(struct super_block *sb)

init_waitqueue_head(&sdp->sd_log_waitq);
init_waitqueue_head(&sdp->sd_logd_waitq);
+ spin_lock_init(&sdp->sd_ail_lock);
INIT_LIST_HEAD(&sdp->sd_ail1_list);
INIT_LIST_HEAD(&sdp->sd_ail2_list);

--
1.7.4
 

Thread Tools




All times are GMT. The time now is 06:16 AM.

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