New issue
Advanced search Search tips

Issue 896314 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 19
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

crosvm discard is slow on host kernels without FALLOC_FL_PUNCH_HOLE support

Project Member Reported by dverkamp@chromium.org, Oct 17

Issue description

The 3.18 branch of the Chrome OS kernel has disabled FALLOC_FL_PUNCH_HOLE for ext4, so translating discard from the block device into fallocate() fails and we fall back to writing buffers of zeroes.  This is very slow for large discards (e.g. the entire disk).  It's also not necessary in the case of a virtio block Discard command, which does not require that the discarded data return zeroes after the command completes (the spec says "The driver MUST NOT assume anything about the data returned by read requests after a range of sectors has been discarded").

We should be able to split up the implementation of Write Zeroes (which must read back as zeroes) and Discard so that Discard can just try the fallocate() and continue without reporting an error if it fails.  This should restore reasonable performance on 3.18 kernels, although it will not actually free up space on the host filesystem on those systems.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/crosvm/+/de198cc9be3f3741a4c9cc6f97275cc4ec7f7505

commit de198cc9be3f3741a4c9cc6f97275cc4ec7f7505
Author: Daniel Verkamp <dverkamp@chromium.org>
Date: Fri Oct 19 19:19:41 2018

devices: virtio: block: ignore Discard failures

Our branch of the 3.18 kernel has FALLOC_FL_PUNCH_HOLE disabled for the
ext4 filesystem, which means that systems running that kernel always
take the fallback path of writing buffers full of zeroes.  This is not
necessary for the Discard command, since it is just a hint and is not
required to actually zero the blocks.

Split the WriteZeroes trait up into a new PunchHole trait, which
corresponds to fallocate() with FALLOC_FL_PUNCH_HOLE, and use the new
trait to implement the virtio block Discard command.

BUG= chromium:896314 
TEST=`mkfs.btrfs /dev/vdb` and verify the desired fallocate() is used
     and no write() calls are issued when inducing a failure

Change-Id: I67fd9a132758d8d766531ccca8358c7fe67b0460
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1286224
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/de198cc9be3f3741a4c9cc6f97275cc4ec7f7505/sys_util/src/lib.rs
[modify] https://crrev.com/de198cc9be3f3741a4c9cc6f97275cc4ec7f7505/qcow/src/qcow.rs
[modify] https://crrev.com/de198cc9be3f3741a4c9cc6f97275cc4ec7f7505/devices/src/virtio/block.rs
[modify] https://crrev.com/de198cc9be3f3741a4c9cc6f97275cc4ec7f7505/sys_util/src/write_zeroes.rs

Status: Fixed (was: Started)

Sign in to add a comment