lkml.org 
[lkml]   [2010]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] cleanup block based fiemap


On Fri, 23 Apr 2010, Josef Bacik wrote:
>
> I'm resending this patch again since it doesn't seem to have made it in yet.
> The generic block fiemap stuff doesn't use the right typing and has a problem
> with not setting the last extent flag properly. Also there is an issue with
> GFS2 where it doesn't like non-block aligned requests, so this fixes all these
> issues. Thanks,

I'd really like the patch to clean up the crazy stuff too.

As-is, there's at least two remaining issues I see from just reading the
patch:

> + if (len >= i_size_read(inode)) {
> + whole_file = true;
> + len = i_size_read(inode);
> + }
...
> if (!past_eof &&
> blk_to_logical(inode, start_blk) >=
> - blk_to_logical(inode, 0)+i_size_read(inode))
> + blk_to_logical(inode, 0) + i_size_read(inode))
> past_eof = 1;

Issue #1: it does that i_size_read() several times. What happens if the
file grows? Maybe we hold the i_mutex already, although I don't see it.
Regardless, it seems bogus to read the size several times.

Issue #2: "blk_to_logical(inode, 0)"? WTF? Since when has shifting zero
ever resulted in anything interesting or relevant? There's at least two of
those things.

Linus


\
 
 \ /
  Last update: 2010-04-23 17:05    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site