Re: [PATCH] mm: Make snapshotting pages for stable writes a per-bio operation

[Jan Kara <jack@suse.cz>]

On Fri 15-03-13 16:28:16, Darrick J. Wong wrote:
> Walking a bio's page mappings has proved problematic, so create a new bio flag
> to indicate that a bio's data needs to be snapshotted in order to guarantee
> stable pages during writeback.  Next, for the one user (ext3/jbd) of
> snapshotting, hook all the places where writes can be initiated without
> PG_writeback set, and set BIO_SNAP_STABLE there.  We must also flag journal
> "metadata" bios for stable writeout if data=journal, since file data is written
> through the journal.  Finally, the MS_SNAP_STABLE mount flag (only used by
> ext3) is now superfluous, so get rid of it.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> [darrick.wong@oracle.com: Fold in a couple of small cleanups from akpm]
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>  fs/buffer.c                 |    9 ++++++++-
>  fs/ext3/super.c             |    3 ++-
>  fs/jbd/commit.c             |   28 +++++++++++++++++++++++++---
>  include/linux/blk_types.h   |    3 ++-
>  include/linux/buffer_head.h |    1 +
>  include/linux/jbd.h         |    1 +
>  include/uapi/linux/fs.h     |    1 -
>  mm/bounce.c                 |   21 +--------------------
>  mm/page-writeback.c         |    4 ----
>  9 files changed, 40 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index b4dcb34..71578d6 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2949,7 +2949,7 @@ static void guard_bh_eod(int rw, struct bio *bio, struct buffer_head *bh)
>  	}
>  }
>  
> -int submit_bh(int rw, struct buffer_head * bh)
> +int _submit_bh(int rw, struct buffer_head *bh, unsigned long bio_flags)
>  {
>  	struct bio *bio;
>  	int ret = 0;
> @@ -2984,6 +2984,7 @@ int submit_bh(int rw, struct buffer_head * bh)
>  
>  	bio->bi_end_io = end_bio_bh_io_sync;
>  	bio->bi_private = bh;
> +	bio->bi_flags |= bio_flags;
>  
>  	/* Take care of bh's that straddle the end of the device */
>  	guard_bh_eod(rw, bio, bh);
> @@ -2997,6 +2998,12 @@ int submit_bh(int rw, struct buffer_head * bh)
>  	bio_put(bio);
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(_submit_bh);
> +
> +int submit_bh(int rw, struct buffer_head *bh)
> +{
> +	return _submit_bh(rw, bh, 0);
> +}
>  EXPORT_SYMBOL(submit_bh);
>  
>  /**
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 1d6e2ed..e845b6de 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -2063,11 +2063,12 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
>  		ext3_mark_recovery_complete(sb, es);
>  		ext3_msg(sb, KERN_INFO, "recovery complete");
>  	}
> +	if (test_opt(sb, DATA_FLAGS) == EXT3_MOUNT_JOURNAL_DATA)
> +		EXT3_SB(sb)->s_journal->j_flags |= JFS_JOURNALS_DATA;
  Sadly this isn't enough. You can have inodes which journal data (there's
an inode flag for this) in data=ordered mode. So what you have to do is to
flag journal_heads (or buffer_heads) as containing journalled data. Or you
can actually use PageChecked flag for this (it is going to be set on all
write-enabled pages with journalled data). But it definitely requires also
some playing with ->page_mkwrite() (calling ext3_journal_get_write_access()
from there) and generally I'd rather postpone that to a separate commit. So
just keep it simple and always set the bio flag as you did in the previous
version. I'll write an optimization (mostly because ext4 needs it as well)
and send it to you for testing.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>