diff mbox

[1/2] ext4: Fix the delalloc writepages to allocate blocks at the right offset.

Message ID 1226050948-32221-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
State Accepted, archived
Headers show

Commit Message

Aneesh Kumar K.V Nov. 7, 2008, 9:42 a.m. UTC
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 <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/inode.c |   56 ++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 39 insertions(+), 17 deletions(-)

Comments

Theodore Ts'o Nov. 26, 2008, 5:53 a.m. UTC | #1
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 <aneesh.kumar@linux.vnet.ibm.com>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aneesh Kumar K.V Nov. 27, 2008, 5:37 a.m. UTC | #2
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 <aneesh.kumar@linux.vnet.ibm.com>
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);