Re: [PATCH 0/4] Fiemap, an extent mapping ioctl - round 2

[jim owens <jowens@hp.com>]

I'm back from vacation and ready to cause fiemap() trouble.

Dave Kleikamp wrote:
> On Thu, 2008-06-26 at 09:15 -0500, Eric Sandeen wrote:
> 
>>>SYNC really doesn't look like it belongs, and it's only there so that
>>>the new ioctl acts like the xfs ioctl.
>>
>>I disagree, while it may have been inspired by the xfs behavior, it's
>>not at all xfs specific.
>>
>>If a filesystem implements delalloc, you may want to know which ranges
>>are still delalloc in the fiemap output, or you may want to put them on
>>disk and know the actual physical location.  And if you want a snapshot
>>of an actual, consistent layout of the file at a point in time, then you
>>need an atomic sync+map - for any filesystem.
> 
> This makes sense.  In fact, I could see always doing the sync if there
> are delalloc blocks to ensure that the location of the blocks will
> always be returned.
> 
  > I guess I was put off by Andreas' response that FIEMAP_FLAG_SYNC is
> there because xfsbmap had it "isn't harmful either".  This seemed a bit
> weak, but I see that there is a better justification than just that.

I say IT IS HARMFUL to have the FIEMAP_FLAG_SYNC.

The email trail points out how this so-called atomic sync+map
will lead programmers to write bad code because it leads them
to think there is some valuable guarantee of consistency by
using the SYNC flag.  This is not true.

The fiemap by itself is equivalent in all cases to reading
multiple disk blocks, while someone else is writing some
random subset of the same blocks.  You have data, but it is
not a clean "before" or "after" picture.

The only way to get a true useful snapshot is to have a
set of commands doing:
    freeze_metadata()
    read_metadata()
    ... userspace operate on metadata ...
    unfreeze_metadata()

If you are going to define fiemap to have an internal
freeze_metadata(), then I say that is even MORE HARMFUL
because it makes every (de)allocate/(de)compress/move
code path take a giant lock just so fiemap can get a
static picture that encompasses all in-range extents.

And that static picture can be invalid the moment the
giant lock is released.

jim