[lkml]   [2010]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    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

    > + 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.


     \ /
      Last update: 2010-04-23 17:05    [W:0.023 / U:54.672 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site