[PATCH] ext4: correctly calculate number of blocks for fiemap

[Surbhi Palande]

Hi Folks,

I was not clear enough about this patch before.

The patch that I picked was cherry picked from upstream and is the 
correct patch. The one on launchpad was obsolete and I have not picked 
that one. Please do consider this patch (cherry picked from upstream) 
for Lucid.

Thanks!

Warm Regards,
Surbhi.


On 04/01/2010 03:44 PM, Surbhi Palande wrote:
> On Thu, 2010-04-01 at 07:53 -0700, Brad Figg wrote:
>> On 04/01/2010 02:37 AM, Stefan Bader wrote:
>>> Ok, looks good to me now.
>>>
>>> Surbhi Palande wrote:
>>>> The following cherry-picked patch fixes the calculation of extents in fiemap.
>>>> Without this patch a user of fiemap could be misled into thinking that there
>>>> are no extents in a file when actually there are. This patch is necessary for
>>>> code such as ureadahead which uses fiemap to find out if the file extents have
>>>> increased.
>>>>
>>>> The commit id in the upstream code is: aca92ff6f57c000d1b4523e383c8bd6b8269b8b1
>>>> This patch fixes bug 474597 on LP. Do consider merging this in Lucid.
>>>>
>>>>   From dfaa33f0502a8bf9dff0e596783a40ce330c7a9b Mon Sep 17 00:00:00 2001
>>>> From: Leonard Michlmayr<leonard.michlmayr at gmail.com>
>>>> Date: Thu, 4 Mar 2010 17:07:28 -0500
>>>> Subject: [PATCH] ext4: correctly calculate number of blocks for fiemap
>>>>
>>>> ext4_fiemap() rounds the length of the requested range down to
>>>> blocksize, which is is not the true number of blocks that cover the
>>>> requested region.  This problem is especially impressive if the user
>>>> requests only the first byte of a file: not a single extent will be
>>>> reported.
>>>>
>>>> We fix this by calculating the last block of the region and then
>>>> subtract to find the number of blocks in the extents.
>>>>
>>>> Signed-off-by: Leonard Michlmayr<leonard.michlmayr at gmail.com>
>>>> Signed-off-by: "Theodore Ts'o"<tytso at mit.edu>
>>>> (cherry picked from commit aca92ff6f57c000d1b4523e383c8bd6b8269b8b1 upstream)
>>>>
>>>> BugLink: http://launchpad.net/bugs/474597
>>>> Signed-off-by: Surbhi Palande<surbhi.palande at canonical.com>
>>> Acked-by: Stefan Bader<stefan.bader at canonical.com>
>>>> ---
>>>>    fs/ext4/extents.c |    9 +++++++--
>>>>    1 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>>>> index 8b8bae4..f199d20 100644
>>>> --- a/fs/ext4/extents.c
>>>> +++ b/fs/ext4/extents.c
>>>> @@ -3711,7 +3711,6 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>    		__u64 start, __u64 len)
>>>>    {
>>>>    	ext4_lblk_t start_blk;
>>>> -	ext4_lblk_t len_blks;
>>>>    	int error = 0;
>>>>
>>>>    	/* fallback to generic here if not in extents fmt */
>>>> @@ -3725,8 +3724,14 @@ int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>>>>    	if (fieinfo->fi_flags&   FIEMAP_FLAG_XATTR) {
>>>>    		error = ext4_xattr_fiemap(inode, fieinfo);
>>>>    	} else {
>>>> +		ext4_lblk_t len_blks;
>>>> +		__u64 last_blk;
>>>> +
>>>>    		start_blk = start>>   inode->i_sb->s_blocksize_bits;
>>>> -		len_blks = len>>   inode->i_sb->s_blocksize_bits;
>>>> +		last_blk = (start + len - 1)>>   inode->i_sb->s_blocksize_bits;
>>>> +		if (last_blk>= EXT_MAX_BLOCK)
>>>> +			last_blk = EXT_MAX_BLOCK-1;
>>>> +		len_blks = ((ext4_lblk_t) last_blk) - start_blk + 1;
>>>>
>>>>    		/*
>>>>    		 * Walk the extent tree gathering extent information.
>>>
>>>
>>
>> Maybe I'm misreading this bug according to
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/474597/comments/7
>> ted applied a different patch to fix this issue.
>>
> Brad, thanks for pointing this out.
>
> The patch that actually got into the linux kernel does seem to have the
> additional check (missing from whats on launchpad)
> +if (last_blk>= EXT_MAX_BLOCK)
> +	last_blk = EXT_MAX_BLOCK-1;
> This check seems necessary for the corner case.
>
> I have pointed the difference out on the buglink too.
>
> Warm Regards,
> Surbhi.
>
>> Brad
>
>
>