* Re: [PATCH 09/34] iomap: inline data should be an iomap type, not a flag
[not found] ` <20180523144357.18985-10-hch@lst.de>
@ 2018-05-30 5:49 ` Darrick J. Wong
0 siblings, 0 replies; only message in thread
From: Darrick J. Wong @ 2018-05-30 5:49 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-xfs, linux-fsdevel, linux-ext4, cluster-devel, linux-mm
On Wed, May 23, 2018 at 04:43:32PM +0200, Christoph Hellwig wrote:
> Inline data is fundamentally different from our normal mapped case in that
> it doesn't even have a block address. So instead of having a flag for it
> it should be an entirely separate iomap range type.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok to me, anyone from gfs2/ext4 want to ack this?
Let's cc those lists and see what happens...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/ext4/inline.c | 4 ++--
> fs/gfs2/bmap.c | 3 +--
> fs/iomap.c | 21 ++++++++++++---------
> include/linux/iomap.h | 2 +-
> 4 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 70cf4c7b268a..e1f00891ef95 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1835,8 +1835,8 @@ int ext4_inline_data_iomap(struct inode *inode, struct iomap *iomap)
> iomap->offset = 0;
> iomap->length = min_t(loff_t, ext4_get_inline_size(inode),
> i_size_read(inode));
> - iomap->type = 0;
> - iomap->flags = IOMAP_F_DATA_INLINE;
> + iomap->type = IOMAP_INLINE;
> + iomap->flags = 0;
>
> out:
> up_read(&EXT4_I(inode)->xattr_sem);
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 278ed0869c3c..cbeedd3cfb36 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -680,8 +680,7 @@ static void gfs2_stuffed_iomap(struct inode *inode, struct iomap *iomap)
> sizeof(struct gfs2_dinode);
> iomap->offset = 0;
> iomap->length = i_size_read(inode);
> - iomap->type = IOMAP_MAPPED;
> - iomap->flags = IOMAP_F_DATA_INLINE;
> + iomap->type = IOMAP_INLINE;
> }
>
> /**
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 0900da23172c..f52209a2c270 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -503,10 +503,13 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
> case IOMAP_DELALLOC:
> flags |= FIEMAP_EXTENT_DELALLOC | FIEMAP_EXTENT_UNKNOWN;
> break;
> + case IOMAP_MAPPED:
> + break;
> case IOMAP_UNWRITTEN:
> flags |= FIEMAP_EXTENT_UNWRITTEN;
> break;
> - case IOMAP_MAPPED:
> + case IOMAP_INLINE:
> + flags |= FIEMAP_EXTENT_DATA_INLINE;
> break;
> }
>
> @@ -514,8 +517,6 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
> flags |= FIEMAP_EXTENT_MERGED;
> if (iomap->flags & IOMAP_F_SHARED)
> flags |= FIEMAP_EXTENT_SHARED;
> - if (iomap->flags & IOMAP_F_DATA_INLINE)
> - flags |= FIEMAP_EXTENT_DATA_INLINE;
>
> return fiemap_fill_next_extent(fi, iomap->offset,
> iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
> @@ -1318,14 +1319,16 @@ static loff_t iomap_swapfile_activate_actor(struct inode *inode, loff_t pos,
> struct iomap_swapfile_info *isi = data;
> int error;
>
> - /* No inline data. */
> - if (iomap->flags & IOMAP_F_DATA_INLINE) {
> + switch (iomap->type) {
> + case IOMAP_MAPPED:
> + case IOMAP_UNWRITTEN:
> + /* Only real or unwritten extents. */
> + break;
> + case IOMAP_INLINE:
> + /* No inline data. */
> pr_err("swapon: file is inline\n");
> return -EINVAL;
> - }
> -
> - /* Only real or unwritten extents. */
> - if (iomap->type != IOMAP_MAPPED && iomap->type != IOMAP_UNWRITTEN) {
> + default:
> pr_err("swapon: file has unallocated extents\n");
> return -EINVAL;
> }
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 4bd87294219a..8f7095fc514e 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -18,6 +18,7 @@ struct vm_fault;
> #define IOMAP_DELALLOC 0x02 /* delayed allocation blocks */
> #define IOMAP_MAPPED 0x03 /* blocks allocated at @addr */
> #define IOMAP_UNWRITTEN 0x04 /* blocks allocated at @addr in unwritten state */
> +#define IOMAP_INLINE 0x05 /* data inline in the inode */
>
> /*
> * Flags for all iomap mappings:
> @@ -34,7 +35,6 @@ struct vm_fault;
> */
> #define IOMAP_F_MERGED 0x10 /* contains multiple blocks/extents */
> #define IOMAP_F_SHARED 0x20 /* block shared with another file */
> -#define IOMAP_F_DATA_INLINE 0x40 /* data inline in the inode */
>
> /*
> * Magic value for addr:
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] only message in thread