Re: [PATCH v2] ext4:Let ext4_ext_fiemap_cb() handle blocks before request range correctly.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, May 10, 2011 at 11:08 PM, Yongqiang Yang <xiaoqiangnk@xxxxxxxxx> wrote:
> On Tue, May 10, 2011 at 10:35 PM, Lukas Czerner <lczerner@xxxxxxxxxx> wrote:
>> On Tue, 10 May 2011, Yongqiang Yang wrote:
>>
>>> v1->v2:
>>>   Add more specific description.
>>>
>>> To get delayed-extent information, ext4_ext_fiemap_cb() lookups pagecache,
>>> it thus collects information starting from a page's head block.
>>> If blockszie < pagesize, the beginning blocks of a page may lie before the
>>> range.  ext4_ext_fiemap_cb() should proceed ignoring them, because they
>>> has been handled before.
>>
>> Thanks for the description, but I have one question below.
>>
>>>
>>> Reported-by: Amir Goldstein <amir73il@xxxxxxxxxxxx>
>>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
>>> ---
>>>  fs/ext4/extents.c |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>>> index e363f21..ec37109 100644
>>> --- a/fs/ext4/extents.c
>>> +++ b/fs/ext4/extents.c
>>> @@ -3718,9 +3718,14 @@ out:
>>>
>>>                       bh = head;
>>>                       do {
>>> +                             if (end < newex->ec_block)
>>> +                                     /* The buffer is not in
>>> +                                      * the request range.
>>> +                                      */
>>> +                                     continue;
>> So if the condition is true, then we might leave the loop, because (bh ==
>> head) in the first iteration, is that desired behavior? Also if we hit
>> this condition in other than first iteration we will spin forever. I do
>> not think this is right. How did you tested this case ?
> You are right.  Maybe the patch sent out is not same as the patch with
> which I tested.  What a stupid error!
> I am not sure what happened now.  I will check the patch with which I
> tested tomorrow.
>
> My working tree passed xfstests 225 in both 1k blocksize and 4k
> blocksize cases.
I checked the code I tested with.  Indeed I tested with this ugly
patch and xfstests 225 passed.  I tested again and results were same '
PASSED'.

I had a look into xfstests 225 and found that there are problems in
xfstests 225 so it can not find the bug induced by this ugly patch.
It seems that current xfstests 225 has a lot of problems.

I am fixing xfstests 225 to enable it find this bug.

Yongqiang.

>
> Sorry!
>
> Yongqiang.
>>
>> Thanks!
>> -Lukas
>>
>>>                               if (buffer_mapped(bh)) {
>>>                                       /* get the 1st mapped buffer. */
>>> -                                     if (end > newex->ec_block +
>>> +                                     if (end >= newex->ec_block +
>>>                                               newex->ec_len)
>>>                                               /* The buffer is out of
>>>                                                * the request range.
>>>
>>
>
>
>
> --
> Best Wishes
> Yongqiang Yang
>



-- 
Best Wishes
Yongqiang Yang
--
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


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux