linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Ext4 Developers List <linux-ext4@vger.kernel.org>
Cc: Theodore Ts'o <tytso@mit.edu>
Subject: [PATCH 2/2] ext4: Clear the EXT4_EOFBLOCKS_FL flag only when warranted
Date: Tue, 25 May 2010 00:18:37 -0400	[thread overview]
Message-ID: <1274761117-26560-2-git-send-email-tytso@mit.edu> (raw)
In-Reply-To: <20100525041737.GB5556@thunk.org>

Dimitry Monakhov discovered an edge case where it was possible for the
EXT4_EOFBLOCKS_FL flag could get cleared unnecessarily.  This is true;
I have a test case that can be exercised via downloading and
decompressing the file:

wget ftp://ftp.kernel.org/pub/linux/kernel/people/tytso/ext4-testcases/eofblocks-fl-test-case.img.bz2
bunzip2 eofblocks-fl-test-case.img
dd if=/dev/zero of=eofblocks-fl-test-case.img bs=1k seek=17925 bs=1k count=1 conv=notrunc

However, triggering it in real life is highly unlikely since it
requires an extremely fragmented sparse file with a hole in exactly
the right place in the extent tree.  (It actually took quite a bit of
work to generate this test case.)  Still, it's nice to get even
extreme corner cases to be correct, so this patch makes sure that we
don't clear the EXT4_EOFBLOCKS_FL incorrectly even in this corner
case.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/extents.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index aeec5c7..c7c304f 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3319,7 +3319,7 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 	struct ext4_extent_header *eh;
 	struct ext4_extent newex, *ex, *last_ex;
 	ext4_fsblk_t newblock;
-	int err = 0, depth, ret, cache_type;
+	int i, err = 0, depth, ret, cache_type;
 	unsigned int allocated = 0;
 	struct ext4_allocation_request ar;
 	ext4_io_end_t *io = EXT4_I(inode)->cur_aio_dio;
@@ -3508,8 +3508,20 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
 			goto out2;
 		}
 		last_ex = EXT_LAST_EXTENT(eh);
-		if (map->m_lblk + ar.len > le32_to_cpu(last_ex->ee_block)
-		    + ext4_ext_get_actual_len(last_ex))
+		/*
+		 * If the current leaf block was reached by looking at
+		 * the last index block all the way down the tree, and
+		 * we are extending the inode beyond the last extent
+		 * in the current leaf block, then clear the
+		 * EOFBLOCKS_FL flag.
+		 */
+		for (i=depth-1; i >= 0; i--) {
+			if (path[i].p_idx != EXT_LAST_INDEX(path[i].p_hdr))
+				break;
+		}
+		if ((i < 0) &&
+		    (map->m_lblk + ar.len > le32_to_cpu(last_ex->ee_block) + 
+		     ext4_ext_get_actual_len(last_ex)))
 			ext4_clear_inode_flag(inode, EXT4_INODE_EOFBLOCKS);
 	}
 	err = ext4_ext_insert_extent(handle, inode, path, &newex, flags);
-- 
1.6.6.1.1.g974db.dirty


  parent reply	other threads:[~2010-05-25  4:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-19 14:32 [PATCH 1/2] ext4: Use bitops to read/modify EXT4_I(inode)->i_flags Dmitry Monakhov
2010-04-19 14:32 ` [PATCH 2/2] ext4: fix eofblock flag handling Dmitry Monakhov
2010-05-25  4:17   ` tytso
2010-05-25  4:18     ` [PATCH 1/2] ext4: Avoid crashing on NULL ptr dereference on a filesystem error Theodore Ts'o
2010-05-25  4:18     ` Theodore Ts'o [this message]
2010-05-25  7:23       ` [PATCH 2/2] ext4: Clear the EXT4_EOFBLOCKS_FL flag only when warranted Dmitry Monakhov
2010-05-25 13:03         ` tytso
2010-05-25 13:12           ` Dmitry Monakhov
2010-05-25 13:15             ` tytso
2010-05-25 13:19               ` Dmitry Monakhov
2010-05-24  3:09 ` [PATCH 1/2] ext4: Use bitops to read/modify EXT4_I(inode)->i_flags tytso
2010-05-24 20:49 ` [PATCH -v2] ext4: Use bitops to read/modify i_flags in struct ext4_inode_info Theodore Ts'o
2010-05-31  8:56   ` [PATCH] ext4: Use bitops to read/modify i_flags part2 Dmitry Monakhov
2010-06-01  3:06     ` tytso
2010-06-03  2:55     ` tytso
2010-06-03  8:48       ` Dmitry Monakhov
2010-06-03 10:37         ` Theodore Tso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1274761117-26560-2-git-send-email-tytso@mit.edu \
    --to=tytso@mit.edu \
    --cc=linux-ext4@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).