On Tue, Apr 28, 2009 at 09:50:49AM +0530, Aneesh Kumar K.V wrote:
> On Mon, Apr 27, 2009 at 04:04:54PM -0700, Mingming Cao wrote:
> .....
>
> >
> > Index: linux-2.6.28-rc6/fs/ext4/inode.c
> > ===================================================================
> > --- linux-2.6.28-rc6.orig/fs/ext4/inode.c 2009-03-12 10:21:05.000000000 -0700
> > +++ linux-2.6.28-rc6/fs/ext4/inode.c 2009-04-27 14:35:21.000000000 -0700
> > @@ -2177,7 +2177,10 @@ static int ext4_da_get_block_prep(struct
> > set_buffer_new(bh_result);
> > set_buffer_delay(bh_result);
> > } else if (ret > 0) {
> > + if (buffer_unwritten(bh_result))
> > + set_buffer_new(bh_result);
> > bh_result->b_size = (ret << inode->i_blkbits);
> > + bh_result->b_bdev = inode->i_sb->s_bdev;
>
>
> Updated patch to set bh_result->b_dev. I also added comments in the
> source to explain whey we need to mark buffer_head new. Also updated
> single line patch summary. I will send the update (-v2) patch.
Looking at the source again i guess setting just b_dev is not enough.
unmap_underlying_metadata looks at the mapping block number, which we
don't have in case on unwritten buffer_head. How about the below patch ?
It involve vfs changes. But i guess it is correct with respect to the
meaning of BH_New (Disk mapping was newly created by get_block). I guess
BH_New implies BH_Mapped.
I haven't tested the patch yet. Also it should be split into multiple
patches. It also a fix a problem where we missed an
unamp_underlying_metadata in case of delayed allocated blocks. I guess
that can also cause corruption with delayed allocation.
From: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
Subject: [PATCH -V3] ext4: Fix sub-block zeroing for buffered writes into unwritten extents.
We need to mark the buffer_head mapping prealloc space
as new during write_begin. Otherwise we don't zero out the
page cache content properly for a partial write. This will
cause file corruption with preallocation.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
---
fs/buffer.c | 9 ++++++++-
fs/ext4/inode.c | 8 +++++---
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index b3e5be7..13f0d52 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1867,15 +1867,22 @@ static int __block_prepare_write(struct inode *inode, struct page *page,
err = get_block(inode, block, bh, 1);
if (err)
break;
- if (buffer_new(bh)) {
+ if (buffer_new(bh))
unmap_underlying_metadata(bh->b_bdev,
bh->b_blocknr);
+ if (buffer_new(bh) || buffer_unwritten(bh) ||
+ buffer_delay(bh)) {
if (PageUptodate(page)) {
clear_buffer_new(bh);
set_buffer_uptodate(bh);
mark_buffer_dirty(bh);
continue;
}
+ /*
+ * sub-block writes into unwritten or
+ * delayed buffer should result in zero out
+ * of the rest of the buffer
+ */
if (block_end > to || block_start < from)
zero_user_segments(page,
to, block_end,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e91f978..504afb7 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1892,13 +1892,17 @@ static void mpage_put_bnr_to_bhs(struct mpage_da_data *mpd, sector_t logical,
if (buffer_delay(bh)) {
bh->b_blocknr = pblock;
clear_buffer_delay(bh);
+ set_buffer_mapped(bh);
bh->b_bdev = inode->i_sb->s_bdev;
+ unmap_underlying_metadata(bh->b_bdev,
+ pblock);
} else if (buffer_unwritten(bh)) {
bh->b_blocknr = pblock;
clear_buffer_unwritten(bh);
set_buffer_mapped(bh);
- set_buffer_new(bh);
bh->b_bdev = inode->i_sb->s_bdev;
+ unmap_underlying_metadata(bh->b_bdev,
+ pblock);
} else if (buffer_mapped(bh))
BUG_ON(bh->b_blocknr != pblock);
@@ -2318,8 +2322,6 @@ static int ext4_da_get_block_prep(struct inode *inode, sector_t iblock,
/* not enough space to reserve */
return ret;
- map_bh(bh_result, inode->i_sb, 0);
- set_buffer_new(bh_result);
set_buffer_delay(bh_result);
} else if (ret > 0) {
bh_result->b_size = (ret << inode->i_blkbits);
--
tg: (2084a87..) preallocate_corruption (depends on: ext4_lock_group_conversion)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
