Re: [PATCH v3] 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 Mon, May 16, 2011 at 7:53 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Mon, May 16, 2011 at 2:27 PM, Yongqiang Yang <xiaoqiangnk@xxxxxxxxx> wrote:
>> To get delayed-extent information, ext4_ext_fiemap_cb() looks up
>> pagecache, it thus collects information starting from a page's
>> head block.
>>
>> If blocksize < pagesize, the beginning blocks of a page may lies
>> before the request range. So ext4_ext_fiemap_cb() should proceed
>> ignoring them, because they has been handled before. If no mapped
>> buffer in the range is found in the 1st page, we need to look up
>> the 2nd page, otherwise delayed-extents after a hole will be ignored.
>
> Does this patch fix the endless loop I encountered?
Yes, it fixes another bug.
> If so, you may want to add to commit message:
> - Reported-by: Amir..
> - that the patch fixes a hang bug
> - how was it reproduced
> - which commit introduced the bug
Sorry, I forgot to add these.   I rewrote the xfstests 225 and found
another bug which the old 225 could not find.
I think I should resend this patch.

>
>>
>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx>
>> ---
>>  fs/ext4/extents.c |   51 ++++++++++++++++++++++++++++++++++++---------------
>>  1 files changed, 36 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index e363f21..474eaad 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -3680,6 +3680,7 @@ static int ext4_ext_fiemap_cb(struct inode *inode, struct ext4_ext_path *path,
>>                pgoff_t         last_offset;
>>                pgoff_t         offset;
>>                pgoff_t         index;
>> +               pgoff_t         start_index = 0;
>>                struct page     **pages = NULL;
>>                struct buffer_head *bh = NULL;
>>                struct buffer_head *head = NULL;
>> @@ -3706,39 +3707,57 @@ out:
>>                                kfree(pages);
>>                                return EXT_CONTINUE;
>>                        }
>> +                       index = 0;
>>
>> +next_page:
>>                        /* Try to find the 1st mapped buffer. */
>> -                       end = ((__u64)pages[0]->index << PAGE_SHIFT) >>
>> +                       end = ((__u64)pages[index]->index << PAGE_SHIFT) >>
>>                                  blksize_bits;
>> -                       if (!page_has_buffers(pages[0]))
>> +                       if (!page_has_buffers(pages[index]))
>>                                goto out;
>> -                       head = page_buffers(pages[0]);
>> +                       head = page_buffers(pages[index]);
>>                        if (!head)
>>                                goto out;
>> -
>> +
>> +                       index++;
>>                        bh = head;
>>                        do {
>> -                               if (buffer_mapped(bh)) {
>> +                               if (end >= newex->ec_block +
>> +                                       newex->ec_len)
>> +                                       /* The buffer is out of
>> +                                        * the request range.
>> +                                        */
>> +                                       goto out;
>> +
>> +                               if (buffer_mapped(bh) &&
>> +                                   end >= newex->ec_block) {
>> +                                       start_index = index - 1;
>>                                        /* get the 1st mapped buffer. */
>> -                                       if (end > newex->ec_block +
>> -                                               newex->ec_len)
>> -                                               /* The buffer is out of
>> -                                                * the request range.
>> -                                                */
>> -                                               goto out;
>>                                        goto found_mapped_buffer;
>>                                }
>> +
>>                                bh = bh->b_this_page;
>>                                end++;
>>                        } while (bh != head);
>>
>> -                       /* No mapped buffer found. */
>> -                       goto out;
>> +                       /* No mapped buffer in the range found in this page,
>> +                        * We need to look up next page.
>> +                        */
>> +                       if (index >= ret) {
>> +                               /* There is no page left, but we need to limit
>> +                                * newex->ec_len.
>> +                                */
>> +                               newex->ec_len = end - newex->ec_block;
>> +                               goto out;
>> +                       }
>> +                       goto next_page;
>>                } else {
>>                        /*Find contiguous delayed buffers. */
>>                        if (ret > 0 && pages[0]->index == last_offset)
>>                                head = page_buffers(pages[0]);
>>                        bh = head;
>> +                       index = 1;
>> +                       start_index = 0;
>>                }
>>
>>  found_mapped_buffer:
>> @@ -3761,7 +3780,7 @@ found_mapped_buffer:
>>                                end++;
>>                        } while (bh != head);
>>
>> -                       for (index = 1; index < ret; index++) {
>> +                       for (; index < ret; index++) {
>>                                if (!page_has_buffers(pages[index])) {
>>                                        bh = NULL;
>>                                        break;
>> @@ -3771,8 +3790,10 @@ found_mapped_buffer:
>>                                        bh = NULL;
>>                                        break;
>>                                }
>> +
>>                                if (pages[index]->index !=
>> -                                       pages[0]->index + index) {
>> +                                   pages[start_index]->index + index
>> +                                   - start_index) {
>>                                        /* Blocks are not contiguous. */
>>                                        bh = NULL;
>>                                        break;
>> --
>> 1.7.5.1
>>
>> --
>> 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
>>
>



-- 
Best Wishes
Yongqiang Yang
ÿôèº{.nÇ+?·?®?­?+%?Ëÿ±éݶ¥?wÿº{.nÇ+?·¥?{±ýìmãø§¶?¡Ü¨}©?²Æ zÚ&j:+v?¨þø¯ù®w¥þ?à2?Þ?¨è­Ú&¢)ß¡«a¶Úÿÿûàz¿äz¹Þ?ú+?ù???Ý¢jÿ?wèþf



[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