linux-ext4 - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190215095007.jx5ol3ss7uhegz3z@localhost.localdomain>
Date:   Fri, 15 Feb 2019 10:50:07 +0100
From:   Lukas Czerner <lczerner@...hat.com>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     "Darrick J. Wong" <darrick.wong@...cle.com>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] libext2fs: revert "try to always use PUNCH_HOLE for
 unix_discard"

On Thu, Feb 14, 2019 at 04:04:48PM -0500, Theodore Y. Ts'o wrote:
> On Mon, Jan 14, 2019 at 03:37:08PM -0800, Darrick J. Wong wrote:
> > 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>
> 
> Thanks, applied.
> 
> 					- Ted

I just noticed this patch, sorry. I think we can still use fallocate,
but we need to set the right flags to make sure it uses discard instead
of zeroout. See fs/block_dev.c

	switch (mode) {
	case FALLOC_FL_ZERO_RANGE:
	case FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE:
		error = blkdev_issue_zeroout(bdev, start >> 9, len >> 9,
					    GFP_KERNEL, BLKDEV_ZERO_NOUNMAP);
		break;
	case FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE:
		error = blkdev_issue_zeroout(bdev, start >> 9, len >> 9,
					     GFP_KERNEL, BLKDEV_ZERO_NOFALLBACK);
		break;
	case FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_NO_HIDE_STALE:
		error = blkdev_issue_discard(bdev, start >> 9, len >> 9,
					     GFP_KERNEL, 0);
		break;
	default:
		return -EOPNOTSUPP;
	}

So if we want a discard (meaning we want to unallocate the blocks
without necessarily making sure we can't read stale data from it) we
have to use FALLOC_FL_NO_HIDE_STALE.

So the flags would be FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_NO_HIDE_STALE

Ted, Darrick what do you think ? Can we keep the
bcca9876a3428c10417c660b78933e6e70e8a5f5 commit and just change the
flags ?

-Lukas