[Top] [All Lists]

Re: xfsprogs: Fix for xfstest 252 hang on ext4

To: Allison Henderson <achender@xxxxxxxxxxxxxxxxxx>
Subject: Re: xfsprogs: Fix for xfstest 252 hang on ext4
From: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
Date: Wed, 1 Jun 2011 09:58:45 +0800
Cc: Ext4 Developers List <linux-ext4@xxxxxxxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, "Ted Ts'o" <tytso@xxxxxxx>
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=PILsl9MFzDDPbxfqSn2v/PbeER0r9rXj+4mlW097rJc=; b=uICJXmnqf+7THIZjpg/3Pz76hqO3skc7YurwqkpXNpAL1dzzBp02iji1XovxGeAC8P CusVGcIu8DtDtjd98AqCMw1f2rC5RbE1qDNKHD/4Y1jLgeqWR/lawueD3DxiCGmbX4nr I9uqkh6Nr7eB3c8R0JBiaym+hKmFOrQX1IIVM=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=mTnK9mmRidJv8KVs9cNDCQwsWLbd8Vp8UvcmLk9GDo54wbxNtqnFLoRwHJJIXJlgHa 5isFJYkuDC0YPxOcvCICnLEAoKI7yijMPZEbH99IMCxOWKmrFS/C6BRQ+77aPjlkLjke kJ8gRLAKJOzogUzuoLMDc2GTEbwbUCpUGJG8Q=
In-reply-to: <4DE50881.90401@xxxxxxxxxxxxxxxxxx>
References: <4DDAC4EF.1050702@xxxxxxxxxxxxxxxxxx> <BANLkTiks0NSDTSPN8n446JHWhKtM61_=Mw@xxxxxxxxxxxxxx> <4DDB1A0C.5030502@xxxxxxxxxxxxxxxxxx> <4DE50881.90401@xxxxxxxxxxxxxxxxxx>
On Tue, May 31, 2011 at 11:25 PM, Allison Henderson
<achender@xxxxxxxxxxxxxxxxxx> wrote:
> On 5/23/2011 7:38 PM, Allison Henderson wrote:
>> On 5/23/2011 6:16 PM, Yongqiang Yang wrote:
>>> On Tue, May 24, 2011 at 4:34 AM, Allison Henderson
>>> <achender@xxxxxxxxxxxxxxxxxx> wrote:
>>>> Hi all,
>>>> While trying to add more punch hole tests to xfstest, I found that
>>>> test 252 hangs on ext4 due to a loop in xfsprogs that does not exit.
>>>> XFS gets out of this loop because there is logic in the loop that
>>>> looks for the last extent flag and breaks out. But it looks like ext4
>>>> does not return a last extent when the file has a hole at the end. I
>>>> am not sure if this is the correct behavior or not, so I will copy
>>>> the ext4 folks on this too. Below is a copy of the fix for xfsprogs:
>>> Hi there,
>>> What's blocksize of the tested ext4? For now, ext4 returns
>>> LAST_EXTENT if the logical offset covered by the extent is greater
>>> than file size, so if there is a hole at the end, no last extent is
>>> returned. Thx!
>>> Yongqiang.
>> Hi there,
>> The block size I've been using is 4096. As long as that behavior is
>> expected, I think the test will be ok with just the xfsprogs fix,
>> though. Thx!
>> Allison Henderson
>>>> diff --git a/io/fiemap.c b/io/fiemap.c
>>>> index fa990cc..81fc92c 100644
>>>> --- a/io/fiemap.c
>>>> +++ b/io/fiemap.c
>>>> @@ -246,7 +246,7 @@ fiemap_f(
>>>> flg_w, _("FLAGS"));
>>>> }
>>>> - while (!last&& ((cur_extent + 1) != max_extents)) {
>>>> + while (!last&& (cur_extent<= max_extents)) {
>>>> if (max_extents)
>>>> num_extents = min(num_extents,
>>>> max_extents - (cur_extent + 1));
>>>> It looks like the loop enters with last=0, cur_extents=0, and
>>>> max_extents = 0, and on the first iteration cur_extents get set to 2,
>>>> so we dont see ((cur_extent + 1) == max_extents for a very long time.
>>>> I doubt the logic was meant to work that way, so this patch should
>>>> fix it, but I wanted to make sure that the fiemap for ext4 is working
>>>> as intended too. Feed back appreciated! Thx all!
>>>> Allison Henderson
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Hi all,
>>> I haven't heard much back on this patch, so Im just poking this
>>> thread to make sure it doesn't get forgotten. I have some patches
>>> out there for punch hole, and I'm currently looking at fixing up
>>> some older punch hole tests in the dmapi code, but they wont do much
>>> good for ext4 with out this fix.  If I could get a quick peek from
>>> some one on the xfs list for this patch, that would be much
>>> appreciated.  Thx all!
>> If ext4 is not setting the last extent flag on the last extent then
>> that's an ext4 bug that the test has detected, right? And so you
>> should be fixing ext4 rather than modifying the test to hide the
>> different behaviour?
>> Cheer
>>-- Dave Chinner david@xxxxxxxxxxxxx
> Hi All,
> Sorry, I should have poked the thread with Yongqiang's response, so I will
> move the dialog into this thread.  At the moment, it sounds like the fiemap
> for ext4 is working as intended.  Yongqiang, do you agree that the fiemap
> for ext4 should be changed?  I think you are more familiar with this part of
Yes, I agree.  ext4 should return LAST extent.  I am thinking if we
can find a new solution collecting extents.

Maybe we can insert delayed extents into extent tree. This way fiemap
will be simpler and much more efficient.

I would like to throw out the proposal inserting delayed-extents into
extent tree.  What will it bring?  AFAIK it will bring:
1. We need to down i_data_sem in delayed write-path to insert
delayed-extents into the tree without journaling it.

2. When we come to block allocation, we can convert delayed-extents to
normal extents.

There is a problem that the solution can be only used in ordered mode.
So what are your opinions?


> the code than I am, and I just want to make sure we find a solution that
> everyone is happy with.  Thx!
> Allison Henderson
>> _______________________________________________
>> xfs mailing list
>> xfs@xxxxxxxxxxx
>> http://oss.sgi.com/mailman/listinfo/xfs

Best Wishes
Yongqiang Yang

<Prev in Thread] Current Thread [Next in Thread>