Mount btrfs disk image using discard option

[mikroskeem]

concourse/concourse#5187

[muntac]

Thanks for the PR! I was going over this description of discard support for btrfs on kernel.org. It says:

"-o discard" can have some negative consequences on performance on some SSDs or at least whether it adds worthwhile performance is up for debate depending on who you ask, and makes undeletion/recovery near impossible while being a security problem if you use dm-crypt underneath (see http://asalor.blogspot.com/2011/08/trim-dm-crypt-problems.html ), therefore it is not enabled by default. You are welcome to run your own benchmarks and post them here, with the caveat that they'll be very SSD firmware specific.

Here's one post sharing an experience where the discard option resulted in poorer performance. And this sizable Debian wiki entry on btrfs lists as one of its recommendations

Do not enable mount -o discard, autodefrag, or space_cache=v2.

And a note under SSD optimization:

NOTE: Using "-o discard" with btrfs is generally unsafe.

This post seems to imply that the slowdown occurs from the discard operation taking place synchronously. It seems a -o discard=async was introduced specific tobtrfs in January to reduce latency using asynchronous operations. However, that's on kernel 5.6, which I imagine is far ahead of a lot of distros.

I don't fully understand the performance issues of but given the warnings going around I would only introduce this using a feature flag that's not turned on by default. Going that route would be broader in scope, however.

I'd also be interested to know if you've used this in production and have seen benefits. Same question goes for @pothos who opened #5187.

cc: @vito

[pothos]

Hi @muntac,

I think you confuse the discard option when used for SSD block storage with the discard option used for loop devices which is the case here. For loop devices this option makes the underlying file sparse. It has no relation to whether the filesystem where this loop back file is on is itself placed on a SSD and mounted with the discard option. That's a separate question and not related to this PR.

[mikroskeem]

I'd also be interested to know if you've used this in production and have seen benefits.

@pothos comment concourse/concourse#5187 (comment) shows how much adding discard option helps.

[muntac]

@pothos Thanks for clearing up my misunderstanding!

Recapping my latest understanding here for future reference. Calling a discard on a loop device means holes will be punched into the file to make it sparse. What I was not sure of is if this also triggers the discard behaviour of the underlying block device the loop back file might be on.

From my best guess read of the code, related comments therein, and article from the issue, this is not the case and only the hole punches are propagated down (likely via "blkdev_issue_zeroout()").