linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Amit K. Arora" <aarora@linux.vnet.ibm.com>
To: linux-ext4@vger.kernel.org
Cc: suparna@in.ibm.com, cmm@us.ibm.com, alex@clusterfs.com
Subject: [PATCH 1/1] Extent overlap bugfix in ext4
Date: Tue, 2 Jan 2007 14:39:09 +0530	[thread overview]
Message-ID: <20070102090909.GA20503@amitarora.in.ibm.com> (raw)

The ext4_ext_get_blocks() and ext4_ext_insert_extent() routines do not
check for extent overlap, when a new extent needs to be inserted in an
inode. An overlap is possible when the new extent being inserted has
ee_block that is not part of any of the existing extents, but the
tail/center portion of this new extent _is_. This is possible only when
we are writing/preallocating blocks across a hole.

This problem was first sighted while stress testing (using modified
fsx-linux stress test) persistent preallocation patches that I posted
earlier.  Though I am not able to reproduce this bug (extent overlap)
without the persistent preallocation patches (because a write through a
hole results in get_blocks() of a single block at a time), but I think
that it is an independant problem and should be solved with a separate
patch. Hence this patch.

Comments please. Thanks!

Signed-off-by: Amit Arora (aarora@in.ibm.com)
---
 fs/ext4/extents.c               |   71 +++++++++++++++++++++++++++++++++++++---
 include/linux/ext4_fs_extents.h |    1 
 2 files changed, 68 insertions(+), 4 deletions(-)

Index: linux-2.6.19.prealloc/fs/ext4/extents.c
===================================================================
--- linux-2.6.19.prealloc.orig/fs/ext4/extents.c	2007-01-02 14:21:57.000000000 +0530
+++ linux-2.6.19.prealloc/fs/ext4/extents.c	2007-01-02 14:22:00.000000000 +0530
@@ -1119,6 +1119,44 @@
 }
 
 /*
+ * ext4_ext_check_overlap:
+ * check if a portion of the "newext" extent overlaps with an
+ * existing extent.
+ */
+struct ext4_extent * ext4_ext_check_overlap(struct inode *inode,
+					struct ext4_extent *newext)
+{
+	struct ext4_ext_path *path;
+	struct ext4_extent *ex;
+	unsigned int depth, b1, b2, len1;
+
+	b1 = le32_to_cpu(newext->ee_block);
+	len1 = le16_to_cpu(newext->ee_len);
+	path = ext4_ext_find_extent(inode, b1, NULL);
+	if (IS_ERR(path))
+		return NULL;
+
+	depth = ext_depth(inode);
+	ex = path[depth].p_ext;
+	if (!ex)
+		return NULL;
+
+	b2 = ext4_ext_next_allocated_block(path);
+	if (b2 == EXT_MAX_BLOCK)
+		return NULL;
+	path = ext4_ext_find_extent(inode, b2, path);
+	if (IS_ERR(path))
+		return NULL;
+	BUG_ON(path[depth].p_hdr == NULL);
+	ex = path[depth].p_ext;
+
+	if (b1 + len1 > b2)
+		return ex;
+
+	return NULL;
+}
+
+/*
  * ext4_ext_insert_extent:
  * tries to merge requsted extent into the existing extent or
  * inserts requested extent as new one into the tree,
@@ -1129,7 +1167,7 @@
 				struct ext4_extent *newext)
 {
 	struct ext4_extent_header * eh;
-	struct ext4_extent *ex, *fex;
+	struct ext4_extent *ex, *fex, *rex;
 	struct ext4_extent *nearex; /* nearest extent */
 	struct ext4_ext_path *npath = NULL;
 	int depth, len, err, next;
@@ -1139,6 +1177,18 @@
 	ex = path[depth].p_ext;
 	BUG_ON(path[depth].p_hdr == NULL);
 
+	/* check for overlap */
+	rex = ext4_ext_check_overlap(inode, newext);
+	if (rex) {
+		printk(KERN_ERR "ERROR: ex=%u/%u overlaps newext=%u/%u\n",
+				le32_to_cpu(rex->ee_block),
+				le16_to_cpu(rex->ee_len),
+				le32_to_cpu(newext->ee_block),
+				le16_to_cpu(newext->ee_len));
+		ext4_ext_show_leaf(inode, path);
+		BUG();
+	}
+
 	/* try to insert block into found extent and return */
 	if (ex && ext4_can_extents_be_merged(inode, ex, newext)) {
 		ext_debug("append %d block to %d:%d (from %llu)\n",
@@ -1921,7 +1971,7 @@
 			int create, int extend_disksize)
 {
 	struct ext4_ext_path *path = NULL;
-	struct ext4_extent newex, *ex;
+	struct ext4_extent newex, *ex, *ex2;
 	ext4_fsblk_t goal, newblock;
 	int err = 0, depth;
 	unsigned long allocated = 0;
@@ -1984,6 +2034,10 @@
 		 */
 		if (ee_len > EXT_MAX_LEN)
 			goto out2;
+
+		if (iblock < ee_block && iblock + max_blocks >= ee_block)
+			allocated = ee_block - iblock;
+
 		/* if found extent covers block, simply return it */
 	        if (iblock >= ee_block && iblock < ee_block + ee_len) {
 			newblock = iblock - ee_block + ee_start;
@@ -2016,7 +2070,17 @@
 
 	/* allocate new block */
 	goal = ext4_ext_find_goal(inode, path, iblock);
-	allocated = max_blocks;
+
+	/* Check if we can really insert (iblock)::(iblock+max_blocks) extent */
+	newex.ee_block = cpu_to_le32(iblock);
+	if (!allocated) {
+		newex.ee_len = cpu_to_le16(max_blocks);
+		ex2 = ext4_ext_check_overlap(inode, &newex);
+		if (ex2)
+			allocated = le32_to_cpu(ex2->ee_block) - iblock;
+		else
+			allocated = max_blocks;
+	}
 	newblock = ext4_new_blocks(handle, inode, goal, &allocated, &err);
 	if (!newblock)
 		goto out2;
@@ -2024,7 +2088,6 @@
 			goal, newblock, allocated);
 
 	/* try to insert new extent into found leaf and return */
-	newex.ee_block = cpu_to_le32(iblock);
 	ext4_ext_store_pblock(&newex, newblock);
 	newex.ee_len = cpu_to_le16(allocated);
 	err = ext4_ext_insert_extent(handle, inode, path, &newex);
Index: linux-2.6.19.prealloc/include/linux/ext4_fs_extents.h
===================================================================
--- linux-2.6.19.prealloc.orig/include/linux/ext4_fs_extents.h	2007-01-02 14:21:57.000000000 +0530
+++ linux-2.6.19.prealloc/include/linux/ext4_fs_extents.h	2007-01-02 14:22:00.000000000 +0530
@@ -190,6 +190,7 @@
 
 extern int ext4_extent_tree_init(handle_t *, struct inode *);
 extern int ext4_ext_calc_credits_for_insert(struct inode *, struct ext4_ext_path *);
+extern struct ext4_extent * ext4_ext_check_overlap(struct inode *, struct ext4_extent *);
 extern int ext4_ext_insert_extent(handle_t *, struct inode *, struct ext4_ext_path *, struct ext4_extent *);
 extern int ext4_ext_walk_space(struct inode *, unsigned long, unsigned long, ext_prepare_callback, void *);
 extern struct ext4_ext_path * ext4_ext_find_extent(struct inode *, int, struct ext4_ext_path *);
--
Regards,
Amit Arora

             reply	other threads:[~2007-01-02  9:09 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-02  9:09 Amit K. Arora [this message]
2007-01-02  9:25 ` [PATCH 1/1] Extent overlap bugfix in ext4 Alex Tomas
2007-01-02  9:47   ` Amit K. Arora
2007-01-03  9:44     ` Alex Tomas
2007-01-03 18:07       ` Mingming Cao
2007-01-04  8:13         ` Amit K. Arora
2007-01-04 10:04         ` Alex Tomas
2007-01-04 18:23           ` Mingming Cao
2007-01-03  1:35 ` Mingming Cao
2007-01-03  6:06   ` Amit K. Arora
2007-01-04  8:54     ` [PATCH 1/1 version2] " Amit K. Arora
2007-01-04 10:25       ` Alex Tomas
2007-01-04 11:16         ` Amit K. Arora
2007-01-04 10:39       ` Alex Tomas
2007-01-04 11:27         ` Amit K. Arora
2007-01-04 11:37           ` Alex Tomas
2007-01-04 17:23             ` Amit K. Arora
2007-01-04 18:50               ` Mingming Cao
2007-01-04 19:19                 ` Alex Tomas
2007-01-05 12:13                 ` Amit K. Arora
2007-01-09  5:51                   ` [PATCH 1/1 version3] " Amit K. Arora
2007-01-04 19:03               ` [PATCH 1/1 version2] " Alex Tomas
2007-01-04 19:47                 ` Mingming Cao
2007-01-05  6:18                   ` Amit K. Arora

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=20070102090909.GA20503@amitarora.in.ibm.com \
    --to=aarora@linux.vnet.ibm.com \
    --cc=alex@clusterfs.com \
    --cc=cmm@us.ibm.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=suparna@in.ibm.com \
    /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).