[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BANLkTikeXcqDGQhx9v6RrRUjOucV4wnzCA@mail.gmail.com>
Date: Mon, 16 May 2011 21:15:51 +0800
From: Yongqiang Yang <xiaoqiangnk@...il.com>
To: Amir Goldstein <amir73il@...il.com>
Cc: linux-ext4@...r.kernel.org, lczerner@...hat.com, tytso@....edu
Subject: Re: [PATCH v3] ext4:Let ext4_ext_fiemap_cb() handle blocks before
request range correctly.
On Mon, May 16, 2011 at 7:53 PM, Amir Goldstein <amir73il@...il.com> wrote:
> On Mon, May 16, 2011 at 2:27 PM, Yongqiang Yang <xiaoqiangnk@...il.com> 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@...il.com>
>> ---
>> 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@...r.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
--
Best Wishes
Yongqiang Yang
Powered by blists - more mailing lists