[Cluster-devel] [GFS2] Move/rename gfs2_attach_bufdata etc.

Steven Whitehouse swhiteho at redhat.com
Wed Dec 12 16:29:37 UTC 2007


Hi,

Sorry, this one and the previously posted one are out of order,
this is the third patch in the series,

Steve.

>From af344b239a96ab3090fc82478d04e45a17b80188 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse <swhiteho at redhat.com>
Date: Tue, 4 Dec 2007 09:29:30 +0000
Subject: [PATCH] [GFS2] Move/rename gfs2_attach_bufdata etc.

The function gfs2_attach_bufdata was doing most of the work for
gfs2_trans_add_bh so that by moving it into trans.c from meta_io.c
and then renaming it gfs2_trans_add_bh, the original gfs2_trans_add_bh
is no longer needed. The extra check that gfs2_trans_add_bh was doing
is now moved into the renamed version of gfs2_attach_bufdata.

All this means that the check for bh->b_private being a gfs2_bufdata
or a gfs2_glock is now under the page lock, thus allowing the tracking
of glocks for all metadata buffers, by extending the functions which
were introduced in the previous patch.

Since the locking is subtle, I've added a comment in incore.h to
explain it.

Signed-off-by: Steven Whitehouse <swhiteho at redhat.com>

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 4030e30..b2487cf 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -103,12 +103,15 @@ struct gfs2_rgrpd {
 enum gfs2_state_bits {
 	BH_Pinned = BH_PrivateStart,
 	BH_Escaped = BH_PrivateStart + 1,
+	BH_Glock = BH_PrivateStart + 2,
 };
 
 BUFFER_FNS(Pinned, pinned)
 TAS_BUFFER_FNS(Pinned, pinned)
 BUFFER_FNS(Escaped, escaped)
 TAS_BUFFER_FNS(Escaped, escaped)
+BUFFER_FNS(Glock, glock)
+TAS_BUFFER_FNS(Glock, glock)
 
 struct gfs2_bufdata {
 	struct buffer_head *bd_bh;
@@ -128,8 +131,37 @@ struct gfs2_bufdata {
 	struct list_head bd_ail_gl_list;
 };
 
+/*
+ * The locking for these two bh_to_xxx functions and the associated
+ * BH_Glock bit is subtle. The page lock is used in getbuf and also
+ * in gfs2_trans_add_bh to provide mutual exclusion, however after
+ * a buffer has been journaled there are places which access the
+ * gfs2_bufdata with no locking. Thats ok, because due to the lifetime
+ * rules for the bh, any journaled bh must have a gfs2_bufdata and
+ * the gfs2_bufdata will only be freed when the buffer_head is freed.
+ *
+ * So we have (for metadata only):
+ * On creation via getbuf: bh->b_private points to a glock
+ * After gfs2_trans_add_bh: bh->b_private points to a gfs2_bufdata
+ *
+ * For data:
+ * On creation: bh->b_private is NULL
+ * If journaled, or ordered write: bh->b_private points to gfs2_bufdata
+ *
+ */
+
 static inline struct gfs2_bufdata *bh_to_bufdata(const struct buffer_head *bh)
 {
+	if (!buffer_glock(bh))
+		return bh->b_private;
+	return NULL;
+}
+
+static inline struct gfs2_glock *bh_to_glock(const struct buffer_head *bh)
+{
+	const struct gfs2_bufdata *bd = bh_to_bufdata(bh);
+	if (bd)
+		return bd->bd_gl;
 	return bh->b_private;
 }
 
diff --git a/fs/gfs2/meta_io.c b/fs/gfs2/meta_io.c
index 7410514..2ffee4f 100644
--- a/fs/gfs2/meta_io.c
+++ b/fs/gfs2/meta_io.c
@@ -173,6 +173,11 @@ static struct buffer_head *getbuf(struct gfs2_glock *gl, u64 blkno, int create)
 	if (!buffer_mapped(bh))
 		map_bh(bh, sdp->sd_vfs, blkno);
 
+	if (!bh_to_glock(bh)) {
+		bh->b_private = gl;
+		set_buffer_glock(bh);
+	}
+
 	unlock_page(page);
 	mark_page_accessed(page);
 	page_cache_release(page);
@@ -262,42 +267,6 @@ int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh)
 	return 0;
 }
 
-/**
- * gfs2_attach_bufdata - attach a struct gfs2_bufdata structure to a buffer
- * @gl: the glock the buffer belongs to
- * @bh: The buffer to be attached to
- * @meta: Flag to indicate whether its metadata or not
- */
-
-void gfs2_attach_bufdata(struct gfs2_glock *gl, struct buffer_head *bh,
-			 int meta)
-{
-	struct gfs2_bufdata *bd;
-
-	if (meta)
-		lock_page(bh->b_page);
-
-	if (bh_to_bufdata(bh)) {
-		if (meta)
-			unlock_page(bh->b_page);
-		return;
-	}
-
-	bd = kmem_cache_zalloc(gfs2_bufdata_cachep, GFP_NOFS | __GFP_NOFAIL),
-	bd->bd_bh = bh;
-	bd->bd_gl = gl;
-
-	INIT_LIST_HEAD(&bd->bd_list_tr);
-	if (meta)
-		lops_init_le(&bd->bd_le, &gfs2_buf_lops);
-	else
-		lops_init_le(&bd->bd_le, &gfs2_databuf_lops);
-	bh->b_private = bd;
-
-	if (meta)
-		unlock_page(bh->b_page);
-}
-
 void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr, int meta)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(bh->b_page->mapping->host);
diff --git a/fs/gfs2/meta_io.h b/fs/gfs2/meta_io.h
index 73e3b1c..dba789c 100644
--- a/fs/gfs2/meta_io.h
+++ b/fs/gfs2/meta_io.h
@@ -48,9 +48,6 @@ int gfs2_meta_read(struct gfs2_glock *gl, u64 blkno,
 		   int flags, struct buffer_head **bhp);
 int gfs2_meta_wait(struct gfs2_sbd *sdp, struct buffer_head *bh);
 
-void gfs2_attach_bufdata(struct gfs2_glock *gl, struct buffer_head *bh,
-			 int meta);
-
 void gfs2_remove_from_journal(struct buffer_head *bh, struct gfs2_trans *tr,
 			      int meta);
 
diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index c1fdc1d..2a4d414 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -131,25 +131,40 @@ void gfs2_trans_end(struct gfs2_sbd *sdp)
 }
 
 /**
- * gfs2_trans_add_bh - Add a to-be-modified buffer to the current transaction
+ * gfs2_trans_add_bh - Add a buffer head into the journal
  * @gl: the glock the buffer belongs to
- * @bh: The buffer to add
- * @meta: True in the case of adding metadata
- *
+ * @bh: The buffer to be added
+ * @meta: Flag to indicate whether its metadata or not
  */
 
 void gfs2_trans_add_bh(struct gfs2_glock *gl, struct buffer_head *bh, int meta)
 {
-	struct gfs2_sbd *sdp = gl->gl_sbd;
 	struct gfs2_bufdata *bd;
+	struct gfs2_sbd *sdp = gl->gl_sbd;
+
+	if (meta)
+		lock_page(bh->b_page);
 
 	bd = bh_to_bufdata(bh);
-	if (bd)
-		gfs2_assert(sdp, bd->bd_gl == gl);
-	else {
-		gfs2_attach_bufdata(gl, bh, meta);
-		bd = bh_to_bufdata(bh);
+	if (bd) {
+		BUG_ON(bd->bd_gl != gl);
+		goto out;
 	}
+
+	bd = kmem_cache_zalloc(gfs2_bufdata_cachep, GFP_NOFS | __GFP_NOFAIL),
+	bd->bd_bh = bh;
+	bd->bd_gl = gl;
+
+	INIT_LIST_HEAD(&bd->bd_list_tr);
+	if (meta)
+		lops_init_le(&bd->bd_le, &gfs2_buf_lops);
+	else
+		lops_init_le(&bd->bd_le, &gfs2_databuf_lops);
+	clear_buffer_glock(bh);
+	bh->b_private = bd;
+out:
+	if (meta)
+		unlock_page(bh->b_page);
 	lops_add(sdp, &bd->bd_le);
 }
 
-- 
1.5.1.2






More information about the Cluster-devel mailing list