Hello,
Milos Nikic, le mer. 16 sept. 2026 04:47:31 -0700, a ecrit:
> diff --git a/ext2fs/ext2fs.c b/ext2fs/ext2fs.c
> index 984df0448..5b996370e 100644
> --- a/ext2fs/ext2fs.c
> +++ b/ext2fs/ext2fs.c
> @@ -271,6 +271,8 @@ main (int argc, char **argv)
> fprintf (stderr, "ext2fs: journaling enabled on %s\n",
> diskfs_disk_name);
> JRNL_LOG_DEBUG ("Global Journal Initialized at %p", ext2_journal);
> diskfs_nput(jnode);
> + /* Recover any orphan inodes left from a previous unclean shutdown. */
No, it's not an unclean shutdown.
Anything that was still memory-mapped but unlinked when ext2fs got shut
down will be in that state. That's very common after upgrading daemons
or libraries.
> + ext2_recover_orphan_list ();
> diff --git a/ext2fs/orphan.c b/ext2fs/orphan.c
> new file mode 100644
> index 000000000..bdd0eb90b
> --- /dev/null
> +++ b/ext2fs/orphan.c
> +/* Add inode NP to the orphan list. */
> +void
> +diskfs_orphan_add (struct node *np)
> +{
> + ino_t inum = np->cache_id;
> + struct ext2_inode *di;
> + diskfs_transaction_t *txn = NULL;
> +
> + if (!ext2_journal)
> + return;
Can't it work without a journal?
Is the orphan list not a separate feature flag?
> + di->i_links_count = 0;
Is that really needed? We'd normally only ever call diskfs_orphan_add
when st_nlink got down to 0.
Also, linux' ext4 seems to be using orphans for truncated files too, we
might want to do that too.
> + /* Update the superblock to point to this inode as the new list head. */
> + sblock->s_last_orphan = htole32 (inum);
> + sblock_dirty = 1;
> +
> + if (txn)
> + {
> + journal_dirty_block (txn, boffs_block (bptr_offs (di)));
> +
> + /* Sync our private sblock to the Mach disk cache so the journal
> captures it */
> + memcpy (boffs_ptr (SBLOCK_OFFS), sblock, SBLOCK_SIZE);
> + journal_dirty_block (txn, boffs_block (SBLOCK_OFFS));
> + }
> +
> + dino_deref (di);
> +
> + diskfs_node_disknode (np)->on_orphan_list = 1;
> + pthread_mutex_unlock (&orphan_lock);
> +
> + if (txn)
> + diskfs_journal_stop_transaction (txn);
> + else
> + alloc_sync (np);
How is synchronization between the superblock, the inode, and the
journal handled? This needs to be explained in the comments.
> +/* Remove inode NP from the orphan list. */
> +void
> +diskfs_orphan_del (struct node *np)
> +{
[...]
> + else
> + {
> + /* Walk the list to find the predecessor. */
Can't we record that somewhere in memory? It can very quickly grow on
machine upgrade.
> +/* Recover (clean up) the orphan list at mount time. */
> +void
> +ext2_recover_orphan_list (void)
> +{
> + if (diskfs_readonly)
> + {
> + ext2_warning ("orphan inodes on readonly fs; leaving for fsck");
> + return;
Please put this warning after checking for empty orphan list.
> diff --git a/libdiskfs/dir-rename.c b/libdiskfs/dir-rename.c
> index 939d0b6ae..e85ef45bb 100644
> --- a/libdiskfs/dir-rename.c
> +++ b/libdiskfs/dir-rename.c
> @@ -240,6 +240,8 @@ diskfs_S_dir_rename (struct protid *fromcred,
> diskfs_node_update (fdp, diskfs_synchronous);
>
> fnp->dn_stat.st_nlink--;
> + if (fnp->dn_stat.st_nlink == 0)
> + diskfs_orphan_add (fnp);
> fnp->dn_set_ctime = 1;
>
> diskfs_node_update (fnp, diskfs_synchronous);
There are other st_nlink-- in this file, don't we want to orphan them?
> diff --git a/libdiskfs/dir-renamed.c b/libdiskfs/dir-renamed.c
> index 97487ce53..8fcaffdba 100644
> --- a/libdiskfs/dir-renamed.c
> +++ b/libdiskfs/dir-renamed.c
> @@ -212,6 +212,8 @@ diskfs_rename_dir (struct node *fdp, struct node *fnp,
> const char *fromname,
> if (!err)
> {
> tnp->dn_stat.st_nlink--;
> + if (tnp->dn_stat.st_nlink == 0)
> + diskfs_orphan_add (tnp);
> tnp->dn_set_ctime = 1;
> }
> diskfs_clear_directory (tnp, tdp, tocred);
Same here, there is another one in an error case.
> @@ -251,6 +253,8 @@ diskfs_rename_dir (struct node *fdp, struct node *fnp,
> const char *fromname,
> diskfs_dirremove (fdp, fnp, fromname, ds);
> ds = 0;
> fnp->dn_stat.st_nlink--;
> + if (fnp->dn_stat.st_nlink == 0)
> + diskfs_orphan_add (fnp);
> fnp->dn_set_ctime = 1;
> diskfs_file_update (fdp, diskfs_synchronous);
> diskfs_node_update (fnp, diskfs_synchronous);
> diff --git a/libdiskfs/node-drop.c b/libdiskfs/node-drop.c
> index a12c29ad0..bb2d38e26 100644
> --- a/libdiskfs/node-drop.c
> +++ b/libdiskfs/node-drop.c
> @@ -43,7 +43,7 @@ diskfs_drop_node (struct node *np)
> /* XXX: if the filesystem is readonly, we cannot remove the files with no
> link
You can now drop the XXX: you are fixing that case.
> but e.g. memory mapping still in memory. This notably happens when
> upgrading packages without restarting the corresponding processes. Fsck
> - will have to fix them. */
> + will have to fix them or the orphan list, if implemented. */
> if (np->dn_stat.st_nlink == 0 && !diskfs_readonly)
> {
> diskfs_check_readonly ();
> @@ -85,9 +85,13 @@ diskfs_drop_node (struct node *np)
> np->dn_stat.st_rdev = 0;
> np->dn_set_ctime = np->dn_set_atime = 1;
> diskfs_node_update (np, diskfs_synchronous);
> + diskfs_orphan_del (np);
> diskfs_free_node (np, savemode);
> }
> else
> + /* Here we don't remove the node from the orphan list
> + so that on the next restart file system has the
> + opportunity to deal with it before fsck. */
> diskfs_node_update (np, diskfs_synchronous);
>
> fshelp_drop_transbox (&np->transbox);
Thanks,
Samuel
