linux-ext4 - [PATCH] libext2fs: revert "try to always use PUNCH_HOLE for unix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190114233708.GA20465@magnolia>
Date:   Mon, 14 Jan 2019 15:37:08 -0800
From:   "Darrick J. Wong" <darrick.wong@...cle.com>
To:     "Theodore Ts'o" <tytso@....edu>
Cc:     linux-ext4 <linux-ext4@...r.kernel.org>
Subject: [PATCH] libext2fs: revert "try to always use PUNCH_HOLE for
 unix_discard"

From: Darrick J. Wong <darrick.wong@...cle.com>

Revert bcca9876a3428c10417c660b78933e6e70e8a5f5, because
fallocate(PUNCH_HOLE) on block devices was changed to use zeroout
instead of discard shortly after block device fallocate was merged.
zeroout isn't necessarily a "drop storage" operation like discard is,
so we prefer to use that on block devices.

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

diff --git a/lib/ext2fs/unix_io.c b/lib/ext2fs/unix_io.c
index 16e2052cd..74fc8a75d 100644
--- a/lib/ext2fs/unix_io.c
+++ b/lib/ext2fs/unix_io.c
@@ -1081,38 +1081,6 @@ 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)
 {
@@ -1123,10 +1091,31 @@ 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);
 
-	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 (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
+	}
 	if (ret < 0) {
 		if (errno == EOPNOTSUPP)
 			goto unimplemented;