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

[Andy Whitcroft]

On Fri, Apr 02, 2010 at 01:44:41AM +0300, 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.

That implies that there is a better patch out there.  Am I expecting
that patch to be posted instead here?

-apw