linux-ext4 - Re: [PATCH 2/2] libext2fs: 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-prev] [day] [month] [year] [list]

Message-Id: <6324DF3C-707B-4A8E-B621-12A9DC7FC53A@dilger.ca>
Date:   Wed, 18 Apr 2018 15:40:14 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     "Darrick J. Wong" <darrick.wong@...cle.com>
Cc:     tytso@....edu, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 2/2] libext2fs: try to always use PUNCH_HOLE for
 unix_discard

On Apr 17, 2018, at 1:19 PM, Darrick J. Wong <darrick.wong@...cle.com> wrote:
> 
> From: Darrick J. Wong <darrick.wong@...cle.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@...cle.com>

Reviewed-by: Andreas Dilger <adilger@...ger.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






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)