linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] libext2fs: Fix regression in ext2fs_extent_set_bmap()
@ 2009-07-20  3:57 Theodore Ts'o
  2009-07-20  3:57 ` [PATCH 2/3] libext2fs: Improve debugging printf's in extent.c Theodore Ts'o
  2009-07-20  3:57 ` [PATCH 3/3] libext2fs: Avoid creating unneeded new extents in ext2fs_extent_set_bmap() Theodore Ts'o
  0 siblings, 2 replies; 3+ messages in thread
From: Theodore Ts'o @ 2009-07-20  3:57 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Commit 0dc291611 introduced a regression when unmapping the first
block in an extent.  This caused e2fsck -fD to corrupt large
directories if the directory has to shrink by more than one block.
The problem was set_bmap should only go to a next leaf when setting a
first block in an extent, and not when it is unmapping the first block
in an extent.

Addresses-Debian-Bug: #537510

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/extent.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index 7fcc2cf..9d7b7de 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -1296,11 +1296,12 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 			}
 			if (retval)
 				goto done;
+			retval = ext2fs_extent_get(handle,
+						   EXT2_EXTENT_NEXT_LEAF,
+						   &extent);
+			if (retval)
+				goto done;
 		}
-		retval = ext2fs_extent_get(handle, EXT2_EXTENT_NEXT_LEAF,
-					   &extent);
-		if (retval)
-			goto done;
 		extent.e_pblk++;
 		extent.e_lblk++;
 		extent.e_len--;
-- 
1.6.3.2.1.gb9f7d.dirty


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/3] libext2fs: Improve debugging printf's in extent.c
  2009-07-20  3:57 [PATCH 1/3] libext2fs: Fix regression in ext2fs_extent_set_bmap() Theodore Ts'o
@ 2009-07-20  3:57 ` Theodore Ts'o
  2009-07-20  3:57 ` [PATCH 3/3] libext2fs: Avoid creating unneeded new extents in ext2fs_extent_set_bmap() Theodore Ts'o
  1 sibling, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2009-07-20  3:57 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Comment out less common debugging printf's, and fix some type
warnings.  Add high-level debugging printf's for ext2fs_extent_goto(),
ext2fs_extent_insert(), ext2fs_extent_delete(), ext2fs_extent_replace()

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/extent.c |   59 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index 9d7b7de..c47fbcc 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -104,7 +104,7 @@ static void dbg_print_extent(char *desc, struct ext2fs_extent *extent)
 {
 	if (desc)
 		printf("%s: ", desc);
-	printf("extent: lblk %llu--%llu, len %lu, pblk %llu, flags: ",
+	printf("extent: lblk %llu--%llu, len %u, pblk %llu, flags: ",
 	       extent->e_lblk, extent->e_lblk + extent->e_len - 1,
 	       extent->e_len, extent->e_pblk);
 	if (extent->e_flags & EXT2_EXTENT_FLAGS_LEAF)
@@ -321,7 +321,7 @@ retry:
 				return EXT2_ET_EXTENT_NO_NEXT;
 		}
 		if (op != EXT2_EXTENT_NEXT_SIB) {
-#ifdef DEBUG
+#ifdef DEBUG_GET_EXTENT
 			printf("<<<< OP = %s\n",
 			       (op == EXT2_EXTENT_DOWN) ? "down" :
 			       ((op == EXT2_EXTENT_UP) ? "up" : "unknown"));
@@ -352,7 +352,7 @@ retry:
 				return EXT2_ET_EXTENT_NO_PREV;
 		}
 		if (op != EXT2_EXTENT_PREV_SIB) {
-#ifdef DEBUG
+#ifdef DEBUG_GET_EXTENT
 			printf("<<<< OP = %s\n",
 			       (op == EXT2_EXTENT_DOWN_AND_LAST) ? "down/last" :
 			       ((op == EXT2_EXTENT_UP) ? "up" : "unknown"));
@@ -366,7 +366,7 @@ retry:
 			op = EXT2_EXTENT_DOWN;
 		else
 			op = EXT2_EXTENT_LAST_SIB;
-#ifdef DEBUG
+#ifdef DEBUG_GET_EXTENT
 		printf("<<<< OP = %s\n",
 			   (op == EXT2_EXTENT_DOWN) ? "down" : "last_sib");
 #endif
@@ -481,7 +481,7 @@ retry:
 			if (handle->level < handle->max_depth)
 				path->visit_num = 1;
 		}
-#ifdef DEBUG
+#ifdef DEBUG_GET_EXTENT
 		printf("Down to level %d/%d, end_blk=%llu\n",
 			   handle->level, handle->max_depth,
 			   path->end_blk);
@@ -495,7 +495,7 @@ retry:
 		return EXT2_ET_NO_CURRENT_NODE;
 
 	extent->e_flags = 0;
-#ifdef DEBUG
+#ifdef DEBUG_GET_EXTENT
 	printf("(Left %d)\n", path->left);
 #endif
 
@@ -628,7 +628,14 @@ static errcode_t extent_goto(ext2_extent_handle_t handle,
 		return EXT2_ET_OP_NOT_SUPPORTED;
 	}
 
+#ifdef DEBUG
+	printf("goto extent ino %u, level %d, %llu\n", handle->ino,
+	       leaf_level, blk);
+#endif
+
+#ifdef DEBUG_GOTO_EXTENTS
 	dbg_print_extent("root", &extent);
+#endif
 	while (1) {
 		if (handle->max_depth - handle->level == leaf_level) {
 			/* block is in this &extent */
@@ -658,7 +665,9 @@ static errcode_t extent_goto(ext2_extent_handle_t handle,
 		if (retval)
 			return retval;
 
+#ifdef DEBUG_GOTO_EXTENTS
 		dbg_print_extent("next", &extent);
+#endif
 		if (blk == extent.e_lblk)
 			goto go_down;
 		if (blk > extent.e_lblk)
@@ -669,7 +678,9 @@ static errcode_t extent_goto(ext2_extent_handle_t handle,
 		if (retval)
 			return retval;
 
+#ifdef DEBUG_GOTO_EXTENTS
 		dbg_print_extent("prev", &extent);
+#endif
 
 	go_down:
 		retval = ext2fs_extent_get(handle, EXT2_EXTENT_DOWN,
@@ -677,7 +688,9 @@ static errcode_t extent_goto(ext2_extent_handle_t handle,
 		if (retval)
 			return retval;
 
+#ifdef DEBUG_GOTO_EXTENTS
 		dbg_print_extent("down", &extent);
+#endif
 	}
 }
 
@@ -765,6 +778,11 @@ errcode_t ext2fs_extent_replace(ext2_extent_handle_t handle,
 	if (!path->curr)
 		return EXT2_ET_NO_CURRENT_NODE;
 
+#ifdef DEBUG
+	printf("extent replace: %u ", handle->ino);
+	dbg_print_extent(0, extent);
+#endif
+
 	if (handle->level == handle->max_depth) {
 		ex = path->curr;
 
@@ -922,7 +940,8 @@ static errcode_t extent_node_split(ext2_extent_handle_t handle)
 		goto done;
 
 #ifdef DEBUG
-	printf("will copy to new node at block %lu\n", new_node_pblk);
+	printf("will copy to new node at block %lu\n",
+	       (unsigned long) new_node_pblk);
 #endif
 
 	/* Copy data into new block buffer */
@@ -1040,6 +1059,11 @@ errcode_t ext2fs_extent_insert(ext2_extent_handle_t handle, int flags,
 	if (!handle->path)
 		return EXT2_ET_NO_CURRENT_NODE;
 
+#ifdef DEBUG
+	printf("extent insert: %u ", handle->ino);
+	dbg_print_extent(0, extent);
+#endif
+
 	path = handle->path + handle->level;
 
 	if (path->entries >= path->max_entries) {
@@ -1132,6 +1156,11 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 
 	EXT2_CHECK_MAGIC(handle, EXT2_ET_MAGIC_EXTENT_HANDLE);
 
+#ifdef DEBUG
+	printf("set_bmap ino %u log %lld phys %lld flags %d\n",
+	       handle->ino, logical, physical, flags);
+#endif
+
 	if (!(handle->fs->flags & EXT2_FLAG_RW))
 		return EXT2_ET_RO_FILSYS;
 
@@ -1364,6 +1393,19 @@ errcode_t ext2fs_extent_delete(ext2_extent_handle_t handle, int flags)
 	if (!handle->path)
 		return EXT2_ET_NO_CURRENT_NODE;
 
+#ifdef DEBUG
+	{
+		struct ext2fs_extent	extent;
+
+		retval = ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT,
+					   &extent);
+		if (retval == 0) {
+			printf("extent delete %u ", handle->ino);
+			dbg_print_extent(0, &extent);
+		}
+	}
+#endif
+
 	path = handle->path + handle->level;
 	if (!path->curr)
 		return EXT2_ET_NO_CURRENT_NODE;
@@ -1886,7 +1928,8 @@ void do_goto_block(int argc, char **argv)
 	retval = extent_goto(current_handle, level, (blk64_t) blk);
 
 	if (retval) {
-		com_err(argv[0], retval, "while trying to go to block %lu, level %d",
+		com_err(argv[0], retval,
+			"while trying to go to block %u, level %d",
 			blk, level);
 		return;
 	}
-- 
1.6.3.2.1.gb9f7d.dirty


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 3/3] libext2fs: Avoid creating unneeded new extents in ext2fs_extent_set_bmap()
  2009-07-20  3:57 [PATCH 1/3] libext2fs: Fix regression in ext2fs_extent_set_bmap() Theodore Ts'o
  2009-07-20  3:57 ` [PATCH 2/3] libext2fs: Improve debugging printf's in extent.c Theodore Ts'o
@ 2009-07-20  3:57 ` Theodore Ts'o
  1 sibling, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2009-07-20  3:57 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Avoiding inserting a new extent if it is possible to merge the new
block to the beginning or the end of the previous or next extent.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 lib/ext2fs/extent.c |  118 ++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 93 insertions(+), 25 deletions(-)

diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index c47fbcc..1505447 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -1146,11 +1146,14 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 	int			mapped = 1; /* logical is mapped? */
 	int			orig_height;
 	int			extent_uninit = 0;
+	int			prev_uninit = 0;
+	int			next_uninit = 0;
 	int			new_uninit = 0;
 	int			max_len = EXT_INIT_MAX_LEN;
+	int			has_prev, has_next;
 	blk64_t			orig_lblk;
 	struct extent_path	*path;
-	struct ext2fs_extent	extent;
+	struct ext2fs_extent	extent, next_extent, prev_extent;
 	struct ext2fs_extent	newextent;
 	struct ext2_extent_info	info;
 
@@ -1222,12 +1225,44 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 	/*
 	 * This may be the extent *before* the requested logical,
 	 * if it's currently unmapped.
+	 *
+	 * Get the previous and next leaf extents, if they are present.
 	 */
 	retval = ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent);
 	if (retval)
 		goto done;
 	if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
 		extent_uninit = 1;
+	retval = ext2fs_extent_get(handle, EXT2_EXTENT_NEXT_LEAF, &next_extent);
+	if (retval) {
+		has_next = 0;
+		if (retval != EXT2_ET_EXTENT_NO_NEXT)
+			goto done;
+	} else {
+		dbg_print_extent("set_bmap: next_extent",
+				 &next_extent);
+		has_next = 1;
+		if (next_extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
+			next_uninit = 1;
+	}
+	retval = ext2fs_extent_goto(handle, logical);
+	if (retval && retval != EXT2_ET_EXTENT_NOT_FOUND)
+		goto done;
+	retval = ext2fs_extent_get(handle, EXT2_EXTENT_PREV_LEAF, &prev_extent);
+	if (retval) {
+		has_prev = 0;
+		if (retval != EXT2_ET_EXTENT_NO_PREV)
+			goto done;
+	} else {
+		has_prev = 1;
+		dbg_print_extent("set_bmap: prev_extent",
+				 &prev_extent);
+		if (prev_extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
+			prev_uninit = 1;
+	}
+	retval = ext2fs_extent_goto(handle, logical);
+	if (retval && retval != EXT2_ET_EXTENT_NOT_FOUND)
+		goto done;
 
 	/* check if already pointing to the requested physical */
 	if (mapped && (new_uninit == extent_uninit) &&
@@ -1248,6 +1283,28 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 		    ((int) extent.e_len < max_len-1)) {
 			extent.e_len++;
 			retval = ext2fs_extent_replace(handle, 0, &extent);
+		} else if ((logical == extent.e_lblk - 1) &&
+			   (physical == extent.e_pblk - 1) &&
+			   (new_uninit == extent_uninit) &&
+			   ((int) extent.e_len < max_len - 1)) {
+			extent.e_len++;
+			extent.e_lblk--;
+			extent.e_pblk--;
+			retval = ext2fs_extent_replace(handle, 0, &extent);
+		} else if (has_next &&
+			   (logical == next_extent.e_lblk - 1) &&
+			   (physical == next_extent.e_pblk - 1) &&
+			   (new_uninit == next_uninit) &&
+			   ((int) next_extent.e_len < max_len - 1)) {
+			retval = ext2fs_extent_get(handle,
+						   EXT2_EXTENT_NEXT_LEAF,
+						   &next_extent);
+			if (retval)
+				goto done;
+			next_extent.e_len++;
+			next_extent.e_lblk--;
+			next_extent.e_pblk--;
+			retval = ext2fs_extent_replace(handle, 0, &next_extent);
 		} else if (logical < extent.e_lblk)
 			retval = ext2fs_extent_insert(handle, 0, &newextent);
 		else
@@ -1279,14 +1336,32 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 #ifdef DEBUG
 		printf("(re/un)mapping last block in extent\n");
 #endif
-		/* Make sure insert works before replacing old extent */
 		if (physical) {
-			retval = ext2fs_extent_insert(handle,
-					EXT2_EXTENT_INSERT_AFTER, &newextent);
+			if (has_next &&
+			    (logical == (next_extent.e_lblk - 1)) &&
+			    (physical == (next_extent.e_pblk - 1)) &&
+			    (new_uninit == next_uninit) &&
+			    ((int) next_extent.e_len < max_len - 1)) {
+				retval = ext2fs_extent_get(handle,
+					EXT2_EXTENT_NEXT_LEAF, &next_extent);
+				if (retval)
+					goto done;
+				next_extent.e_len++;
+				next_extent.e_lblk--;
+				next_extent.e_pblk--;
+				retval = ext2fs_extent_replace(handle, 0,
+							       &next_extent);
+				if (retval)
+					goto done;
+			} else
+				retval = ext2fs_extent_insert(handle,
+				      EXT2_EXTENT_INSERT_AFTER, &newextent);
 			if (retval)
 				goto done;
 			/* Now pointing at inserted extent; move back to prev */
-			retval = ext2fs_extent_goto(handle, logical - 1);
+			retval = ext2fs_extent_get(handle,
+						   EXT2_EXTENT_PREV_LEAF,
+						   &extent);
 			if (retval)
 				goto done;
 		}
@@ -1299,30 +1374,23 @@ errcode_t ext2fs_extent_set_bmap(ext2_extent_handle_t handle,
 		printf("(re/un)mapping first block in extent\n");
 #endif
 		if (physical) {
-			retval = ext2fs_extent_get(handle, 
-						   EXT2_EXTENT_PREV_LEAF,
-						   &extent);
-			if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
-				extent_uninit = 1;
-			if (retval == EXT2_ET_EXTENT_NO_PREV) {
-				retval = ext2fs_extent_goto(handle, logical);
+			if (has_prev &&
+			    (logical == (prev_extent.e_lblk +
+					 prev_extent.e_len)) &&
+			    (physical == (prev_extent.e_pblk +
+					  prev_extent.e_len)) &&
+			    (new_uninit == prev_uninit) &&
+			    ((int) prev_extent.e_len < max_len-1)) {
+				retval = ext2fs_extent_get(handle, 
+					EXT2_EXTENT_PREV_LEAF, &prev_extent);
 				if (retval)
 					goto done;
-				retval = ext2fs_extent_insert(handle,
-							      0, &newextent);
-			} else if (retval)
-				goto done;
-			else if ((logical == extent.e_lblk + extent.e_len) &&
-				 (physical == extent.e_pblk + extent.e_len) &&
-				 (new_uninit == extent_uninit) &&
-				 ((int) extent.e_len < max_len-1)) {
-				extent.e_len++;
+				prev_extent.e_len++;
 				retval = ext2fs_extent_replace(handle, 0,
-							       &extent);
-			} else {
+							       &prev_extent);
+			} else
 				retval = ext2fs_extent_insert(handle,
-				      EXT2_EXTENT_INSERT_AFTER, &newextent);
-			}
+							      0, &newextent);
 			if (retval)
 				goto done;
 			retval = ext2fs_extent_get(handle,
-- 
1.6.3.2.1.gb9f7d.dirty


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-07-20  3:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-20  3:57 [PATCH 1/3] libext2fs: Fix regression in ext2fs_extent_set_bmap() Theodore Ts'o
2009-07-20  3:57 ` [PATCH 2/3] libext2fs: Improve debugging printf's in extent.c Theodore Ts'o
2009-07-20  3:57 ` [PATCH 3/3] libext2fs: Avoid creating unneeded new extents in ext2fs_extent_set_bmap() Theodore Ts'o

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