From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 29 Jan 2008 08:33:08 +0000 Subject: [Cluster-devel] [GFS2 PATCH] Only do lo_incore_commit once In-Reply-To: <1201540810.18461.82.camel@technetium.msp.redhat.com> References: <1201540810.18461.82.camel@technetium.msp.redhat.com> Message-ID: <1201595588.22038.318.camel@quoit> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hi, Now in the -nmw tree. Thanks, Steve. On Mon, 2008-01-28 at 11:20 -0600, Bob Peterson wrote: > Hi, > > This patch is performance related. When we're doing a log flush, > I noticed we were calling buf_lo_incore_commit twice: once for > data bufs and once for metadata bufs. Since this is the same > function and does the same thing in both cases, there should be > no reason to call it twice. Since we only need to call it once, > we can also make it faster by removing it from the generic "lops" > code and making it a stand-along static function. > > Regards, > > Bob Peterson > Red Hat GFS > > Signed-off-by: Bob Peterson > -- > fs/gfs2/incore.h | 1 - > fs/gfs2/log.c | 17 ++++++++++++++++- > fs/gfs2/lops.c | 17 ----------------- > fs/gfs2/lops.h | 11 +---------- > 4 files changed, 17 insertions(+), 28 deletions(-) > > diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h > index 010eb70..c1518f0 100644 > --- a/fs/gfs2/incore.h > +++ b/fs/gfs2/incore.h > @@ -44,7 +44,6 @@ struct gfs2_log_header_host { > > struct gfs2_log_operations { > void (*lo_add) (struct gfs2_sbd *sdp, struct gfs2_log_element *le); > - void (*lo_incore_commit) (struct gfs2_sbd *sdp, struct gfs2_trans *tr); > void (*lo_before_commit) (struct gfs2_sbd *sdp); > void (*lo_after_commit) (struct gfs2_sbd *sdp, struct gfs2_ail *ai); > void (*lo_before_scan) (struct gfs2_jdesc *jd, > diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c > index 161ab6f..b335304 100644 > --- a/fs/gfs2/log.c > +++ b/fs/gfs2/log.c > @@ -779,6 +779,21 @@ static void log_refund(struct gfs2_sbd *sdp, struct gfs2_trans *tr) > gfs2_log_unlock(sdp); > } > > +static void buf_lo_incore_commit(struct gfs2_sbd *sdp, struct gfs2_trans *tr) > +{ > + struct list_head *head = &tr->tr_list_buf; > + struct gfs2_bufdata *bd; > + > + gfs2_log_lock(sdp); > + while (!list_empty(head)) { > + bd = list_entry(head->next, struct gfs2_bufdata, bd_list_tr); > + list_del_init(&bd->bd_list_tr); > + tr->tr_num_buf--; > + } > + gfs2_log_unlock(sdp); > + gfs2_assert_warn(sdp, !tr->tr_num_buf); > +} > + > /** > * gfs2_log_commit - Commit a transaction to the log > * @sdp: the filesystem > @@ -790,7 +805,7 @@ static void log_refund(struct gfs2_sbd *sdp, struct gfs2_trans *tr) > void gfs2_log_commit(struct gfs2_sbd *sdp, struct gfs2_trans *tr) > { > log_refund(sdp, tr); > - lops_incore_commit(sdp, tr); > + buf_lo_incore_commit(sdp, tr); > > sdp->sd_vfs->s_dirt = 1; > up_read(&sdp->sd_log_flush_lock); > diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c > index fae59d6..7138737 100644 > --- a/fs/gfs2/lops.c > +++ b/fs/gfs2/lops.c > @@ -152,21 +152,6 @@ out: > unlock_buffer(bd->bd_bh); > } > > -static void buf_lo_incore_commit(struct gfs2_sbd *sdp, struct gfs2_trans *tr) > -{ > - struct list_head *head = &tr->tr_list_buf; > - struct gfs2_bufdata *bd; > - > - gfs2_log_lock(sdp); > - while (!list_empty(head)) { > - bd = list_entry(head->next, struct gfs2_bufdata, bd_list_tr); > - list_del_init(&bd->bd_list_tr); > - tr->tr_num_buf--; > - } > - gfs2_log_unlock(sdp); > - gfs2_assert_warn(sdp, !tr->tr_num_buf); > -} > - > static void buf_lo_before_commit(struct gfs2_sbd *sdp) > { > struct buffer_head *bh; > @@ -737,7 +722,6 @@ static void databuf_lo_after_commit(struct gfs2_sbd *sdp, struct gfs2_ail *ai) > > const struct gfs2_log_operations gfs2_buf_lops = { > .lo_add = buf_lo_add, > - .lo_incore_commit = buf_lo_incore_commit, > .lo_before_commit = buf_lo_before_commit, > .lo_after_commit = buf_lo_after_commit, > .lo_before_scan = buf_lo_before_scan, > @@ -763,7 +747,6 @@ const struct gfs2_log_operations gfs2_rg_lops = { > > const struct gfs2_log_operations gfs2_databuf_lops = { > .lo_add = databuf_lo_add, > - .lo_incore_commit = buf_lo_incore_commit, > .lo_before_commit = databuf_lo_before_commit, > .lo_after_commit = databuf_lo_after_commit, > .lo_scan_elements = databuf_lo_scan_elements, > diff --git a/fs/gfs2/lops.h b/fs/gfs2/lops.h > index 41a00df..3c0b273 100644 > --- a/fs/gfs2/lops.h > +++ b/fs/gfs2/lops.h > @@ -1,6 +1,6 @@ > /* > * Copyright (C) Sistina Software, Inc. 1997-2003 All rights reserved. > - * Copyright (C) 2004-2006 Red Hat, Inc. All rights reserved. > + * Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved. > * > * This copyrighted material is made available to anyone wishing to use, > * modify, copy, or redistribute it subject to the terms and conditions > @@ -57,15 +57,6 @@ static inline void lops_add(struct gfs2_sbd *sdp, struct gfs2_log_element *le) > le->le_ops->lo_add(sdp, le); > } > > -static inline void lops_incore_commit(struct gfs2_sbd *sdp, > - struct gfs2_trans *tr) > -{ > - int x; > - for (x = 0; gfs2_log_ops[x]; x++) > - if (gfs2_log_ops[x]->lo_incore_commit) > - gfs2_log_ops[x]->lo_incore_commit(sdp, tr); > -} > - > static inline void lops_before_commit(struct gfs2_sbd *sdp) > { > int x; > >