[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BANLkTimkxGE9DacKCFqPKp_20=7iQvjOpQ@mail.gmail.com>
Date: Sun, 22 May 2011 11:35:59 +0300
From: Amir Goldstein <amir73il@...il.com>
To: Yongqiang Yang <xiaoqiangnk@...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 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).
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
>
Powered by blists - more mailing lists