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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux