On 10/16/19 2:16 PM, Jan Kara wrote:
On Tue 20-08-19 18:36:34, Ritesh Harjani wrote:
This moves ext4_fiemap to use iomap infrastructure.
Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx>
Thanks for the patch. I like how it removes lots of code :) The patch looks
good to me, just two small comments below. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Sure, thanks. It's been some quite some time.
Let me rebase these patches on latest upstream kernel.
+static int ext4_xattr_iomap_fiemap(struct inode *inode, struct iomap *iomap)
{
- __u64 physical = 0;
- __u64 length;
- __u32 flags = FIEMAP_EXTENT_LAST;
+ u64 physical = 0;
+ u64 length;
int blockbits = inode->i_sb->s_blocksize_bits;
- int error = 0;
+ int ret = 0;
/* in-inode? */
if (ext4_test_inode_state(inode, EXT4_STATE_XATTR)) {
struct ext4_iloc iloc;
- int offset; /* offset of xattr in inode */
+ int offset; /* offset of xattr in inode */
- error = ext4_get_inode_loc(inode, &iloc);
- if (error)
- return error;
+ ret = ext4_get_inode_loc(inode, &iloc);
+ if (ret)
+ goto out;
physical = (__u64)iloc.bh->b_blocknr << blockbits;
offset = EXT4_GOOD_OLD_INODE_SIZE +
EXT4_I(inode)->i_extra_isize;
Hum, I see you've just copied this from the old code but this actually
won't give full information for FIEMAP of inode with extended attribute
inodes.
Could you please elaborate on above? I am anyway taking a look at it in
parallel. I can provide that as a separate patch, if required.
Not something you need to fix for your patch but I wanted to
mention this so that it doesn't get lost.
Sure :)
physical += offset;
length = EXT4_SB(inode->i_sb)->s_inode_size - offset;
- flags |= FIEMAP_EXTENT_DATA_INLINE;
brelse(iloc.bh);
} else { /* external block */
physical = (__u64)EXT4_I(inode)->i_file_acl << blockbits;
length = inode->i_sb->s_blocksize;
}
- if (physical)
- error = fiemap_fill_next_extent(fieinfo, 0, physical,
- length, flags);
- return (error < 0 ? error : 0);
+ iomap->addr = physical;
+ iomap->offset = 0;
+ iomap->length = length;
+ iomap->type = IOMAP_INLINE;
+ iomap->flags = 0;
+out:
+ return ret;
}
...
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d6a34214e9df..92705e99e07c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3581,15 +3581,24 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
iomap->type = delalloc ? IOMAP_DELALLOC : IOMAP_HOLE;
iomap->addr = IOMAP_NULL_ADDR;
} else {
- if (map.m_flags & EXT4_MAP_MAPPED) {
- iomap->type = IOMAP_MAPPED;
- } else if (map.m_flags & EXT4_MAP_UNWRITTEN) {
+ /*
+ * There can be a case where map.m_flags may
+ * have both EXT4_MAP_UNWRITTEN & EXT4_MAP_MERGED
+ * set. This is when we do fallocate first and
+ * then try to write to that area, then it may have
+ * both flags set. So check for unwritten first.
+ */
+ if (map.m_flags & EXT4_MAP_UNWRITTEN) {
iomap->type = IOMAP_UNWRITTEN;
+ } else if (map.m_flags & EXT4_MAP_MAPPED) {
+ iomap->type = IOMAP_MAPPED;
This should be already part of Matthew's series so once you rebase on top
of it, you can just drop this hunk.
Sure, will do.
-riteshh
