On Fri, Jul 25, 2014 at 4:04 AM, Andreas Dilger adil...@dilger.ca wrote:
On Jul 24, 2014, at 1:22 PM, David Sterba dste...@suse.cz wrote:
On Thu, Jul 17, 2014 at 12:07:57AM -0600, Andreas Dilger wrote:
any progress on this patch series?
I'm sorry I got distracted at the end of year and did not finish the
series.
I never saw an updated version of this patch series after the last round of
reviews, but it would be great to move it forward. I have filefrag patches
in my e2fsprogs tree waiting for an updated version of your patch.
I recall the main changes were:
- add FIEMAP_EXTENT_PHYS_LENGTH flag to indicate if fe_phys_length was valid
fe_phys_length will be always valid, so other the flags are set only if it's
not equal to the logical length.
- rename fe_length to fe_logi_length and #define fe_length fe_logi_length
- always fill in fe_phys_length (= fe_logi_length for uncompressed files)
and set FIEMAP_EXTENT_PHYS_LENGTH whether the extent is compressed or not
This is my understanding and contradicts the first point.
I think Dave Chinner's former point was that having fe_phys_length validity
depend on FIEMAP_EXTENT_DATA_COMPRESSED is a non-intuitive interface. It is
not true that fe_phys_length would always be valid, since that is not the
case for older kernels that currently always set this field to 0, so they
need some flag to indicate if fe_phys_length is valid. Alternately,
userspace could do:
if (ext-fe_phys_length == 0)
ext-fe_phys_length = ext-fe_logi_length;
but that pre-supposes that fe_phys_length == 0 is never a valid value when
fe_logi_length is non-zero, and this might introduce errors in some cases.
I could imagine that some compression methods might not allocate any space
at all if it was all zeroes, and just store a bit in the blockpointer or
extent, so having a separate FIEMAP_EXTENT_PHYS_LENGTH is probably safer
in the long run.
zfs is an example of this.
That opens up the question of whether a written zero
filled space that gets compressed away is different from a hole, but I'd
prefer to just return whatever the file mapping is than interpret it.
Cheers, Andreas
- add WARN_ONCE() in fiemap_fill_next_extent() as described below
I don't know if there was any clear statement about whether there should be
separate FIEMAP_EXTENT_PHYS_LENGTH and FIEMAP_EXTENT_DATA_COMPRESSED flags,
or if the latter should be implicit? Probably makes sense to have separate
flags. It should be fine to use:
#define FIEMAP_EXTENT_PHYS_LENGTH0x0010
since this flag was never used.
I've kept only FIEMAP_EXTENT_DATA_COMPRESSED, I don't see a need for
FIEMAP_EXTENT_PHYS_LENGTH and this would be yet another flag because the
FIEMAP_EXTENT_DATA_ENCODED is also implied.
I'll send V4, we can discuss the PHYS_LENGTH flag then.
Cheers, Andreas
Regards,
Rohan
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
