Re: [Cluster-devel] [GFS2 PATCH] Only do lo_incore_commit once

2008-01-29 Thread Steven Whitehouse
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 [EMAIL PROTECTED] 
 --
  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)
 - 

[Cluster-devel] [GFS2 PATCH] Only do lo_incore_commit once

2008-01-28 Thread Bob Peterson
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 [EMAIL PROTECTED] 
--
 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;