[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m3irfobg29.fsf@bzzz.home.net>
Date: Wed, 03 Jan 2007 12:44:14 +0300
From: Alex Tomas <alex@...sterfs.com>
To: "Amit K. Arora" <aarora@...ux.vnet.ibm.com>
Cc: Alex Tomas <alex@...sterfs.com>, linux-ext4@...r.kernel.org,
suparna@...ibm.com, cmm@...ibm.com
Subject: Re: [PATCH 1/1] Extent overlap bugfix in ext4
>>>>> Amit K Arora (AKA) writes:
AKA> On Tue, Jan 02, 2007 at 12:25:21PM +0300, Alex Tomas (AT) wrote:
>> >>>>> Amit K Arora (AKA) writes:
>>
AKA> The ext4_ext_get_blocks() and ext4_ext_insert_extent() routines do not
AKA> check for extent overlap, when a new extent needs to be inserted in an
AKA> inode. An overlap is possible when the new extent being inserted has
AKA> ee_block that is not part of any of the existing extents, but the
AKA> tail/center portion of this new extent _is_. This is possible only when
AKA> we are writing/preallocating blocks across a hole.
>>
AT> not sure I understand ... you shouldn't insert an extent that overlap
AT> any existing extent. when you write block(s), you first check is
AT> it already allocated and insert new extent only if it's not.
AKA> You are right. That is what this patch does.
AKA> The current ext4 code is inserting an overlapped extent in a particular
AKA> scenario (explained above). The suggested patch fixes this by having a
AKA> check in get_blocks() for _not_ inserting an extent that may overlap
AKA> with an existing one.
I think that stuff that converts uninitialized blocks
to initialized ones should be a separate codepath and
shouldn't be done in the insert path. and an insert
(basic tree manipulation) should BUG_ON() one tries
to add extent with a block which is already covered
by the tree.
IMHO, get_blocks() should look like:
path = find_path()
if (found extent covers request block(s)) {
if (extent is uninitialized) {
convert();
}
}
where
function convert()
{
/* adopt existing extent so that it
* doesn't cover requested blocks */
/* insert head or tail of existing
* extent, if necessary */
/* insert new extent of initialized blocks */
}
thanks, Alex
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists