Applied, thanks! Samuel
Milos Nikic, le ven. 26 juin 2026 14:56:36 -0700, a ecrit: > OK i take it back. > > I think i managed to reproduce it, or at least find a separate deadlock bug. > > This one was happening when i attached a 50gb secondary partition with > Journaling enabled to a root partition that also has Journaling enabled and > then ./configure and make util-linux on the 50gb partition. > It wouldn't happen every time, just sometimes, it would lock up. > > I have a fix and i am sending it here in the attachment. > I have a brief explanation in the commit message what (i think) is going on > and > how do i fix it, let me know if you want me to expand on it/change it etc. > > Let me also know if you want it on a separate mail thread. > > I hope this also improves on what you are seeing > > Thank you, > Milos > > On Thu, Jun 25, 2026 at 9:18 PM Milos Nikic <[1][email protected]> wrote: > > Hey Samuel, > > I have spent time trying to reproduce it locally on my machine in qemu > today. > I have a secondary 50gb image and i start it in qemu as a > qemu-system-x86_64 \ > -m 1024 -s -machine pc -cpu > host -accel kvm \ > -drive file=/home/loshmi/ > Projects/hurd/debian-hurd_32.img,if=ide,index=0,format=raw,cache= > writethrough \ > -drive file=/home/loshmi/ > Projects/hurd/hurd-build-drive.img,if=ide,index=1,format=raw,cache= > writethrough \ > -boot order=c \ > -netdev > user,id=net0,net=[2] > 192.168.76.0/24,dhcpstart=192.168.76.5,hostfwd=tcp:127.0.0.1:2222-:22 \ > -device e1000,netdev=net0 \ > -serial stdio > > then from inside hurd i did: > $ cd /dev > $ sudo ./MAKEDEV hd1 > $ sudo mke2fs /dev/hd1 > $ sudo tune2fs -j /dev/hd1 > $ sudo fsck.ext2 -f /dev/hd1 > $ sudo settrans -ca /mnt/build /hurd/ext2fs.static /dev/hd1 > $ cd /mnt/build/ > $ sudo apt-get source util-linux > $ cd util-linux-2.41 > > than had to fight with autoconf version etc, but in the end for me > configure and then run works without any issues , i tried multiple times, > even while running synthetic background tasks. > I wonder is it because my laptop is backed by nvme or something similar, > or > do you find that something is a miss in my emulation? > > > As far as: > /dev/sd3: Superblock needs_recovery flag is clear, but journal has data. > /dev/sd3: Run journal anyway > > That happens if it locks up and it has to be restarted, because journal > didn't manage to commit/checkpoint so things are still there. > The issue is that it locked up on real machine. > > Regards, > Milos > > On Thu, Jun 25, 2026 at 3:25 AM Samuel Thibault > <[3][email protected] > > wrote: > > Hello, > > I'm getting hangs quite often on my i386 hurd box, just by compiling > the > util-linux package on a 50G partition. > > One time, I got at reboot: > > /dev/sd3: Superblock needs_recovery flag is clear, but journal has > data. > /dev/sd3: Run journal anyway > > /dev/sd3: UNEXPECTED INCONSISTENCY; RUN fsck MANUALLY. > (i.e., without -a or -p options) > > Samuel > > > References: > > [1] mailto:[email protected] > [2] > http://192.168.76.0/24,dhcpstart=192.168.76.5,hostfwd=tcp:127.0.0.1:2222-:22 > [3] mailto:[email protected] > From 82057d164de5fdd4dacc05ba5d66382d228477ce Mon Sep 17 00:00:00 2001 > From: Milos Nikic <[email protected]> > Date: Fri, 26 Jun 2026 14:33:23 -0700 > Subject: [PATCH] ext2fs: Fix journal deadlock during transaction finalization > > When a VFS thread finalizes a transaction in journal_stop_transaction_locked, > it holds the global j_state_lock while performing memcpy to hydrate shadow > buffers from the live Mach virtual memory cache (bptr). > > Under heavy memory pressure, this memcpy can trigger a Page Fault. If the > Mach kernel decides to evict dirty pages to satisfy the fault, the ext2fs > pager is invoked. The pager then attempts to write blocks to disk, which > requires acquiring the j_state_lock but that lock is already held by the > suspended VFS thread, causing a circular deadlock. > > Fix: > Decouple memory hydration (memcpy) from the j_state_lock. > - While holding the lock, identify blocks needing hydration and stage them in > a thread-local list using jb_next. > - Temporarily increment t_updates to protect the transaction from being freed. > - Drop the j_state_lock to perform the memcpy. If a page fault occurs here, > the pager can now safely acquire the lock to satisfy the I/O. > - Re-acquire the lock and decrement t_updates to finalize the commit. > > This ensures the lock is never held while accessing pageable memory. > > Test: > I have run the scenario in which i can recreate the deadlock multiple > times with debugging messages on and off, and i don't manage to cause this > specific > deadlock scenario any more. > --- > ext2fs/journal.c | 90 ++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 64 insertions(+), 26 deletions(-) > > diff --git a/ext2fs/journal.c b/ext2fs/journal.c > index c50c742f8..60cf926f0 100644 > --- a/ext2fs/journal.c > +++ b/ext2fs/journal.c > @@ -1080,14 +1080,18 @@ journal_stop_transaction_locked (journal_t *journal, > return; > } > txn->t_updates--; > - if (txn->t_updates == 0) > + /* Continue until the map is proven clean while locked */ > + while (txn->t_updates == 0) > { > size_t iter = 0; > journal_buffer_t *jb_exp; > + journal_buffer_t *copy_list_head = NULL; > + journal_buffer_t *copy_list_tail = NULL; > + > + /* Traverse while map is locked */ > while ((jb_exp = > journal_map_iterate (&txn->t_buffer_map, &iter)) != NULL) > { > - /* Buffer hydration (memory copying) time. */ > if (jb_exp->needs_copy) > { > if (jb_exp->lifeboat_index >= 0) > @@ -1095,35 +1099,68 @@ journal_stop_transaction_locked (journal_t *journal, > memcpy (jb_exp->jb_shadow_data, > ext2_lifeboat.payloads[jb_exp->lifeboat_index], > block_size); > + jb_exp->needs_copy = 0; > } > else > { > - /** > - * Calculate the pointer to the live Mach VM cache for this > block. > - * Because t_updates is 0 AND we hold a lock, we are > mathematically > - * guaranteed that no VFS threads are currently mutating this > block > - * because if they were mutating it they would have to first > obtain > - * the journal lock AND also increase the t_updates. > - */ > - void *live_cache_ptr = bptr (jb_exp->jb_blocknr); > - /** > - * We execute exactly ONE memory copy per block, > - * capturing the fully settled, tear-free state of the RAM. > - * We do this even if jb_is_written == 1, because if the pager > - * rushed the block, we MUST capture this settled state into > the > - * WAL so it can overwrite the pager's rushed data during > recovery! > - */ > - memcpy (jb_exp->jb_shadow_data, live_cache_ptr, block_size); > + jb_exp->jb_next = NULL; > + if (!copy_list_head) > + copy_list_head = jb_exp; > + else > + copy_list_tail->jb_next = jb_exp; > + copy_list_tail = jb_exp; > } > - /* We are done with this block even if t_updates reach 0 again > before > - * this transaction is committed. If we get notified that this > block > - * has been modified again journal_dirty_blocks must set > needs_copy > - * back to 1. */ > - jb_exp->needs_copy = 0; > } > } > - /* If anyone is sleeping in the commit loop waiting for this, wake > them */ > - pthread_cond_broadcast (&journal->j_commit_wait); > + > + if (!copy_list_head) > + { > + /* Map is perfectly clean. Wake the committer and break the loop. */ > + pthread_cond_broadcast (&journal->j_commit_wait); > + break; > + } > + > + /* Now lockless hydration, we will update t_updates to > 0 so none can > steal the txn. */ > + txn->t_updates++; > + JOURNAL_UNLOCK (journal); > + > + journal_buffer_t *curr = copy_list_head; > + while (curr) > + { > + /** > + * Calculate the pointer to the live Mach VM cache for this block. > + * Because t_updates is 0 AND we hold a lock, we are mathematically > + * guaranteed that no VFS threads are currently mutating this block > + * because if they were mutating it they would have to first obtain > + * the journal lock AND also increase the t_updates. > + */ > + void *live_cache_ptr = bptr (curr->jb_blocknr); > + /** > + * We execute exactly ONE memory copy per block, > + * capturing the fully settled, tear-free state of the RAM. > + * We do this even if jb_is_written == 1, because if the pager > + * rushed the block, we MUST capture this settled state into the > + * WAL so it can overwrite the pager's rushed data during recovery! > + */ > + memcpy (curr->jb_shadow_data, live_cache_ptr, block_size); > + /* We are done with this block even if t_updates reach 0 again before > + * this transaction is committed. If we get notified that this block > + * has been modified again journal_dirty_blocks must set needs_copy > + * back to 1. */ > + curr->needs_copy = 0; > + > + journal_buffer_t *next = curr->jb_next; > + curr->jb_next = NULL; > + curr = next; > + } > + > + JOURNAL_LOCK (journal); > + /* We need to decrement what we incremented. */ > + txn->t_updates--; > + > + /* If t_updates is still 0, the loop repeats to verify no blocks were > + added. If t_updates > 0, another thread joined while we were > unlocked > + and they will handle the final sweep. We safely fall out. */ > } > } > > @@ -1390,7 +1427,8 @@ journal_quiesce_checkpoints (void) > /* Wait for any active commit to finish writing to the log */ > while (ext2_journal->j_committing_transaction != NULL && err == 0) > err = pthread_cond_clockwait (&ext2_journal->j_commit_done, > - &ext2_journal->j_state_lock, CLOCK_MONOTONIC, > &ts); > + &ext2_journal->j_state_lock, > + CLOCK_MONOTONIC, &ts); > if (err) > { > /* If we hit ETIMEDOUT, a VFS thread likely leaked a t_updates refcount > -- > 2.54.0 > -- Samuel if (argc > 1 && strcmp(argv[1], "-advice") == 0) { printf("Don't Panic!\n"); exit(42); } -- Arnold Robbins in the LJ of February '95, describing RCS
