[Cluster-devel] [PATCH] gfs2: rest of the fix for 238162

Benjamin Marzinski bmarzins at redhat.com
Wed Jun 6 07:20:50 UTC 2007


This is the patch for the last issues of bz #238162. The first issue is that
gfs2 was never actually writing unstuffed journaled datablocks to their in-place
location on disk. This is because gfs2_writepage() would always put the buffers
back on the long.  To fix this, gfs2_writepage() needs to distinguish between
pages that were dirtied through the normal file writes, and pages that were
dirtied by mmap, or some other alternative way.  I copied the ext3 solution to
this problem, and made gfs2 implement its own set_page_dirty function that
sets PageChecked.  This set_page_dirty function is called by mmap when it
dirties a page. In gfs2_writepage(), gfs2 checks PageChecked. If it is
set, it journals the buffers. If not, it simply writes them out.

The second issue is that on unmount, gfs2 can writeback and discard buffers
that are still on the active items list, causing gfs2_ail1_start_one() and
gfs2_ail1_empty_one() to crash because the bufdata is no longer associated with
any buffer.  To fix this, gfs2_ail1_start_one() and fs2_ail1_empty_one() now
check for this case, and simply move the bufdata to the ail2 list if it happens.

The final issue is that when gfs2 discards the buffers on unmount, it decouples
the bufdata from the buffers.  Because of this, some bufdata structs were
getting lost and never freed.  discard_buffer() and gfs2_ail2_empty_one() now
free the bufdata if it is about to get lost.

Due to time constraints, I didn't build or test this patch with the latest
code in the .git tree.  I tested it with code that was a couple of weeks old. I
also didn't test it as much as I would like to have in general. I plan on doing
that later today, after I get a couple hours of sleep.

Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
-------------- next part --------------
diff -urpN --exclude-from=gfs2-2.6-nmw-070530-clean/Documentation/dontdiff gfs2-2.6-nmw-070530-clean/fs/gfs2/log.c gfs2-2.6-nmw-070530/fs/gfs2/log.c
--- gfs2-2.6-nmw-070530-clean/fs/gfs2/log.c	2007-06-05 20:26:05.000000000 -0500
+++ gfs2-2.6-nmw-070530/fs/gfs2/log.c	2007-06-05 20:35:28.000000000 -0500
@@ -83,6 +83,11 @@ static void gfs2_ail1_start_one(struct g
 
 			gfs2_assert(sdp, bd->bd_ail == ai);
 
+			if (!bh){
+				list_move(&bd->bd_ail_st_list, &ai->ai_ail2_list);
+                                continue;
+                        }
+
 			if (!buffer_busy(bh)) {
 				if (!buffer_uptodate(bh)) {
 					gfs2_log_unlock(sdp);
@@ -125,6 +130,11 @@ static int gfs2_ail1_empty_one(struct gf
 					 bd_ail_st_list) {
 		bh = bd->bd_bh;
 
+		if (!bh){
+			list_move(&bd->bd_ail_st_list, &ai->ai_ail2_list);
+			continue;
+		}
+
 		gfs2_assert(sdp, bd->bd_ail == ai);
 
 		if (buffer_busy(bh)) {
@@ -227,7 +237,10 @@ static void gfs2_ail2_empty_one(struct g
 		list_del(&bd->bd_ail_st_list);
 		list_del(&bd->bd_ail_gl_list);
 		atomic_dec(&bd->bd_gl->gl_ail_count);
-		brelse(bd->bd_bh);
+		if (bd->bd_bh)
+			brelse(bd->bd_bh);
+		else
+			kmem_cache_free(gfs2_bufdata_cachep, bd);
 	}
 }
 
diff -urpN --exclude-from=gfs2-2.6-nmw-070530-clean/Documentation/dontdiff gfs2-2.6-nmw-070530-clean/fs/gfs2/ops_address.c gfs2-2.6-nmw-070530/fs/gfs2/ops_address.c
--- gfs2-2.6-nmw-070530-clean/fs/gfs2/ops_address.c	2007-06-05 20:26:05.000000000 -0500
+++ gfs2-2.6-nmw-070530/fs/gfs2/ops_address.c	2007-06-05 20:53:21.000000000 -0500
@@ -137,7 +137,9 @@ static int gfs2_writepage(struct page *p
 		return 0; /* don't care */
 	}
 
-	if (sdp->sd_args.ar_data == GFS2_DATA_ORDERED || gfs2_is_jdata(ip)) {
+	if ((sdp->sd_args.ar_data == GFS2_DATA_ORDERED || gfs2_is_jdata(ip)) &&
+	    PageChecked(page)) {
+		ClearPageChecked(page);
 		error = gfs2_trans_begin(sdp, RES_DINODE + 1, 0);
 		if (error)
 			goto out_ignore;
@@ -574,6 +576,23 @@ fail_nounlock:
 }
 
 /**
+ * gfs2_set_page_dirty - Page dirtying function
+ * @page: The page to dirty
+ *
+ * Returns: 1 if it dirtyed the page, or 0 otherwise
+ */
+ 
+static int gfs2_set_page_dirty(struct page *page)
+{
+	struct gfs2_inode *ip = GFS2_I(page->mapping->host);
+	struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host);
+
+	if (sdp->sd_args.ar_data == GFS2_DATA_ORDERED || gfs2_is_jdata(ip))
+		SetPageChecked(page);
+	return __set_page_dirty_buffers(page);
+}
+
+/**
  * gfs2_bmap - Block map function
  * @mapping: Address space info
  * @lblock: The block to map
@@ -609,6 +628,8 @@ static void discard_buffer(struct gfs2_s
 	if (bd) {
 		bd->bd_bh = NULL;
 		bh->b_private = NULL;
+		if (!bd->bd_ail && list_empty(&bd->bd_le.le_list))
+			kmem_cache_free(gfs2_bufdata_cachep, bd);
 	}
 	gfs2_log_unlock(sdp);
 
@@ -629,6 +650,8 @@ static void gfs2_invalidatepage(struct p
 	unsigned int curr_off = 0;
 
 	BUG_ON(!PageLocked(page));
+	if (offset == 0)
+		ClearPageChecked(page);
 	if (!page_has_buffers(page))
 		return;
 
@@ -841,6 +864,7 @@ const struct address_space_operations gf
 	.sync_page = block_sync_page,
 	.prepare_write = gfs2_prepare_write,
 	.commit_write = gfs2_commit_write,
+	.set_page_dirty = gfs2_set_page_dirty,
 	.bmap = gfs2_bmap,
 	.invalidatepage = gfs2_invalidatepage,
 	.releasepage = gfs2_releasepage,


More information about the Cluster-devel mailing list