All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.