On Fri 13-03-26 08:12:19, NeilBrown wrote:
...
> 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.
> @@ -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
...
> @@ -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()?
> @@ -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 <[email protected]>
SUSE Labs, CR
