Re: [PATCH 1/2] ext3: Fix buffer dirtying in data=journal mode (fwd)

[Nick Piggin <npiggin@suse.de>]

On Wed, Aug 04, 2010 at 05:35:22PM +0200, Jan Kara wrote:
> On Tue 03-08-10 15:58:42, Andrew Morton wrote:
> >From 40310ae771ba1d625c7917891a5d4f84f5b458ac Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@suse.cz>
> Date: Wed, 4 Aug 2010 17:10:56 +0200
> Subject: [PATCH] fs: Remove obsolete buffer handling
> 
> When we freshly allocate a new block under an uptodate page, we have
> to make sure it doesn't get zeroed and gets marked dirty. Commit
> 637aff46 added a code to handle this case properly. Later, commit
> afddba49 added page_zero_new_buffers() function which takes care of
> this case properly as well and all filesystems use it either directly
> or indirectly. So let's remove the obsolete code so that we handle
> this special case only in one place.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

I don't see a problem, but it is a special case. Because the data
which is causing the buffer to be dirtied is already in the pagecache
-- not the data we're about to write in there.

But with the comment it is clear enough.

Acked-by: Nick Piggin <npiggin@suse.de>


> ---
>  fs/buffer.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index d54812b..a2947c2 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -1876,9 +1876,17 @@ static int __block_prepare_write(struct inode *inode, struct page *page,
>  				unmap_underlying_metadata(bh->b_bdev,
>  							bh->b_blocknr);
>  				if (PageUptodate(page)) {
> -					clear_buffer_new(bh);
> +					/*
> +					 * New buffer underlying already
> +					 * uptodate data. This can happen when
> +					 * data was already written via mmap.
> +					 * We must not zero-out already valid
> +					 * data and must mark buffer dirty.
> +					 * page_zero_new_buffers() takes care
> +					 * of this (or block_commit_write() if
> +					 * data gets overwritten).
> +					 */
>  					set_buffer_uptodate(bh);
> -					mark_buffer_dirty(bh);
>  					continue;
>  				}
>  				if (block_end > to || block_start < from)
> --