All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Marzinski <bmarzins@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH] gfs2: rest of the fix for 238162
Date: Wed, 6 Jun 2007 02:20:50 -0500	[thread overview]
Message-ID: <20070606072050.GD8814@ether.msp.redhat.com> (raw)

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@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,

                 reply	other threads:[~2007-06-06  7:20 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070606072050.GD8814@ether.msp.redhat.com \
    --to=bmarzins@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.