On 11/28/2017 11:20 AM, Darrick J. Wong wrote:
Ok, if I recall correctly, I think I had found a bug where the offset was not getting set when it should have, and fixed it when I added the rename patch to the original set. I will fold these down into their respective patches.On Fri, Nov 17, 2017 at 11:21:43AM -0700, Allison Henderson wrote:This patch removes the old parent pointer attribute during the rename operation, and re-adds the updated parent pointer Signed-off-by: Allison Henderson<allison.henderson@xxxxxxxxxx> --- fs/xfs/libxfs/xfs_dir2.c | 6 ++++-- fs/xfs/xfs_inode.c | 26 ++++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c index 25b370b..ed25203 100644 --- a/fs/xfs/libxfs/xfs_dir2.c +++ b/fs/xfs/libxfs/xfs_dir2.c @@ -324,10 +324,11 @@ xfs_dir_createname( else rval = xfs_dir2_node_addname(args);+out_free:/* return the location that this entry was place in the parent inode */ if (offset) *offset = args->offset; -out_free: + kmem_free(args); return rval; } @@ -496,9 +497,10 @@ xfs_dir_removename( rval = xfs_dir2_leaf_removename(args); else rval = xfs_dir2_node_removename(args); +out_free: if (offset) *offset = args->offset; -out_free: +Why do these labels need to be moved here? The *offset = ... lines are new, so why not put them in their final place in the patch that adds those lines?
kmem_free(args); return rval; } diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index a289a40..da5c761 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2984,6 +2984,8 @@ xfs_rename( bool src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode); int spaceres; int error; + xfs_dir2_dataptr_t new_diroffset; + xfs_dir2_dataptr_t old_diroffset;trace_xfs_rename(src_dp, target_dp, src_name, target_name); @@ -3086,13 +3088,12 @@ xfs_rename(*/ error = xfs_dir_createname(tp, target_dp, target_name, src_ip->i_ino, &first_block, &dfops, - spaceres, NULL); + spaceres, &new_diroffset); if (error) goto out_bmap_cancel;xfs_trans_ichgtime(tp, target_dp,XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG); -Huh?
Sorry, will fix
Oh, initially I had added this with the thought that we update parent pointers when we get new parents, but I think you're right. We'll need to update if the name changes too. I will take out the new_parent check. Thx!if (new_parent && src_is_directory) { error = xfs_bumplink(tp, target_dp); if (error) @@ -3126,7 +3127,7 @@ xfs_rename( */ error = xfs_dir_replace(tp, target_dp, target_name, src_ip->i_ino, &first_block, &dfops, - spaceres, NULL); + spaceres, &new_diroffset); if (error) goto out_bmap_cancel;@@ -3161,7 +3162,7 @@ xfs_rename(*/ error = xfs_dir_replace(tp, src_ip, &xfs_name_dotdot, target_dp->i_ino, &first_block, &dfops, - spaceres, NULL); + spaceres, &new_diroffset); ASSERT(error != -EEXIST); if (error) goto out_bmap_cancel; @@ -3200,11 +3201,12 @@ xfs_rename( */ if (wip) { error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino, - &first_block, &dfops, spaceres, NULL); + &first_block, &dfops, spaceres, + &old_diroffset); } else error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino, &first_block, &dfops, spaceres, - NULL); + &old_diroffset); if (error) goto out_bmap_cancel;@@ -3234,6 +3236,18 @@ xfs_rename(VFS_I(wip)->i_state &= ~I_LINKABLE; }+ if (new_parent && xfs_sb_version_hasparent(&mp->m_sb)) {I'm confused about checking new_parent -- does this cause us to forget to update the pptr for a rename within a directory? I'm assuming that all of these directory operations will get their own xfstests in time... (create a file, check pptr; hardlink a file, check both pptrs; delete one of the links, check pptrs; check pptr after a rename within a dir; check pptr after a rename between dirs; etc.) --D
+ error = xfs_parent_add(tp, target_dp, src_ip, target_name, + new_diroffset, &dfops, &first_block); + if (error) + goto out_bmap_cancel; + + error = xfs_parent_remove(tp, src_dp, src_ip, + old_diroffset, &dfops, &first_block); + if (error) + goto out_bmap_cancel; + } + xfs_trans_ichgtime(tp, src_dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG); xfs_trans_log_inode(tp, src_dp, XFS_ILOG_CORE); if (new_parent) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message tomajordomo@xxxxxxxxxxxxxxx More majordomo info athttp://vger.kernel.org/majordomo-info.html
-- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
