When iterating through the pages with all mapped buffer_heads
we failed to update the b_state value. This result in allocating
blocks at logical offset 0.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/inode.c | 56 ++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d42f748..95d0d12 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1644,35 +1644,39 @@ static void ext4_da_page_release_reservation(struct page *page,
*/
static int mpage_da_submit_io(struct mpage_da_data *mpd)
{
- struct address_space *mapping = mpd->inode->i_mapping;
- int ret = 0, err, nr_pages, i;
- unsigned long index, end;
- struct pagevec pvec;
long pages_skipped;
+ struct pagevec pvec;
+ unsigned long index, end;
+ int ret = 0, err, nr_pages, i;
+ struct inode *inode = mpd->inode;
+ struct address_space *mapping = inode->i_mapping;
BUG_ON(mpd->next_page <= mpd->first_page);
- pagevec_init(&pvec, 0);
+ /*
+ * we need to start from the first_page to the next_page - 1
+ * That is to make sure we also write the mapped dirty
+ * buffer_heads. If we look at mpd->lbh.b_blocknr we
+ * would only be looking at currently mapped buffer_heads.
+ */
index = mpd->first_page;
end = mpd->next_page - 1;
+ pagevec_init(&pvec, 0);
while (index <= end) {
- /*
- * We can use PAGECACHE_TAG_DIRTY lookup here because
- * even though we have cleared the dirty flag on the page
- * We still keep the page in the radix tree with tag
- * PAGECACHE_TAG_DIRTY. See clear_page_dirty_for_io.
- * The PAGECACHE_TAG_DIRTY is cleared in set_page_writeback
- * which is called via the below writepage callback.
- */
- nr_pages = pagevec_lookup_tag(&pvec, mapping, &index,
- PAGECACHE_TAG_DIRTY,
- min(end - index,
- (pgoff_t)PAGEVEC_SIZE-1) + 1);
+ nr_pages = pagevec_lookup(&pvec, mapping, index, PAGEVEC_SIZE);
if (nr_pages == 0)
break;
for (i = 0; i < nr_pages; i++) {
struct page *page = pvec.pages[i];
+ index = page->index;
+ if (index > end)
+ break;
+ index++;
+
+ BUG_ON(!PageLocked(page));
+ BUG_ON(PageWriteback(page));
+
pages_skipped = mpd->wbc->pages_skipped;
err = mapping->a_ops->writepage(page, mpd->wbc);
if (!err && (pages_skipped == mpd->wbc->pages_skipped))
@@ -2086,11 +2090,29 @@ static int __mpage_da_writepage(struct page *page,
bh = head;
do {
BUG_ON(buffer_locked(bh));
+ /*
+ * We need to try to allocte
+ * unmapped blocks in the same page.
+ * Otherwise we won't make progress
+ * with the page in ext4_da_writepage
+ */
if (buffer_dirty(bh) &&
(!buffer_mapped(bh) || buffer_delay(bh))) {
mpage_add_bh_to_extent(mpd, logical, bh);
if (mpd->io_done)
return MPAGE_DA_EXTENT_TAIL;
+ } else if (buffer_dirty(bh) && (buffer_mapped(bh))) {
+ /*
+ * mapped dirty buffer. We need to update
+ * the b_state because we look at
+ * b_state in mpage_da_map_blocks. We don't
+ * update b_size because if we find an
+ * unmapped buffer_head later we need to
+ * use the b_state flag of that buffer_head.
+ */
+ if (mpd->lbh.b_size == 0)
+ mpd->lbh.b_state =
+ bh->b_state & BH_FLAGS;
}
logical++;
} while ((bh = bh->b_this_page) != head);
--
1.6.0.3.640.g6331a
We need to make sure we mark the buffer_heads as dirty and uptodate
so that block_write_full_page write them correctly.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/inode.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 95d0d12..d986018 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2351,6 +2351,8 @@ static int ext4_da_writepage(struct page *page,
unlock_page(page);
return 0;
}
+ /* now mark the buffer_heads as dirty and uptodate */
+ block_commit_write(page, 0, PAGE_CACHE_SIZE);
}
if (test_opt(inode->i_sb, NOBH) && ext4_should_writeback_data(inode))
--
1.6.0.3.640.g6331a
On Fri, Nov 07, 2008 at 03:12:28PM +0530, Aneesh Kumar K.V wrote:
> We need to make sure we mark the buffer_heads as dirty and uptodate
> so that block_write_full_page write them correctly.
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
> fs/ext4/inode.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 95d0d12..d986018 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2351,6 +2351,8 @@ static int ext4_da_writepage(struct page *page,
> unlock_page(page);
> return 0;
> }
> + /* now mark the buffer_heads as dirty and uptodate */
> + block_commit_write(page, 0, PAGE_CACHE_SIZE);
> }
>
> if (test_opt(inode->i_sb, NOBH) && ext4_should_writeback_data(inode))
With this patch i am able to run the mmap program from Linus without
errors. Can you test the changes with rtorrent and see if the change
fixes the file corruption ?
-aneesh
On Fri, Nov 07, 2008 at 03:12:27PM +0530, Aneesh Kumar K.V wrote:
> When iterating through the pages with all mapped buffer_heads
> we failed to update the b_state value. This result in allocating
> blocks at logical offset 0.
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
Hey Aneesh,
I've been going through patches in the patch queue and I noticed that
the subsequent patch
[PATCH 2/2] ext4: Mark the buffer_heads as dirty and uptodate after prepare_write
has been merged to mainline, but this one wasn't. From looking at the
e-mail record, you said that the second one fixed the rtorrent
corruption --- but looking at this patch and its description, it looks
like this one should perhaps get pushed to Linus ASAP as well. Do you
remember if both patches were needed to fix the rtorrent corruption
problem, or only the second one?
Thanks,
- Ted
On Wed, Nov 26, 2008 at 12:53:21AM -0500, Theodore Tso wrote:
> On Fri, Nov 07, 2008 at 03:12:27PM +0530, Aneesh Kumar K.V wrote:
> > When iterating through the pages with all mapped buffer_heads
> > we failed to update the b_state value. This result in allocating
> > blocks at logical offset 0.
> >
> > Signed-off-by: Aneesh Kumar K.V <[email protected]>
>
> Hey Aneesh,
>
> I've been going through patches in the patch queue and I noticed that
> the subsequent patch
>
> [PATCH 2/2] ext4: Mark the buffer_heads as dirty and uptodate after prepare_write
>
> has been merged to mainline, but this one wasn't. From looking at the
> e-mail record, you said that the second one fixed the rtorrent
> corruption --- but looking at this patch and its description, it looks
> like this one should perhaps get pushed to Linus ASAP as well. Do you
> remember if both patches were needed to fix the rtorrent corruption
> problem, or only the second one?
Both patches are not needed to fix the rtorrent problem. The second
patch actually fix the rtorrent issue.
-aneesh