2014-07-30 17:18:29

by David Sterba

[permalink] [raw]
Subject: [PATCH 0/6 v5] fiemap: introduce DATA_COMPRESSED and PHYS_LENGTH flags

The original FIEMAP patch did not define the bits, btrfs would like to use a
flag for compressed extents. The PHYS_LENGTH flag emerged during patchset
revisions to keep backward compatibility and flexible fiemap API.

Currently, the 'filefrag' utility has no way to recognize and denote a
compressed extent. As implemented in btrfs right now, the compression step
splits a big extent into smaller chunks and this is reported as a heavily
fragmented file. Adding the flag to filefrag will at least give some
explanation why, this has been confusing users for some time already.

fiemap_fill_next_extent is extended and takes argument to fill the physical
length.

V5:
Physical length is by default undefined. Btrfs use of compressed flag was
enhanced to reflect the fiemap changes. Patches reordered so the generic fiemap
go first.

V4:
The physical length is always set and equal to logical, or different and
then sets the COMPRESSED flag.
fiemap_extent::fe_length renamed to fe_logi_length

V3:
Based on feedback from Andreas, implement #1 from V2, current users of
fiemap_fill_next_extent (fs/, ext4, gfs2, ocfs2, nilfs2, xfs) updated
accordingly, no functional change.

V2:
Based on feedback from Andreas, the fiemap_extent is now able to hold the
physical extent length, to be filled by the filesystem callback.

1) extend fiemap_fill_next_extent to take phys_length and update all
users (ext4, gfs2, ocfs2, nilfs2, xfs)


David Sterba (6):
fiemap: fix comment at EXTENT_DATA_ENCRYPTED
fiemap: add fe_phys_length and EXTENT_PHYS_LENGTH flag
fiemap: add FIEMAP_EXTENT_DATA_COMPRESSED flag
Documentation/fiemap: Document DATA_COMPRESSED and PHYS_LENGTH flags
fiemap: rename fe_length to fe_logi_length
btrfs: set FIEMAP_EXTENT_DATA_COMPRESSED for compressed extents

Documentation/filesystems/fiemap.txt | 24 ++++++++++++++++++++----
fs/btrfs/extent_io.c | 11 ++++++++---
fs/ext4/extents.c | 4 ++--
fs/ext4/inline.c | 2 +-
fs/gfs2/inode.c | 2 +-
fs/ioctl.c | 21 ++++++++++++++-------
fs/nilfs2/inode.c | 8 +++++---
fs/ocfs2/extent_map.c | 4 ++--
fs/xfs/xfs_iops.c | 2 +-
include/linux/fs.h | 2 +-
include/uapi/linux/fiemap.h | 16 +++++++++++++---
11 files changed, 68 insertions(+), 28 deletions(-)

--
1.8.4.5



2014-07-30 17:18:36

by David Sterba

[permalink] [raw]
Subject: [PATCH 2/6] fiemap: add fe_phys_length and EXTENT_PHYS_LENGTH flag

Add a new member to fiemap_extent that represents the physical extent
length. This value is undefined if the flag EXTENT_PHYS_LENGTH is not
set.

No functional change to existing fiemap users.

Signed-off-by: David Sterba <[email protected]>
---
fs/btrfs/extent_io.c | 2 +-
fs/ext4/extents.c | 4 ++--
fs/ext4/inline.c | 2 +-
fs/gfs2/inode.c | 2 +-
fs/ioctl.c | 13 ++++++++-----
fs/nilfs2/inode.c | 8 +++++---
fs/ocfs2/extent_map.c | 4 ++--
fs/xfs/xfs_iops.c | 2 +-
include/linux/fs.h | 2 +-
include/uapi/linux/fiemap.h | 6 +++++-
10 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index a389820d158b..c85da2e54ce7 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4357,7 +4357,7 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
end = 1;
}
ret = fiemap_fill_next_extent(fieinfo, em_start, disko,
- em_len, flags);
+ em_len, 0, flags);
if (ret)
goto out_free;
}
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 4da228a0e6d0..a194024e96a2 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2253,7 +2253,7 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
(__u64)es.es_lblk << blksize_bits,
(__u64)es.es_pblk << blksize_bits,
(__u64)es.es_len << blksize_bits,
- flags);
+ 0, flags);
if (err < 0)
break;
if (err == 1) {
@@ -5125,7 +5125,7 @@ static int ext4_xattr_fiemap(struct inode *inode,

if (physical)
error = fiemap_fill_next_extent(fieinfo, 0, physical,
- length, flags);
+ length, 0, flags);
return (error < 0 ? error : 0);
}

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 645205d8ada6..16f93d8100db 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1825,7 +1825,7 @@ int ext4_inline_data_fiemap(struct inode *inode,

if (physical)
error = fiemap_fill_next_extent(fieinfo, 0, physical,
- length, flags);
+ length, 0, flags);
brelse(iloc.bh);
out:
up_read(&EXT4_I(inode)->xattr_sem);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index e62e59477884..d0d35557ab5f 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1931,7 +1931,7 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
len = size - start;
if (start < size)
ret = fiemap_fill_next_extent(fieinfo, start, phys,
- len, flags);
+ len, 0, flags);
if (ret == 1)
ret = 0;
} else {
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 8ac3fad36192..127551c70b0a 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -70,6 +70,7 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
* @logical: Extent logical start offset, in bytes
* @phys: Extent physical start offset, in bytes
* @len: Extent length, in bytes
+ * @phys_len: Physical extent length in bytes
* @flags: FIEMAP_EXTENT flags that describe this extent
*
* Called from file system ->fiemap callback. Will populate extent
@@ -83,7 +84,7 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
#define SET_NO_UNMOUNTED_IO_FLAGS (FIEMAP_EXTENT_DATA_ENCRYPTED)
#define SET_NOT_ALIGNED_FLAGS (FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
- u64 phys, u64 len, u32 flags)
+ u64 phys, u64 len, u64 phys_len, u32 flags)
{
struct fiemap_extent extent;
struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
@@ -109,6 +110,7 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
extent.fe_physical = phys;
extent.fe_length = len;
extent.fe_flags = flags;
+ extent.fe_phys_length = phys_len;

dest += fieinfo->fi_extents_mapped;
if (copy_to_user(dest, &extent, sizeof(extent)))
@@ -318,10 +320,11 @@ int __generic_block_fiemap(struct inode *inode,
flags = FIEMAP_EXTENT_MERGED|FIEMAP_EXTENT_LAST;
ret = fiemap_fill_next_extent(fieinfo, logical,
phys, size,
- flags);
+ 0, flags);
} else if (size) {
ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, flags);
+ phys, size,
+ 0, flags);
size = 0;
}

@@ -347,7 +350,7 @@ int __generic_block_fiemap(struct inode *inode,
if (start_blk > last_blk && !whole_file) {
ret = fiemap_fill_next_extent(fieinfo, logical,
phys, size,
- flags);
+ 0, flags);
break;
}

@@ -358,7 +361,7 @@ int __generic_block_fiemap(struct inode *inode,
if (size) {
ret = fiemap_fill_next_extent(fieinfo, logical,
phys, size,
- flags);
+ 0, flags);
if (ret)
break;
}
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 6252b173a465..a99e6d093f11 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -1017,7 +1017,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
if (size) {
/* End of the current extent */
ret = fiemap_fill_next_extent(
- fieinfo, logical, phys, size, flags);
+ fieinfo, logical, phys, size, 0,
+ flags);
if (ret)
break;
}
@@ -1067,7 +1068,8 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_LAST;

ret = fiemap_fill_next_extent(
- fieinfo, logical, phys, size, flags);
+ fieinfo, logical, phys, size,
+ 0, flags);
if (ret)
break;
size = 0;
@@ -1083,7 +1085,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
/* Terminate the current extent */
ret = fiemap_fill_next_extent(
fieinfo, logical, phys, size,
- flags);
+ 0, flags);
if (ret || blkoff > end_blkoff)
break;

diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 767370b656ca..cb0276908155 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -736,7 +736,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
id2.i_data.id_data);

ret = fiemap_fill_next_extent(fieinfo, 0, phys, id_count,
- flags);
+ 0, flags);
if (ret < 0)
return ret;
}
@@ -809,7 +809,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;

ret = fiemap_fill_next_extent(fieinfo, virt_bytes, phys_bytes,
- len_bytes, fe_flags);
+ len_bytes, 0, fe_flags);
if (ret)
break;

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 205613a06068..5a7c1f6d1189 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -1030,7 +1030,7 @@ xfs_fiemap_format(
fiemap_flags |= FIEMAP_EXTENT_LAST;

error = fiemap_fill_next_extent(fieinfo, logical, physical,
- length, fiemap_flags);
+ length, 0, fiemap_flags);
if (error > 0) {
error = 0;
*full = 1; /* user array now full */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e11d60cc867b..609e1d72c3e0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1412,7 +1412,7 @@ struct fiemap_extent_info {
fiemap_extent array */
};
int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
- u64 phys, u64 len, u32 flags);
+ u64 phys, u64 len, u64 phys_len, u32 flags);
int fiemap_check_flags(struct fiemap_extent_info *fieinfo, u32 fs_flags);

/*
diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h
index 93abfcd9ac47..ed52d3f7821d 100644
--- a/include/uapi/linux/fiemap.h
+++ b/include/uapi/linux/fiemap.h
@@ -19,7 +19,9 @@ struct fiemap_extent {
__u64 fe_physical; /* physical offset in bytes for the start
* of the extent from the beginning of the disk */
__u64 fe_length; /* length in bytes for this extent */
- __u64 fe_reserved64[2];
+ __u64 fe_phys_length; /* physical length in bytes, may be different from
+ * fe_length, is valid if PHYS_LENGTH flag set */
+ __u64 fe_reserved64;
__u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */
__u32 fe_reserved[3];
};
@@ -50,6 +52,8 @@ struct fiemap {
* Sets EXTENT_UNKNOWN. */
#define FIEMAP_EXTENT_ENCODED 0x00000008 /* Data can not be read
* while fs is unmounted */
+#define FIEMAP_EXTENT_PHYS_LENGTH 0x00000010 /* Physical length of extent
+ * not the same as logical */
#define FIEMAP_EXTENT_DATA_ENCRYPTED 0x00000080 /* Data is encrypted by fs.
* Sets EXTENT_ENCODED */
#define FIEMAP_EXTENT_NOT_ALIGNED 0x00000100 /* Extent offsets may not be
--
1.8.4.5


2014-07-30 20:06:16

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH 2/6] fiemap: add fe_phys_length and EXTENT_PHYS_LENGTH flag


On Jul 30, 2014, at 11:18 AM, David Sterba <[email protected]> wrote:
> Add a new member to fiemap_extent that represents the physical extent
> length. This value is undefined if the flag EXTENT_PHYS_LENGTH is not
> set.

The description here of PHYS_LENGTH makes sense...

The patch description should also mention the name of the new member,
namely "fe_phys_length"

> +#define FIEMAP_EXTENT_PHYS_LENGTH 0x00000010 /* Physical length of extent
> + * not the same as logical */

But the comment doesn't match. This implies that if PHYS_LENGTH is
set, fe_phys_length != fe_logi_length, but I don't think that is
necessarily correct. I think it makes more sense to just set
PHYS_LENGTH when fe_phys_length is valid, and if PHYS_LENGTH is not
set then fe_phys_length aware applications should just use
fe_phys_length = fe_logi_length, and older applications would just
use fe_length for both as they would already today.

Cheers, Andreas






Attachments:
signature.asc (833.00 B)
Message signed with OpenPGP using GPGMail