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

[Surbhi Palande]

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