mido icon indicating copy to clipboard operation
mido copied to clipboard

TRIM pass through encrypted "/data" partition (dm-crypt)

Open mirfatif opened this issue 5 years ago • 3 comments

~# cat /sys/firmware/devicetree/base/model /sys/class/block/mmcblk0/device/{uevent,date}
Qualcomm Technologies, Inc. MSM8953 + PMI8950 QRD SKU3
DRIVER=mmcblk
MMC_TYPE=MMC
MMC_NAME=RX1BMB
02/2017
~# getprop ro.build.product
mido

The underlying hardware MCP: RX1BMB contains eMMC 5.1, which definitely supports TRIM. So it should work on our device. Other filesystems accept FITRIM ioctl requests, except /data which is mounted from virtual block device /dev/block/dm-0 encrypted over /dev/block/bootdevice/by-name/userdata:

~# grep -E 'cache|dm-' /proc/self/mountstats
device /dev/block/bootdevice/by-name/cache mounted on /cache with fstype f2fs
device /dev/block/dm-0 mounted on /data with fstype f2fs

~# fstrim -v /cache; fstim -v /data
/cache: 0 B (0 bytes) trimmed
fstrim: /data: the discard operation is not supported

~# /system/bin/sm fstrim
PersistableBundle[{}]
~# logcat | tr -s ' ' | cut -d ' ' -f5- | grep vold
D vold    : Starting trim of /data
W vold    : Trim failed on /data: Operation not supported on transport endpoint
D vold    : Starting trim of /cache
I vold    : Trimmed 18446744073709551615 bytes on /cache in 0ms

Using btrace (requires CONFIG_BLK_DEV_IO_TRACE=y and possibly performance governor (1)) shows discard operations received at dm-0 but not at userdata.

TRIM passthrough dm-crypt is disabled by default because it exposes the free space at undelying block level, otherwise looking completely filled (encrypted random data). Optional Support for allow_discards was added in version 1.11.0 (2) of crypt (3) target of kernel version 3.1 and cryptsetup 1.4.x (4).

~# dmsetup --version; cryptsetup --version
Library version:   1.02.156 (2019-03-22)
Driver version:    4.28.0
cryptsetup 2.1.0

~# dmsetup targets
req-crypt        v1.0.0
verity           v1.3.0
crypt            v1.14.0
striped          v1.5.1
linear           v1.2.1
error            v1.2.0

So vold should pass allow_discards to kernel when loading dm-crypt mapping table (5), but as we can see from log that allow_discards is not being appended to extra parameters:

I device-mapper: req-crypt: dm-req-crypt successfully initalized.
...
I Cryptfs : Extra parameters for dm_crypt: fde_enabled ice
D Cryptfs_hw: HW based disk encryption is enabled
I Cryptfs : target_type = req-crypt
I Cryptfs : real_blk_name = /dev/block/bootdevice/by-name/userdata, extra_params = fde_enabled ice
I device-mapper: req-crypt: req_crypt_ctr: Mapping block_device /dev/block/bootdevice/by-name/userdata to dm-req-crypt ok!
...
W F2FS-fs (dm-0): mounting with "discard" option, but the device does not support discard

I also tried to refresh table mapping with allow_discards using cryptsetup, as seen with dmsetup table (6):

~# file /dev/mapper/*
/dev/mapper/*: cannot open `/dev/mapper/*' (No such file or directory)

~# dmsetup mknodes; file /dev/mapper/*
/dev/mapper/control:  character special (10/236)
/dev/mapper/userdata: block special (253/0)

~# dmsetup table userdata
0 50322351 req-crypt

~# cryptsetup --allow-discards refresh userdata; dmsetup table userdata
0 50322351 req-crypt

I came to know that on Qualcomm devices, Android uses Qualcomm's hardware-accelerated req-crypt target (7). A patch was applied to vold to incorporate dm-req-crypt functionality, but was later removed with Treble, to be handled in HALs at SoC/OEM vendor end. But still passing allow_discards is at vold's end.

Creating new mapping using cryptsetup/dmsetup works for crypt target, but causes kernel panic with req-crypt (or may be I'm doing it wrong):

~# truncate -s 1G test.img && DEV=$(losetup -f --show test.img)
~# cryptsetup --type plain open $DEV test
Enter passphrase for /dev/loop1:
~# dmsetup table test | sed 's/00//g'
0 2097152 crypt aes-cbc-essiv:sha256 0 7:1 0
~# cryptsetup --allow-discards refresh test; dmsetup table test | sed 's/00//g'
Enter passphrase for /dev/loop1:
0 2097152 crypt aes-cbc-essiv:sha256 0 7:1 0 1 allow_discards

~# cryptsetup close test && dmsetup create test --table "0 $(blockdev --getsz $DEV) crypt aes-cbc-essiv:sha256 $(echo -n "mypassword" | sha256sum | awk '{print $1}') 0 $DEV 0 1 allow_discards" && dmsetup mknodes && dmsetup table test | sed 's/00//g'
0 2097152 crypt aes-cbc-essiv:sha256 0 7:1 0 1 allow_discards

~# cryptsetup close test && dmsetup create test --table "0 $(blockdev --getsz $DEV) req-crypt aes-xts $(echo -n "mypassword" | sha256sum | awk '{print $1}') 0 $DEV 0 1 allow_discards" && dmsetup mknodes && dmsetup table test | sed 's/00//g'

After reboot, /sys/fs/pstore/console-ramoops contains:

[69029.452591] Kernel panic - not syncing: Fatal exception
[69029.452605] CPU4: stopping
[69029.452622] CPU: 4 PID: 32158 Comm: dmsetup Tainted: G    D W    3.18.134-FrancoKernel
[69029.452627] Hardware name: Qualcomm Technologies, Inc. MSM8953 + PMI8950 QRD SKU3 (DT)
...
[69030.871027] Rebooting in 5 seconds..
[69035.871038] Going down for restart now

dmsetup/cryptsetup failed to work with req-crypt. So I came across this commit which forces allow_discards in dm-crypt target instead of fixing in vold source. I looked into dm-req-crypt for the same and realized that vold isn't the only culprit, it's also TODO: Discard Support in kernel:

--- a/drivers/md/dm-req-crypt.c	2019-06-29 11:51:40.826829700 +0500
+++ b/drivers/md/dm-req-crypt.c	2019-06-29 12:05:18.779391900 +0500
@@ -1316,6 +1316,8 @@
 	 */
 	ti->num_flush_bios = 1;
 	/* TODO: Discard support */
+	DMINFO("%s: Forcing \'allow_discards\' in-kernel",__func__);
+	ti->num_discard_bios = 1;
 
 	err = 0;
 	DMINFO("%s: Mapping block_device %s to dm-req-crypt ok!\n",

And now it works:

I device-mapper: req-crypt: dm-req-crypt successfully initalized.
...
I Cryptfs : Extra parameters for dm_crypt: fde_enabled ice
D Cryptfs_hw: HW based disk encryption is enabled 
I Cryptfs : target_type = req-crypt
I Cryptfs : real_blk_name = /dev/block/bootdevice/by-name/userdata, extra_params = fde_enabled ice
I device-mapper: req-crypt: req_crypt_ctr: Forcing 'allow_discards' in-kernel
I device-mapper: req-crypt: req_crypt_ctr: Mapping block_device /dev/block/bootdevice/by-name/userdata to dm-req-crypt ok!
...
I F2FS-fs (dm-0): Mounted with checkpoint version = 56cc765c
~# fstrim -v /data
/data: 3.9 MiB (4096000 bytes) trimmed

@franciscofranco you can merge this to force allow_discards, though people are doing opposite too.

mirfatif avatar Jun 30 '19 20:06 mirfatif

Sure.

franciscofranco avatar Jul 03 '19 16:07 franciscofranco

Also consider write /sys/block/mmcblk0/queue/scheduler noop, it's considered more flash media friendly.

mirfatif avatar Jul 04 '19 08:07 mirfatif

Also consider write /sys/block/mmcblk0/queue/scheduler noop, it's considered more flash media friendly.

It's not considered more "flash media friendly", but that's a discussion for another moon.

franciscofranco avatar Jul 04 '19 16:07 franciscofranco