[Cluster-devel] GFS2: Don't assign b_private until we know it's safe

[Cluster-devel] GFS2: Don't assign b_private until we know it's safe

[Bob Peterson]

Hi,

Before this patch, functions gfs2_trans_add_meta and _data would
assign bh->b_private by calling gfs2_alloc_bufdata, despite the
fact that locks to protect the value are dropped, and instead, a
page lock is taken. This patch removes the page lock and adds
checks to see if someone else assigned the value after the proper
locks have been restored.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 fs/gfs2/trans.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/gfs2/trans.c b/fs/gfs2/trans.c
index c75cacaa349b..32debf17c8e2 100644
--- a/fs/gfs2/trans.c
+++ b/fs/gfs2/trans.c
@@ -134,7 +134,6 @@ static struct gfs2_bufdata *gfs2_alloc_bufdata(struct gfs2_glock *gl,
 	bd->bd_gl = gl;
 	bd->bd_ops = lops;
 	INIT_LIST_HEAD(&bd->bd_list);
-	bh->b_private = bd;
 	return bd;
 }
 
@@ -179,12 +178,15 @@ void gfs2_trans_add_data(struct gfs2_glock *gl, struct buffer_head *bh)
 	if (bd == NULL) {
 		gfs2_log_unlock(sdp);
 		unlock_buffer(bh);
-		if (bh->b_private == NULL)
-			bd = gfs2_alloc_bufdata(gl, bh, &gfs2_databuf_lops);
-		else
-			bd = bh->b_private;
+		bd = gfs2_alloc_bufdata(gl, bh, &gfs2_databuf_lops);
 		lock_buffer(bh);
 		gfs2_log_lock(sdp);
+		if (bh->b_private) { /* preempted by someone else */
+			kmem_cache_free(gfs2_bufdata_cachep, bd);
+			bd = bh->b_private;
+		} else {
+			bh->b_private = bd;
+		}
 	}
 	gfs2_assert(sdp, bd->bd_gl == gl);
 	set_bit(TR_TOUCHED, &tr->tr_flags);
@@ -219,14 +221,15 @@ void gfs2_trans_add_meta(struct gfs2_glock *gl, struct buffer_head *bh)
 	if (bd == NULL) {
 		gfs2_log_unlock(sdp);
 		unlock_buffer(bh);
-		lock_page(bh->b_page);
-		if (bh->b_private == NULL)
-			bd = gfs2_alloc_bufdata(gl, bh, &gfs2_buf_lops);
-		else
-			bd = bh->b_private;
-		unlock_page(bh->b_page);
+		bd = gfs2_alloc_bufdata(gl, bh, &gfs2_buf_lops);
 		lock_buffer(bh);
 		gfs2_log_lock(sdp);
+		if (bh->b_private) { /* preempted by someone else */
+			kmem_cache_free(gfs2_bufdata_cachep, bd);
+			bd = bh->b_private;
+		} else {
+			bh->b_private = bd;
+		}
 	}
 	gfs2_assert(sdp, bd->bd_gl == gl);
 	set_bit(TR_TOUCHED, &tr->tr_flags);