On Fri 13-03-26 08:12:20, NeilBrown wrote: > From: NeilBrown <[email protected]> > > ext4_fc_replay_link_internal() uses two dentries to simply code-reuse > when replaying a "link" operation. It does not need to interact with > the dcache and removes the dentries shortly after adding them. > > They are passed to __ext4_link() which only performs read accesses on > these dentries and only uses the name and parent of dentry_inode (plus > checking a flag is unset) and only uses the inode of the parent. > > So instead of allocating dentries and adding them to the dcache, allocat > two dentries on the stack, set up the required fields, and pass these to > __ext4_link(). > > This substantially simplifies the code and removes on of the few uses of > d_alloc() - preparing for its removal. > > Signed-off-by: NeilBrown <[email protected]>
Looks good to me. Feel free to add: Reviewed-by: Jan Kara <[email protected]> Honza > --- > fs/ext4/fast_commit.c | 40 ++++++++-------------------------------- > 1 file changed, 8 insertions(+), 32 deletions(-) > > diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c > index 2a5daf1d9667..e3593bb90a62 100644 > --- a/fs/ext4/fast_commit.c > +++ b/fs/ext4/fast_commit.c > @@ -1446,8 +1446,6 @@ static int ext4_fc_replay_link_internal(struct > super_block *sb, > struct inode *inode) > { > struct inode *dir = NULL; > - struct dentry *dentry_dir = NULL, *dentry_inode = NULL; > - struct qstr qstr_dname = QSTR_INIT(darg->dname, darg->dname_len); > int ret = 0; > > dir = ext4_iget(sb, darg->parent_ino, EXT4_IGET_NORMAL); > @@ -1457,28 +1455,14 @@ static int ext4_fc_replay_link_internal(struct > super_block *sb, > goto out; > } > > - dentry_dir = d_obtain_alias(dir); > - if (IS_ERR(dentry_dir)) { > - ext4_debug("Failed to obtain dentry"); > - dentry_dir = NULL; > - goto out; > - } > + { > + struct dentry dentry_dir = { .d_inode = dir }; > + const struct dentry dentry_inode = { > + .d_parent = &dentry_dir, > + .d_name = QSTR_LEN(darg->dname, darg->dname_len), > + }; > > - dentry_inode = d_alloc(dentry_dir, &qstr_dname); > - if (!dentry_inode) { > - ext4_debug("Inode dentry not created."); > - ret = -ENOMEM; > - 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); > + ret = __ext4_link(dir, inode, &dentry_inode); > } > /* > * It's possible that link already existed since data blocks > @@ -1493,16 +1477,8 @@ static int ext4_fc_replay_link_internal(struct > super_block *sb, > > ret = 0; > out: > - if (dentry_dir) { > - d_drop(dentry_dir); > - dput(dentry_dir); > - } else if (dir) { > + if (dir) > iput(dir); > - } > - if (dentry_inode) { > - d_drop(dentry_inode); > - dput(dentry_inode); > - } > > return ret; > } > -- > 2.50.0.107.gf914562f5916.dirty > -- Jan Kara <[email protected]> SUSE Labs, CR
