Re: [PATCH 32/53] ext4: move dcache modifying code out of __ext4_link()
From: Jan Kara <jack@suse.cz>
Date: 2026-03-17 10:00:49
Also in:
ceph-devel, gfs2, linux-cifs, linux-efi, linux-ext4, linux-fsdevel, linux-mm, linux-nfs, linux-um, linux-unionfs, linux-xfs, lkml
On Fri 13-03-26 08:12:19, NeilBrown wrote: ...
quoted hunk ↗ jump to hunk
diff --git a/fs/dcache.c b/fs/dcache.c index a1219b446b74..c48337d95f9a 100644 --- a/fs/dcache.c +++ b/fs/dcache.c@@ -358,7 +358,7 @@ static inline int dname_external(const struct dentry *dentry) return dentry->d_name.name != dentry->d_shortname.string; } -void take_dentry_name_snapshot(struct name_snapshot *name, struct dentry *dentry) +void take_dentry_name_snapshot(struct name_snapshot *name, const struct dentry *dentry) { unsigned seq; const unsigned char *s;
The constification of take_dentry_name_snapshot() should probably be a separate patch? Also I'd note that this constification (and the constification of __ext4_fc_track_link()) isn't really needed here because ext4_fc_track_link() will immediately bail through ext4_fc_disabled() when fast commit replay is happening so __ext4_fc_track_link() never gets called in that case - more about that below.
quoted hunk ↗ jump to hunk
@@ -1471,7 +1471,15 @@ static int ext4_fc_replay_link_internal(struct super_block *sb, goto out; } + ihold(inode); + inc_nlink(inode); ret = __ext4_link(dir, inode, dentry_inode); + if (ret) { + drop_nlink(inode); + iput(inode); + } else { + d_instantiate(dentry_inode, inode); + } /* * It's possible that link already existed since data blocks * for the dir in question got persisted before we crashed OR
...
quoted hunk ↗ jump to hunk
@@ -3460,8 +3460,6 @@ int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry) ext4_handle_sync(handle); inode_set_ctime_current(inode); - ext4_inc_count(inode); - ihold(inode); err = ext4_add_entry(handle, dentry, inode); if (!err) {@@ -3471,11 +3469,7 @@ int __ext4_link(struct inode *dir, struct inode *inode, struct dentry *dentry) */ if (inode->i_nlink == 1) ext4_orphan_del(handle, inode); - d_instantiate(dentry, inode); - ext4_fc_track_link(handle, dentry); - } else { - drop_nlink(inode); - iput(inode); + __ext4_fc_track_link(handle, inode, dentry);
This looks wrong. If fastcommit replay is running, we must skip calling __ext4_fc_track_link(). Similarly if the filesystem is currently inelligible for fastcommit (due to some complex unsupported operations running in parallel). Why did you change ext4_fc_track_link() to __ext4_fc_track_link()?
quoted hunk ↗ jump to hunk
@@ -3504,7 +3498,16 @@ static int ext4_link(struct dentry *old_dentry, err = dquot_initialize(dir); if (err) return err; - return __ext4_link(dir, inode, dentry); + ihold(inode); + ext4_inc_count(inode);
I'd put inc_nlink() here instead. We are guaranteed to have a regular file anyway and it matches what we do in ext4_fc_replay_link_internal(). Alternatively we could consistently use ext4_inc_count() & ext4_dec_count() in these functions.
+ err = __ext4_link(dir, inode, dentry);
+ if (err) {
+ drop_nlink(inode);
+ iput(inode);
+ } else {
+ d_instantiate(dentry, inode);
+ }
+ return err;
}Honza -- Jan Kara [off-list ref] SUSE Labs, CR