diff mbox series

[2/2] libext2fs: try to always use PUNCH_HOLE for unix_discard

Message ID 152399275063.4654.12673225054216079609.stgit@magnolia
State New, archived
Headers show
Series e2fprogs: block zero/discard cleanups | expand

Commit Message

Darrick Wong April 17, 2018, 7:19 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Now that block devices support PUNCH_HOLE via fallocate, refactor the
unix_discard code into a helper that will always try to use it.  For
block devices we can fall back to BLKDISCARD, but we prefer to use
fallocate because it will always invalidate the page cache of the zeroed
region.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 lib/ext2fs/unix_io.c |   61 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 25 deletions(-)

Comments

Andreas Dilger April 18, 2018, 3:49 p.m. UTC | #1 
On Apr 17, 2018, at 1:19 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that block devices support PUNCH_HOLE via fallocate, refactor the
> unix_discard code into a helper that will always try to use it.  For
> block devices we can fall back to BLKDISCARD, but we prefer to use
> fallocate because it will always invalidate the page cache of the zeroed
> region.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> lib/ext2fs/unix_io.c |   61 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 36 insertions(+), 25 deletions(-)
> 
> 
> diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
> index 1fb6279..0cf8ca2 100644
> --- a/lib/ext2fs/unix_io.c
> +++ b/lib/ext2fs/unix_io.c
> @@ -1073,6 +1073,38 @@ static errcode_t unix_set_option(io_channel channel, const char *option,
> #define BLKDISCARD		_IO(0x12,119)
> #endif
> 
> +/*
> + * Try a PUNCH_HOLE to unmap blocks, then BLKDISCARD if that doesn't work.
> + * We prefer PUNCH_HOLE because it invalidates the page cache, even on block
> + * devices.
> + */
> +static int __unix_discard(int fd, int is_bdev, off_t offset, off_t len)
> +{
> +#ifdef BLKDISCARD
> +	__u64 range[2];
> +#endif
> +	int ret = -1;
> +
> +#if defined(HAVE_FALLOCATE) && defined(FALLOC_FL_PUNCH_HOLE) && defined(FALLOC_FL_KEEP_SIZE)
> +	ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> +			offset, len);
> +	if (ret == 0)
> +		return 0;
> +#endif
> +#ifdef BLKDISCARD
> +	if (is_bdev) {
> +		range[0] = (__u64)offset;
> +		range[1] = (__u64)len;
> +
> +		ret = ioctl(fd, BLKDISCARD, &range);
> +		if (ret == 0)
> +			return 0;
> +	}
> +#endif
> +	errno = EOPNOTSUPP;
> +	return ret;
> +}
> +
> static errcode_t unix_discard(io_channel channel, unsigned long long block,
> 			      unsigned long long count)
> {
> @@ -1083,31 +1115,10 @@ static errcode_t unix_discard(io_channel channel, unsigned long long block,
> 	data = (struct unix_private_data *) channel->private_data;
> 	EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);
> 
> -	if (channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE) {
> -#ifdef BLKDISCARD
> -		__u64 range[2];
> -
> -		range[0] = (__u64)(block) * channel->block_size + data->offset;
> -		range[1] = (__u64)(count) * channel->block_size;
> -
> -		ret = ioctl(data->dev, BLKDISCARD, &range);
> -#else
> -		goto unimplemented;
> -#endif
> -	} else {
> -#if defined(HAVE_FALLOCATE) && defined(FALLOC_FL_PUNCH_HOLE)
> -		/*
> -		 * If we are not on block device, try to use punch hole
> -		 * to reclaim free space.
> -		 */
> -		ret = fallocate(data->dev,
> -				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> -				(off_t)(block) * channel->block_size + data->offset,
> -				(off_t)(count) * channel->block_size);
> -#else
> -		goto unimplemented;
> -#endif
> -	}
> +	ret = __unix_discard(data->dev,
> +			(channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE),
> +			(off_t)(block) * channel->block_size + data->offset,
> +			(off_t)(count) * channel->block_size);
> 	if (ret < 0) {
> 		if (errno == EOPNOTSUPP)
> 			goto unimplemented;
> 


Cheers, Andreas
Andreas Dilger April 18, 2018, 9:40 p.m. UTC | #2 
On Apr 17, 2018, at 1:19 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that block devices support PUNCH_HOLE via fallocate, refactor the
> unix_discard code into a helper that will always try to use it.  For
> block devices we can fall back to BLKDISCARD, but we prefer to use
> fallocate because it will always invalidate the page cache of the zeroed
> region.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> lib/ext2fs/unix_io.c |   61 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 36 insertions(+), 25 deletions(-)
> 
> 
> diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
> index 1fb6279..0cf8ca2 100644
> --- a/lib/ext2fs/unix_io.c
> +++ b/lib/ext2fs/unix_io.c
> @@ -1073,6 +1073,38 @@ static errcode_t unix_set_option(io_channel channel, const char *option,
> #define BLKDISCARD		_IO(0x12,119)
> #endif
> 
> +/*
> + * Try a PUNCH_HOLE to unmap blocks, then BLKDISCARD if that doesn't work.
> + * We prefer PUNCH_HOLE because it invalidates the page cache, even on block
> + * devices.
> + */
> +static int __unix_discard(int fd, int is_bdev, off_t offset, off_t len)
> +{
> +#ifdef BLKDISCARD
> +	__u64 range[2];
> +#endif
> +	int ret = -1;
> +
> +#if defined(HAVE_FALLOCATE) && defined(FALLOC_FL_PUNCH_HOLE) && defined(FALLOC_FL_KEEP_SIZE)
> +	ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> +			offset, len);
> +	if (ret == 0)
> +		return 0;
> +#endif
> +#ifdef BLKDISCARD
> +	if (is_bdev) {
> +		range[0] = (__u64)offset;
> +		range[1] = (__u64)len;
> +
> +		ret = ioctl(fd, BLKDISCARD, &range);
> +		if (ret == 0)
> +			return 0;
> +	}
> +#endif
> +	errno = EOPNOTSUPP;
> +	return ret;
> +}
> +
> static errcode_t unix_discard(io_channel channel, unsigned long long block,
> 			      unsigned long long count)
> {
> @@ -1083,31 +1115,10 @@ static errcode_t unix_discard(io_channel channel, unsigned long long block,
> 	data = (struct unix_private_data *) channel->private_data;
> 	EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);
> 
> -	if (channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE) {
> -#ifdef BLKDISCARD
> -		__u64 range[2];
> -
> -		range[0] = (__u64)(block) * channel->block_size + data->offset;
> -		range[1] = (__u64)(count) * channel->block_size;
> -
> -		ret = ioctl(data->dev, BLKDISCARD, &range);
> -#else
> -		goto unimplemented;
> -#endif
> -	} else {
> -#if defined(HAVE_FALLOCATE) && defined(FALLOC_FL_PUNCH_HOLE)
> -		/*
> -		 * If we are not on block device, try to use punch hole
> -		 * to reclaim free space.
> -		 */
> -		ret = fallocate(data->dev,
> -				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> -				(off_t)(block) * channel->block_size + data->offset,
> -				(off_t)(count) * channel->block_size);
> -#else
> -		goto unimplemented;
> -#endif
> -	}
> +	ret = __unix_discard(data->dev,
> +			(channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE),
> +			(off_t)(block) * channel->block_size + data->offset,
> +			(off_t)(count) * channel->block_size);
> 	if (ret < 0) {
> 		if (errno == EOPNOTSUPP)
> 			goto unimplemented;
> 


Cheers, Andreas
diff mbox series

Patch

diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 1fb6279..0cf8ca2 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -1073,6 +1073,38 @@  static errcode_t unix_set_option(io_channel channel, const char *option,
 #define BLKDISCARD		_IO(0x12,119)
 #endif
 
+/*
+ * Try a PUNCH_HOLE to unmap blocks, then BLKDISCARD if that doesn't work.
+ * We prefer PUNCH_HOLE because it invalidates the page cache, even on block
+ * devices.
+ */
+static int __unix_discard(int fd, int is_bdev, off_t offset, off_t len)
+{
+#ifdef BLKDISCARD
+	__u64 range[2];
+#endif
+	int ret = -1;
+
+#if defined(HAVE_FALLOCATE) && defined(FALLOC_FL_PUNCH_HOLE) && defined(FALLOC_FL_KEEP_SIZE)
+	ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			offset, len);
+	if (ret == 0)
+		return 0;
+#endif
+#ifdef BLKDISCARD
+	if (is_bdev) {
+		range[0] = (__u64)offset;
+		range[1] = (__u64)len;
+
+		ret = ioctl(fd, BLKDISCARD, &range);
+		if (ret == 0)
+			return 0;
+	}
+#endif
+	errno = EOPNOTSUPP;
+	return ret;
+}
+
 static errcode_t unix_discard(io_channel channel, unsigned long long block,
 			      unsigned long long count)
 {
@@ -1083,31 +1115,10 @@  static errcode_t unix_discard(io_channel channel, unsigned long long block,
 	data = (struct unix_private_data *) channel->private_data;
 	EXT2_CHECK_MAGIC(data, EXT2_ET_MAGIC_UNIX_IO_CHANNEL);
 
-	if (channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE) {
-#ifdef BLKDISCARD
-		__u64 range[2];
-
-		range[0] = (__u64)(block) * channel->block_size + data->offset;
-		range[1] = (__u64)(count) * channel->block_size;
-
-		ret = ioctl(data->dev, BLKDISCARD, &range);
-#else
-		goto unimplemented;
-#endif
-	} else {
-#if defined(HAVE_FALLOCATE) && defined(FALLOC_FL_PUNCH_HOLE)
-		/*
-		 * If we are not on block device, try to use punch hole
-		 * to reclaim free space.
-		 */
-		ret = fallocate(data->dev,
-				FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-				(off_t)(block) * channel->block_size + data->offset,
-				(off_t)(count) * channel->block_size);
-#else
-		goto unimplemented;
-#endif
-	}
+	ret = __unix_discard(data->dev,
+			(channel->flags & CHANNEL_FLAGS_BLOCK_DEVICE),
+			(off_t)(block) * channel->block_size + data->offset,
+			(off_t)(count) * channel->block_size);
 	if (ret < 0) {
 		if (errno == EOPNOTSUPP)
 			goto unimplemented;