From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [PATCH 1/2] ext3: Fix buffer dirtying in data=journal mode (fwd) Date: Fri, 6 Aug 2010 00:19:12 +1000 Message-ID: <20100805141912.GA17264@amd> References: <20100721170854.GB1215@atrey.karlin.mff.cuni.cz> <20100802145701.6952d607.akpm@linux-foundation.org> <20100802230647.GM3278@quack.suse.cz> <20100802161928.68e30ddc.akpm@linux-foundation.org> <20100803135748.GH3322@quack.suse.cz> <20100803120644.5e7247b3.akpm@linux-foundation.org> <20100803223417.GL3322@quack.suse.cz> <20100803155842.ce0fa9eb.akpm@linux-foundation.org> <20100804153521.GB7037@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andrew Morton , npiggin@suse.de, linux-fsdevel@vger.kernel.org To: Jan Kara Return-path: Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:61501 "EHLO ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760558Ab0HEOTR (ORCPT ); Thu, 5 Aug 2010 10:19:17 -0400 Content-Disposition: inline In-Reply-To: <20100804153521.GB7037@quack.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > 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 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 > --- > 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) > --