[PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.

[PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.

[Qu Wenruo]

When page aligned start and len passed to extent_fiemap(), the result is
good, but when start and len is not aligned, e.g. start = 1 and len =
4095 is passed to extent_fiemap(), it returns no extent.

The problem is that start and len is all rounded up which causes the
problem. This patch will round down start and round up (start + len) to
return right extent.

Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
changelog:
v2: reword the description(ALIGN rounds up, not rounds down).
---
 fs/btrfs/extent_io.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a389820..1c70cff 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 		return -ENOMEM;
 	path->leave_spinning = 1;
 
-	start = ALIGN(start, BTRFS_I(inode)->root->sectorsize);
-	len = ALIGN(len, BTRFS_I(inode)->root->sectorsize);
+	start = round_down(start, BTRFS_I(inode)->root->sectorsize);
+	len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start;
 
 	/*
 	 * lookup the last file extent.  We're not using i_size here
-- 
2.0.2

Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.

[Satoru Takeuchi]

Hi Qu,

(2014/07/25 10:49), Qu Wenruo wrote:
> When page aligned start and len passed to extent_fiemap(), the result is
> good, but when start and len is not aligned, e.g. start = 1 and len =
> 4095 is passed to extent_fiemap(), it returns no extent.
> 
> The problem is that start and len is all rounded up which causes the
> problem. This patch will round down start and round up (start + len) to
> return right extent.
> 
> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> changelog:
> v2: reword the description(ALIGN rounds up, not rounds down).
> ---
>   fs/btrfs/extent_io.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index a389820..1c70cff 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>   		return -ENOMEM;
>   	path->leave_spinning = 1;
>   
> -	start = ALIGN(start, BTRFS_I(inode)->root->sectorsize);
> -	len = ALIGN(len, BTRFS_I(inode)->root->sectorsize);
> +	start = round_down(start, BTRFS_I(inode)->root->sectorsize);
> +	len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start;

I have one question.

Why isn't it "len = round_up(len, BTRFS_I(inode)->root->sectorsize);" ?
Old behavior make len sectorsize aligned. However, if start is not
aligned to sectorsize, new bahavior make len not sectorsize aligned.
Does it an expected behavior?

CMIIW. I'm not good at extent code.

Thanks,
Satoru

>   
>   	/*
>   	 * lookup the last file extent.  We're not using i_size here
> 

Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH v2] btrfs: Return right extent when fiemap gives 
unaligned offset and len.
From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org>
Date: 2014年07月25日 15:52
> Hi Qu,
>
> (2014/07/25 10:49), Qu Wenruo wrote:
>> When page aligned start and len passed to extent_fiemap(), the result is
>> good, but when start and len is not aligned, e.g. start = 1 and len =
>> 4095 is passed to extent_fiemap(), it returns no extent.
>>
>> The problem is that start and len is all rounded up which causes the
>> problem. This patch will round down start and round up (start + len) to
>> return right extent.
>>
>> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>> changelog:
>> v2: reword the description(ALIGN rounds up, not rounds down).
>> ---
>>    fs/btrfs/extent_io.c | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index a389820..1c70cff 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>    		return -ENOMEM;
>>    	path->leave_spinning = 1;
>>    
>> -	start = ALIGN(start, BTRFS_I(inode)->root->sectorsize);
>> -	len = ALIGN(len, BTRFS_I(inode)->root->sectorsize);
>> +	start = round_down(start, BTRFS_I(inode)->root->sectorsize);
>> +	len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start;
> I have one question.
>
> Why isn't it "len = round_up(len, BTRFS_I(inode)->root->sectorsize);" ?
> Old behavior make len sectorsize aligned. However, if start is not
> aligned to sectorsize, new bahavior make len not sectorsize aligned.
> Does it an expected behavior?
In fact, in my code, len is also aligned.
The calculation is somewhat easy, round_down original 'start' and 
round_up original end(in the code, max is original
unaligned end position),
then calculate 'len' by minus aligned end with aligned 'start'.
Since the *new* 'start' is already aligned and the new end is aligned, 
the len will also be aligned.

Thanks,
Qu
>
> CMIIW. I'm not good at extent code.
>
> Thanks,
> Satoru
>
>>    
>>    	/*
>>    	 * lookup the last file extent.  We're not using i_size here
>>

Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.

[Satoru Takeuchi]

Hi Qu,

(2014/07/25 17:16), Qu Wenruo wrote:
>
> -------- Original Message --------
> Subject: Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.
> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org>
> Date: 2014年07月25日 15:52
>> Hi Qu,
>>
>> (2014/07/25 10:49), Qu Wenruo wrote:
>>> When page aligned start and len passed to extent_fiemap(), the result is
>>> good, but when start and len is not aligned, e.g. start = 1 and len =
>>> 4095 is passed to extent_fiemap(), it returns no extent.
>>>
>>> The problem is that start and len is all rounded up which causes the
>>> problem. This patch will round down start and round up (start + len) to
>>> return right extent.
>>>
>>> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> ---
>>> changelog:
>>> v2: reword the description(ALIGN rounds up, not rounds down).
>>> ---
>>>    fs/btrfs/extent_io.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>>> index a389820..1c70cff 100644
>>> --- a/fs/btrfs/extent_io.c
>>> +++ b/fs/btrfs/extent_io.c
>>> @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>            return -ENOMEM;
>>>        path->leave_spinning = 1;
>>> -    start = ALIGN(start, BTRFS_I(inode)->root->sectorsize);
>>> -    len = ALIGN(len, BTRFS_I(inode)->root->sectorsize);
>>> +    start = round_down(start, BTRFS_I(inode)->root->sectorsize);
>>> +    len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start;
>> I have one question.
>>
>> Why isn't it "len = round_up(len, BTRFS_I(inode)->root->sectorsize);" ?
>> Old behavior make len sectorsize aligned. However, if start is not
>> aligned to sectorsize, new bahavior make len not sectorsize aligned.
>> Does it an expected behavior?
> In fact, in my code, len is also aligned.
> The calculation is somewhat easy, round_down original 'start' and round_up original end(in the code, max is original
> unaligned end position),
> then calculate 'len' by minus aligned end with aligned 'start'.
> Since the *new* 'start' is already aligned and the new end is aligned, the len will also be aligned.

Thank you for your explanation. I understood.
I forgot to that "start" is aligned here.

Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

>
> Thanks,
> Qu
>>
>> CMIIW. I'm not good at extent code.
>>
>> Thanks,
>> Satoru
>>
>>>        /*
>>>         * lookup the last file extent.  We're not using i_size here
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len.

[David Sterba]

On Fri, Jul 25, 2014 at 09:49:03AM +0800, Qu Wenruo wrote:
> When page aligned start and len passed to extent_fiemap(), the result is
> good, but when start and len is not aligned, e.g. start = 1 and len =
> 4095 is passed to extent_fiemap(), it returns no extent.
> 
> The problem is that start and len is all rounded up which causes the
> problem. This patch will round down start and round up (start + len) to
> return right extent.
> 
> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Reviewed-by: David Sterba <dsterba@suse.cz>

> ---
> changelog:
> v2: reword the description(ALIGN rounds up, not rounds down).

JFYI, if you're just rewording the changelog, you can keep the
reviewed-by tags.