[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <BANLkTikwCDBWU0+7fgtY4OXsZKe9yaiMwA@mail.gmail.com>
Date: Sun, 22 May 2011 19:59:40 +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 Sun, May 22, 2011 at 4:35 PM, Amir Goldstein <amir73il@...il.com> wrote:
> On Mon, May 16, 2011 at 4:15 PM, Yongqiang Yang <xiaoqiangnk@...il.com> wrote:
>> 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.
>
> Hi Yongqiang,
>
> Did you resend this patch? I see that Ted hasn't picked it up yet.
> Apparently, he is expecting to get a new version from you with proper
> commit log (I think he mentioned that on last week's confcall).
Sorry, I had a travel in last three days without a computer. I will
send it out tomorrow.
>
> Amir.
>
>>
>>>
>>>>
>>>> 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
>>
>
--
Best Wishes
Yongqiang Yang
--
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