[Cluster-devel] Re: [PATCH] GFS2: bz 283162 - GFS2: Journaled file write/unstuff bug
Steven Whitehouse
swhiteho at redhat.com
Tue Jun 12 16:30:39 UTC 2007
Hi,
Now in the GFS2 -nmw git tree. Thanks,
Steve.
On Tue, 2007-06-12 at 11:24 -0500, Robert Peterson wrote:
> This patch is for bugzilla bug 283162, which uncovered a number of
> bugs pertaining to writing to files that have the journaled bit on.
> These bugs happen most often when writing to the meta_fs because
> the files are always journaled. So operations like gfs2_grow were
> particularly vulnerable, although many of the problems could be
> recreated with normal files after setting the journaled bit on.
> The problems fixed are:
>
> -GFS2 wasn't ever writing unstuffed journaled data blocks to their
> in-place location on disk. Now it does.
>
> -If you unmounted too quickly after doing IO to a journaled file,
> GFS2 was crashing because you would discard a buffer whose bufdata
> was still on the active items list. GFS2 now deals with this
> gracefully.
>
> -GFS2 was losing track of the bufdata for journaled data blocks,
> and it wasn't getting freed, causing an error when you tried to
> unmount the module. GFS2 now frees all the bufdata structures.
>
> -There was a memory corruption occurring because GFS2 wrote
> twice as many log entries for journaled buffers.
>
> -It was occasionally trying to write journal headers in buffers
> that weren't currently mapped.
>
> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> --
>
> fs/gfs2/log.c | 15 ++++++++++++++-
> fs/gfs2/lops.c | 4 +++-
> fs/gfs2/ops_address.c | 26 +++++++++++++++++++++++++-
> 3 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 1fb846f..fbdc0dc 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -83,6 +83,11 @@ static void gfs2_ail1_start_one(struct gfs2_sbd *sdp, struct gfs2_ail *ai)
>
> 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 gfs2_sbd *sdp, struct gfs2_ail *ai, int fl
> 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 gfs2_sbd *sdp, struct gfs2_ail *ai)
> 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 --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 3e971f2..2730071 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -607,7 +607,8 @@ static void databuf_lo_before_commit(struct gfs2_sbd *sdp)
> if (unlikely(magic != 0))
> set_buffer_escaped(bh1);
> gfs2_log_lock(sdp);
> - if (n++ > num)
> + n += 2;
> + if (n >= num)
> break;
> } else if (!bh1) {
> total_dbuf--;
> @@ -624,6 +625,7 @@ static void databuf_lo_before_commit(struct gfs2_sbd *sdp)
> }
> gfs2_log_unlock(sdp);
> if (bh) {
> + set_buffer_mapped(bh);
> set_buffer_dirty(bh);
> ll_rw_block(WRITE, 1, &bh);
> bh = NULL;
> diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
> index ac56595..9ab35a9 100644
> --- a/fs/gfs2/ops_address.c
> +++ b/fs/gfs2/ops_address.c
> @@ -137,7 +137,9 @@ static int gfs2_writepage(struct page *page, struct writeback_control *wbc)
> 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_sbd *sdp, struct buffer_head *bh)
> 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 page *page, unsigned long offset)
> 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 gfs2_file_aops = {
> .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