From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin Marzinski Date: Wed, 6 Jun 2007 02:20:50 -0500 Subject: [Cluster-devel] [PATCH] gfs2: rest of the fix for 238162 Message-ID: <20070606072050.GD8814@ether.msp.redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 -------------- 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,