Hello, Please add this as a patch to the series. It is just initialization of the fields so we don't get junk in some instances.
Btw i caught this with a small asan build on Linux against libidksfs and ext2fs. I can now run selective tests against the code under all the *sans and exercise the code paths quickly, Perhaps interesting things will come out of this, we shall see. Thanks, Milos On Sun, Oct 25, 2026 at 8:19 PM Milos Nikic <[email protected]> wrote: > Hello, > > First to state a problem: > > Ext2fs preallocates up to 8 blocks when a file grows, and marks them > allocated in the block bitmap while only the node's memory owns them. > A crash before the file uses them or gives them back leaks them. With > the journal this matters even more: the bitmap change commits with the > allocation, and a normal boot only replays the log, without the full > check that would free the blocks. Killing the translator during > tar -xf of a Linux tree left such blocks repeatedly. > > Fix: > Patch 1 is the fix the journal needs. It keeps the preallocation > window in memory only, in a per-group map next to the busy blocks, and > sets a block in the bitmap only when a file takes it. The layout does > not change: in the test below, files get exactly the same fragments as > before (1949 each). Ten kills of the translator at random times during > rm -rf and tar -xf, and filling the disk with several writers, left > e2fsck -f clean. Simply not preallocating with a journal, as I first > tried, also fixes the leak, but doubles the fragments (4097). > > Improvement: > Patch 2 is an improvement on top inspired by ext3, and does not depend > on the journal. Files written at the same time take blocks in turn, > so each fragment was at most one 8-block window long. > A file that keeps using up its window now gets one twice as big, up to > 1024 blocks, and the window is dropped at the last close instead of > whenever > the inode is written. > ext3 also doubled its reservation windows, up to 1027 blocks. > > Fragments per file, 3 runs each on a fresh 4 KB-block filesystem in an > i386 VM: > > before patch 1 patch 2 > 8 files appended in turn (64 MB) 1949 1949 23 > 4 parallel dd (256 MB) ~6400 ~6300 55-70 > > The same holds without a journal (23 for the appended files). With > two concurrent tar -xf of a Linux tree, half as many files are > fragmented with patch 2 (0.4% against 0.8%), but about 200 more small > holes of 4 to 64K stay in the free space, 1% of it, mostly what small > files leave of their windows. The time is the same (about 205 s). > Capping a window at the file's size made both worse, so it is not in > the series. A large Debian upgrade with both patches on the root > filesystem left e2fsck clean. > > The patches are independent of other work that is still pending. > > Thanks, > Milos > > Milos Nikic (2): > ext2fs: Keep block preallocation in memory > ext2fs: Grow preallocation windows for sequential writers > > ext2fs/balloc.c | 274 ++++++++++++++++++++++++++++++++-------------- > ext2fs/ext2_fs.h | 3 + > ext2fs/ext2fs.h | 6 +- > ext2fs/getblk.c | 70 ++++++------ > ext2fs/ialloc.c | 1 + > ext2fs/inode.c | 17 ++- > ext2fs/truncate.c | 4 +- > 7 files changed, 256 insertions(+), 119 deletions(-) > > -- > 2.56.0 > >
From dd60837b953405c14e3185a5c8ab156b13dc4384 Mon Sep 17 00:00:00 2001 From: Milos Nikic <[email protected]> Date: Mon, 26 Oct 2026 09:01:44 -0700 Subject: [PATCH] ext2fs: Start a node with no preallocation window A node can be dropped before its inode is read: a lookup that lost the race to make it, or whose read of the inode failed, drops the node it made. Dropping a node gives its preallocation window back, now that the window is kept in memory, but the window's fields were only set when the inode was read. Such a node gave back garbage. Clear the window when the node is made. --- ext2fs/inode.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext2fs/inode.c b/ext2fs/inode.c index 2f35ab3fd..a1384b937 100644 --- a/ext2fs/inode.c +++ b/ext2fs/inode.c @@ -65,6 +65,12 @@ diskfs_user_make_node (struct node **npp, struct lookup_context *ctx) dn->on_orphan_list = 0; dn->orphan_prev = NULL; dn->orphan_next = NULL; + /* No preallocation window: a node can be dropped before its inode is + read (a lookup that lost a race, a failed read), and dropping it + gives its window back. */ + dn->info.i_prealloc_block = 0; + dn->info.i_prealloc_count = 0; + dn->info.i_prealloc_size = 0; pthread_rwlock_init (&dn->alloc_lock, NULL); pokel_init (&dn->indir_pokel, diskfs_disk_pager, disk_cache); -- 2.56.0
