more iomap inline enablement

more iomap inline enablement

[Christoph Hellwig]

Hi Darrick,

below are a few more iomap patches to enable inline data for the
iomap-4.19-merge branch.

[PATCH 1/3] iomap: refactor iomap_dio_actor

[Christoph Hellwig]

Split the function up into two helpers for the bio based I/O and hole
case, and a small helper to call the two.  This separates the code a
little better in preparation for supporting I/O to inline data.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/iomap.c | 88 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 52 insertions(+), 36 deletions(-)

diff --git a/fs/iomap.c b/fs/iomap.c
index 2ebff76039b5..4d8ff0f5ecc9 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1327,10 +1327,9 @@ iomap_dio_zero(struct iomap_dio *dio, struct iomap *iomap, loff_t pos,
 }
 
 static loff_t
-iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
-		void *data, struct iomap *iomap)
+iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
+		struct iomap_dio *dio, struct iomap *iomap)
 {
-	struct iomap_dio *dio = data;
 	unsigned int blkbits = blksize_bits(bdev_logical_block_size(iomap->bdev));
 	unsigned int fs_block_size = i_blocksize(inode), pad;
 	unsigned int align = iov_iter_alignment(dio->submit.iter);
@@ -1344,41 +1343,27 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
 	if ((pos | length | align) & ((1 << blkbits) - 1))
 		return -EINVAL;
 
-	switch (iomap->type) {
-	case IOMAP_HOLE:
-		if (WARN_ON_ONCE(dio->flags & IOMAP_DIO_WRITE))
-			return -EIO;
-		/*FALLTHRU*/
-	case IOMAP_UNWRITTEN:
-		if (!(dio->flags & IOMAP_DIO_WRITE)) {
-			length = iov_iter_zero(length, dio->submit.iter);
-			dio->size += length;
-			return length;
-		}
+	if (iomap->type == IOMAP_UNWRITTEN) {
 		dio->flags |= IOMAP_DIO_UNWRITTEN;
 		need_zeroout = true;
-		break;
-	case IOMAP_MAPPED:
-		if (iomap->flags & IOMAP_F_SHARED)
-			dio->flags |= IOMAP_DIO_COW;
-		if (iomap->flags & IOMAP_F_NEW) {
-			need_zeroout = true;
-		} else {
-			/*
-			 * Use a FUA write if we need datasync semantics, this
-			 * is a pure data IO that doesn't require any metadata
-			 * updates and the underlying device supports FUA. This
-			 * allows us to avoid cache flushes on IO completion.
-			 */
-			if (!(iomap->flags & (IOMAP_F_SHARED|IOMAP_F_DIRTY)) &&
-			    (dio->flags & IOMAP_DIO_WRITE_FUA) &&
-			    blk_queue_fua(bdev_get_queue(iomap->bdev)))
-				use_fua = true;
-		}
-		break;
-	default:
-		WARN_ON_ONCE(1);
-		return -EIO;
+	}
+
+	if (iomap->flags & IOMAP_F_SHARED)
+		dio->flags |= IOMAP_DIO_COW;
+
+	if (iomap->flags & IOMAP_F_NEW) {
+		need_zeroout = true;
+	} else {
+		/*
+		 * Use a FUA write if we need datasync semantics, this
+		 * is a pure data IO that doesn't require any metadata
+		 * updates and the underlying device supports FUA. This
+		 * allows us to avoid cache flushes on IO completion.
+		 */
+		if (!(iomap->flags & (IOMAP_F_SHARED|IOMAP_F_DIRTY)) &&
+		    (dio->flags & IOMAP_DIO_WRITE_FUA) &&
+		    blk_queue_fua(bdev_get_queue(iomap->bdev)))
+			use_fua = true;
 	}
 
 	/*
@@ -1457,6 +1442,37 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
 	return copied;
 }
 
+static loff_t
+iomap_dio_hole_actor(loff_t length, struct iomap_dio *dio)
+{
+	length = iov_iter_zero(length, dio->submit.iter);
+	dio->size += length;
+	return length;
+}
+
+static loff_t
+iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
+		void *data, struct iomap *iomap)
+{
+	struct iomap_dio *dio = data;
+
+	switch (iomap->type) {
+	case IOMAP_HOLE:
+		if (WARN_ON_ONCE(dio->flags & IOMAP_DIO_WRITE))
+			return -EIO;
+		return iomap_dio_hole_actor(length, dio);
+	case IOMAP_UNWRITTEN:
+		if (!(dio->flags & IOMAP_DIO_WRITE))
+			return iomap_dio_hole_actor(length, dio);
+		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
+	case IOMAP_MAPPED:
+		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
+	default:
+		WARN_ON_ONCE(1);
+		return -EIO;
+	}
+}
+
 /*
  * iomap_dio_rw() always completes O_[D]SYNC writes regardless of whether the IO
  * is being issued as AIO or not.  This allows us to optimise pure data writes
-- 
2.18.0

[PATCH 2/3] iomap: support direct I/O to inline data

[Christoph Hellwig]

From: Andreas Gruenbacher <agruenba@redhat.com>

Add support for reading from and writing to inline data to iomap_dio_rw.
This saves filesystems from having to implement fallback code for this
case.

The inline data is actually cached in the inode, so the I/O is only
direct in the sense that it doesn't go through the page cache.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/iomap.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/fs/iomap.c b/fs/iomap.c
index 4d8ff0f5ecc9..98a1fdd5c091 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -1450,6 +1450,33 @@ iomap_dio_hole_actor(loff_t length, struct iomap_dio *dio)
 	return length;
 }
 
+static loff_t
+iomap_dio_inline_actor(struct inode *inode, loff_t pos, loff_t length,
+		struct iomap_dio *dio, struct iomap *iomap)
+{
+	struct iov_iter *iter = dio->submit.iter;
+	size_t copied;
+
+	BUG_ON(pos + length > PAGE_SIZE - offset_in_page(iomap->inline_data));
+
+	if (dio->flags & IOMAP_DIO_WRITE) {
+		loff_t size = inode->i_size;
+
+		if (pos > size)
+			memset(iomap->inline_data + size, 0, pos - size);
+		copied = copy_from_iter(iomap->inline_data + pos, length, iter);
+		if (copied) {
+			if (pos + copied > size)
+				i_size_write(inode, pos + copied);
+			mark_inode_dirty(inode);
+		}
+	} else {
+		copied = copy_to_iter(iomap->inline_data + pos, length, iter);
+	}
+	dio->size += copied;
+	return copied;
+}
+
 static loff_t
 iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
 		void *data, struct iomap *iomap)
@@ -1467,6 +1494,8 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
 		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
 	case IOMAP_MAPPED:
 		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
+	case IOMAP_INLINE:
+		return iomap_dio_inline_actor(inode, pos, length, dio, iomap);
 	default:
 		WARN_ON_ONCE(1);
 		return -EIO;
-- 
2.18.0

[PATCH 3/3] iomap: add inline data support to iomap_readpage_actor

[Christoph Hellwig]

From: Andreas Gruenbacher <agruenba@redhat.com>

Just copy the inline data into the page using the existing helper.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/iomap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/iomap.c b/fs/iomap.c
index 98a1fdd5c091..13cdcf33e6c0 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -155,6 +155,12 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 	bool is_contig = false;
 	sector_t sector;
 
+	if (iomap->type == IOMAP_INLINE) {
+		WARN_ON_ONCE(poff);
+		iomap_read_inline_data(inode, page, iomap);
+		return PAGE_SIZE;
+	}
+
 	/* we don't support blocksize < PAGE_SIZE quite yet. */
 	WARN_ON_ONCE(pos != page_offset(page));
 	WARN_ON_ONCE(plen != PAGE_SIZE);
-- 
2.18.0

Re: [PATCH 3/3] iomap: add inline data support to iomap_readpage_actor

[Darrick J. Wong]

On Mon, Jul 02, 2018 at 08:54:58AM -0600, Christoph Hellwig wrote:
> From: Andreas Gruenbacher <agruenba@redhat.com>
> 
> Just copy the inline data into the page using the existing helper.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/iomap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 98a1fdd5c091..13cdcf33e6c0 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -155,6 +155,12 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  	bool is_contig = false;
>  	sector_t sector;
>  
> +	if (iomap->type == IOMAP_INLINE) {
> +		WARN_ON_ONCE(poff);
> +		iomap_read_inline_data(inode, page, iomap);
> +		return PAGE_SIZE;
> +	}
> +
>  	/* we don't support blocksize < PAGE_SIZE quite yet. */
>  	WARN_ON_ONCE(pos != page_offset(page));
>  	WARN_ON_ONCE(plen != PAGE_SIZE);
> -- 
> 2.18.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

Re: [PATCH 2/3] iomap: support direct I/O to inline data

[Darrick J. Wong]

On Mon, Jul 02, 2018 at 08:54:57AM -0600, Christoph Hellwig wrote:
> From: Andreas Gruenbacher <agruenba@redhat.com>
> 
> Add support for reading from and writing to inline data to iomap_dio_rw.
> This saves filesystems from having to implement fallback code for this
> case.
> 
> The inline data is actually cached in the inode, so the I/O is only
> direct in the sense that it doesn't go through the page cache.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/iomap.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 4d8ff0f5ecc9..98a1fdd5c091 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1450,6 +1450,33 @@ iomap_dio_hole_actor(loff_t length, struct iomap_dio *dio)
>  	return length;
>  }
>  
> +static loff_t
> +iomap_dio_inline_actor(struct inode *inode, loff_t pos, loff_t length,
> +		struct iomap_dio *dio, struct iomap *iomap)
> +{
> +	struct iov_iter *iter = dio->submit.iter;
> +	size_t copied;
> +
> +	BUG_ON(pos + length > PAGE_SIZE - offset_in_page(iomap->inline_data));
> +
> +	if (dio->flags & IOMAP_DIO_WRITE) {
> +		loff_t size = inode->i_size;
> +
> +		if (pos > size)
> +			memset(iomap->inline_data + size, 0, pos - size);
> +		copied = copy_from_iter(iomap->inline_data + pos, length, iter);
> +		if (copied) {
> +			if (pos + copied > size)
> +				i_size_write(inode, pos + copied);
> +			mark_inode_dirty(inode);
> +		}
> +	} else {
> +		copied = copy_to_iter(iomap->inline_data + pos, length, iter);
> +	}
> +	dio->size += copied;
> +	return copied;
> +}
> +
>  static loff_t
>  iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
>  		void *data, struct iomap *iomap)
> @@ -1467,6 +1494,8 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
>  		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
>  	case IOMAP_MAPPED:
>  		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
> +	case IOMAP_INLINE:
> +		return iomap_dio_inline_actor(inode, pos, length, dio, iomap);
>  	default:
>  		WARN_ON_ONCE(1);
>  		return -EIO;
> -- 
> 2.18.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

Re: [PATCH 1/3] iomap: refactor iomap_dio_actor

[Darrick J. Wong]

On Mon, Jul 02, 2018 at 08:54:56AM -0600, Christoph Hellwig wrote:
> Split the function up into two helpers for the bio based I/O and hole
> case, and a small helper to call the two.  This separates the code a
> little better in preparation for supporting I/O to inline data.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/iomap.c | 88 ++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 52 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 2ebff76039b5..4d8ff0f5ecc9 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -1327,10 +1327,9 @@ iomap_dio_zero(struct iomap_dio *dio, struct iomap *iomap, loff_t pos,
>  }
>  
>  static loff_t
> -iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
> -		void *data, struct iomap *iomap)
> +iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
> +		struct iomap_dio *dio, struct iomap *iomap)
>  {
> -	struct iomap_dio *dio = data;
>  	unsigned int blkbits = blksize_bits(bdev_logical_block_size(iomap->bdev));
>  	unsigned int fs_block_size = i_blocksize(inode), pad;
>  	unsigned int align = iov_iter_alignment(dio->submit.iter);
> @@ -1344,41 +1343,27 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
>  	if ((pos | length | align) & ((1 << blkbits) - 1))
>  		return -EINVAL;
>  
> -	switch (iomap->type) {
> -	case IOMAP_HOLE:
> -		if (WARN_ON_ONCE(dio->flags & IOMAP_DIO_WRITE))
> -			return -EIO;
> -		/*FALLTHRU*/
> -	case IOMAP_UNWRITTEN:
> -		if (!(dio->flags & IOMAP_DIO_WRITE)) {
> -			length = iov_iter_zero(length, dio->submit.iter);
> -			dio->size += length;
> -			return length;
> -		}
> +	if (iomap->type == IOMAP_UNWRITTEN) {
>  		dio->flags |= IOMAP_DIO_UNWRITTEN;
>  		need_zeroout = true;
> -		break;
> -	case IOMAP_MAPPED:
> -		if (iomap->flags & IOMAP_F_SHARED)
> -			dio->flags |= IOMAP_DIO_COW;
> -		if (iomap->flags & IOMAP_F_NEW) {
> -			need_zeroout = true;
> -		} else {
> -			/*
> -			 * Use a FUA write if we need datasync semantics, this
> -			 * is a pure data IO that doesn't require any metadata
> -			 * updates and the underlying device supports FUA. This
> -			 * allows us to avoid cache flushes on IO completion.
> -			 */
> -			if (!(iomap->flags & (IOMAP_F_SHARED|IOMAP_F_DIRTY)) &&
> -			    (dio->flags & IOMAP_DIO_WRITE_FUA) &&
> -			    blk_queue_fua(bdev_get_queue(iomap->bdev)))
> -				use_fua = true;
> -		}
> -		break;
> -	default:
> -		WARN_ON_ONCE(1);
> -		return -EIO;
> +	}
> +
> +	if (iomap->flags & IOMAP_F_SHARED)
> +		dio->flags |= IOMAP_DIO_COW;
> +
> +	if (iomap->flags & IOMAP_F_NEW) {
> +		need_zeroout = true;
> +	} else {
> +		/*
> +		 * Use a FUA write if we need datasync semantics, this
> +		 * is a pure data IO that doesn't require any metadata
> +		 * updates and the underlying device supports FUA. This
> +		 * allows us to avoid cache flushes on IO completion.
> +		 */
> +		if (!(iomap->flags & (IOMAP_F_SHARED|IOMAP_F_DIRTY)) &&
> +		    (dio->flags & IOMAP_DIO_WRITE_FUA) &&
> +		    blk_queue_fua(bdev_get_queue(iomap->bdev)))
> +			use_fua = true;
>  	}
>  
>  	/*
> @@ -1457,6 +1442,37 @@ iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
>  	return copied;
>  }
>  
> +static loff_t
> +iomap_dio_hole_actor(loff_t length, struct iomap_dio *dio)
> +{
> +	length = iov_iter_zero(length, dio->submit.iter);
> +	dio->size += length;
> +	return length;
> +}
> +
> +static loff_t
> +iomap_dio_actor(struct inode *inode, loff_t pos, loff_t length,
> +		void *data, struct iomap *iomap)
> +{
> +	struct iomap_dio *dio = data;
> +
> +	switch (iomap->type) {
> +	case IOMAP_HOLE:
> +		if (WARN_ON_ONCE(dio->flags & IOMAP_DIO_WRITE))
> +			return -EIO;
> +		return iomap_dio_hole_actor(length, dio);
> +	case IOMAP_UNWRITTEN:
> +		if (!(dio->flags & IOMAP_DIO_WRITE))
> +			return iomap_dio_hole_actor(length, dio);
> +		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
> +	case IOMAP_MAPPED:
> +		return iomap_dio_bio_actor(inode, pos, length, dio, iomap);
> +	default:
> +		WARN_ON_ONCE(1);
> +		return -EIO;
> +	}
> +}
> +
>  /*
>   * iomap_dio_rw() always completes O_[D]SYNC writes regardless of whether the IO
>   * is being issued as AIO or not.  This allows us to optimise pure data writes
> -- 
> 2.18.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