Directory block spare space is not zeroed

[anssih]

The code in mkfs.erofs does not seem to explicitly zero the spare space left in directory blocks (at least in all cases).

This is normally hidden for normal mkfs.erofs runs by erofs_dev_open() performing ftruncate() (for files) or BLKDISCARD (for block devices) at the beginning of operation, but AFAICS BLKDISCARD does not guarantee that the next read returns zeroes, and that is indeed what I observed in my case. This then breaks image reproducibility.

Attached is a simple patch that adds a hack that allows to simulate non-blkdiscard-zero block devices by skipping ftruncate() on request, and a simple reproducer that first creates an image normally and then again with a "device" prefilled with ones and ftruncate() disabled, resulting in a different image.

reproducer.sh
reproducer.patch

[hsiangkao]

Hi @anssih , is your original use case to use mkfs.erofs generate images to block devices directly?
Honestly, it needs more refactoring than expected, if that is really your use cases, how about triggering BLKDISCARDZEROES (old kernels) and fallocate(FALLOC_FL_WRITE_ZEROES) in mkfs.erofs beforehand for now?

[anssih]

Hi @anssih , is your original use case to use mkfs.erofs generate images to block devices directly?

Yes.

Specifically, I transport filesystem contents as .tar.xz to minimize size (as xz naturally compresses plain data better than the already-lz4-compressed erofs image) and then use mkfs.erofs --tar=f to regenerate the filesystem reproducibly directly on the block device (with verification afterwards).

Honestly, it needs more refactoring than expected, if that is really your use cases, how about triggering BLKDISCARDZEROES (old kernels) and fallocate(FALLOC_FL_WRITE_ZEROES) in mkfs.erofs beforehand for now?

BLKDISCARDZEROES just asks whether discard should result in zeroes so that won't help alone.

I'm fine with working around the issue by generating the image into a tmpfs file and then writing that to the block device, it only adds a couple of seconds at most to the process. So there is no rush for a "proper" fix from my end :)

[hsiangkao]

Honestly, it needs more refactoring than expected, if that is really your use cases, how about triggering BLKDISCARDZEROES (old kernels) and fallocate(FALLOC_FL_WRITE_ZEROES) in mkfs.erofs beforehand for now?

BLKDISCARDZEROES just asks whether discard should result in zeroes so that won't help alone.

I don't quite get the point, for example, e2fsprogs use it for zeroout devices beforehind so that there will
not be garbaged data too.

But BLKDISCARDZEROES is noop in the recent kernels, and BLKZEROOUT ioctl replaces it, and it can also be triggered by fallocate(FALLOC_FL_WRITE_ZEROES)

I'm fine with working around the issue by generating the image into a tmpfs file and then writing that to the block device, it only adds a couple of seconds at most to the process. So there is no rush for a "proper" fix from my end :)

I understand, and the problem now is that mkfs.erofs writes metadata in the byte granularity due to historical reasons.
In principle, it should write metadata in aligned filesystem blocks, so that garbaged data will be replaced with bridged zeros and it avoids RMW cycles on the raw block devices.

But it needs some developments, so it seems not a simple fix for now.

[anssih]

Honestly, it needs more refactoring than expected, if that is really your use cases, how about triggering BLKDISCARDZEROES (old kernels) and fallocate(FALLOC_FL_WRITE_ZEROES) in mkfs.erofs beforehand for now?

BLKDISCARDZEROES just asks whether discard should result in zeroes so that won't help alone.

I don't quite get the point, for example, e2fsprogs use it for zeroout devices beforehind so that there will not be garbaged data too.

e2fsprogs calls it but just sets a flag (CHANNEL_FLAGS_DISCARD_ZEROES) based on the result, and that flag is later used to skip calling ext2fs_zero_blocks2() (which in turn would try in order FALLOC_FL_ZERO_RANGE, FALLOC_FL_PUNCH_HOLE, writing zeroes manually).

So yes, you can call BLKDISCARDZEROES and if it returns 1 then nothing else needs to be done, but if it returns 0 (like on current kernels) then further actions are needed (e.g. the mentioned FALLOC_FL_WRITE_ZEROES, but that is not available on all devices/kernels so fallbacks are needed).

[hsiangkao]

Honestly, it needs more refactoring than expected, if that is really your use cases, how about triggering BLKDISCARDZEROES (old kernels) and fallocate(FALLOC_FL_WRITE_ZEROES) in mkfs.erofs beforehand for now?

BLKDISCARDZEROES just asks whether discard should result in zeroes so that won't help alone.

I don't quite get the point, for example, e2fsprogs use it for zeroout devices beforehind so that there will not be garbaged data too.

e2fsprogs calls it but just sets a flag (CHANNEL_FLAGS_DISCARD_ZEROES) based on the result, and that flag is later used to skip calling ext2fs_zero_blocks2() (which in turn would try in order FALLOC_FL_ZERO_RANGE, FALLOC_FL_PUNCH_HOLE, writing zeroes manually).

So yes, you can call BLKDISCARDZEROES and if it returns 1 then nothing else needs to be done, but if it returns 0 (like on current kernels) then further actions are needed (e.g. the mentioned FALLOC_FL_WRITE_ZEROES, but that is not available on all devices/kernels so fallbacks are needed).

Yes, but recent kernels have software fallback anyway. And anyway, it's another temporary workaround, I have to find time to develop block-aligned metadata writer when I have extra time next.