From: "Darrick J. Wong" <djwong@kernel.org>
To: Allison Henderson <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH RESEND v2 15/18] xfs: Add parent pointers to rename
Date: Tue, 9 Aug 2022 11:49:55 -0700 [thread overview]
Message-ID: <YvKsUyj3Ob9lqFYh@magnolia> (raw)
In-Reply-To: <20220804194013.99237-16-allison.henderson@oracle.com>
On Thu, Aug 04, 2022 at 12:40:10PM -0700, Allison Henderson wrote:
> This patch removes the old parent pointer attribute during the rename
> operation, and re-adds the updated parent pointer. In the case of
> xfs_cross_rename, we modify the routine not to roll the transaction just
> yet. We will do this after the parent pointer is added in the calling
> xfs_rename function.
>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> fs/xfs/xfs_inode.c | 128 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 94 insertions(+), 34 deletions(-)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 69bb67f2a252..8a81b78b6dd7 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2776,7 +2776,7 @@ xfs_cross_rename(
> }
> xfs_trans_ichgtime(tp, dp1, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> xfs_trans_log_inode(tp, dp1, XFS_ILOG_CORE);
> - return xfs_finish_rename(tp);
> + return 0;
>
> out_trans_abort:
> xfs_trans_cancel(tp);
> @@ -2834,26 +2834,31 @@ xfs_rename_alloc_whiteout(
> */
> int
> xfs_rename(
> - struct user_namespace *mnt_userns,
> - struct xfs_inode *src_dp,
> - struct xfs_name *src_name,
> - struct xfs_inode *src_ip,
> - struct xfs_inode *target_dp,
> - struct xfs_name *target_name,
> - struct xfs_inode *target_ip,
> - unsigned int flags)
> + struct user_namespace *mnt_userns,
> + struct xfs_inode *src_dp,
> + struct xfs_name *src_name,
> + struct xfs_inode *src_ip,
> + struct xfs_inode *target_dp,
> + struct xfs_name *target_name,
> + struct xfs_inode *target_ip,
> + unsigned int flags)
> {
> - struct xfs_mount *mp = src_dp->i_mount;
> - struct xfs_trans *tp;
> - struct xfs_inode *wip = NULL; /* whiteout inode */
> - struct xfs_inode *inodes[__XFS_SORT_INODES];
> - int i;
> - int num_inodes = __XFS_SORT_INODES;
> - bool new_parent = (src_dp != target_dp);
> - bool src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
> - int spaceres;
> - bool retried = false;
> - int error, nospace_error = 0;
> + struct xfs_mount *mp = src_dp->i_mount;
> + struct xfs_trans *tp;
> + struct xfs_inode *wip = NULL; /* whiteout inode */
> + struct xfs_inode *inodes[__XFS_SORT_INODES];
> + int i;
> + int num_inodes = __XFS_SORT_INODES;
> + bool new_parent = (src_dp != target_dp);
> + bool src_is_directory = S_ISDIR(VFS_I(src_ip)->i_mode);
> + int spaceres;
> + bool retried = false;
> + int error, nospace_error = 0;
> + xfs_dir2_dataptr_t new_diroffset;
> + xfs_dir2_dataptr_t old_diroffset;
> + struct xfs_parent_defer *old_parent_ptr = NULL;
> + struct xfs_parent_defer *new_parent_ptr = NULL;
> + struct xfs_parent_defer *target_parent_ptr = NULL;
>
> trace_xfs_rename(src_dp, target_dp, src_name, target_name);
>
> @@ -2877,6 +2882,15 @@ xfs_rename(
>
> xfs_sort_for_rename(src_dp, target_dp, src_ip, target_ip, wip,
> inodes, &num_inodes);
> + if (xfs_has_parent(mp)) {
> + error = xfs_parent_init(mp, src_ip, NULL, &old_parent_ptr);
> + if (error)
> + goto out_release_wip;
> + error = xfs_parent_init(mp, src_ip, target_name,
> + &new_parent_ptr);
> + if (error)
> + goto out_release_wip;
> + }
>
> retry:
> nospace_error = 0;
> @@ -2889,7 +2903,7 @@ xfs_rename(
> &tp);
> }
> if (error)
> - goto out_release_wip;
> + goto drop_incompat;
>
> /*
> * Attach the dquots to the inodes
> @@ -2911,14 +2925,14 @@ xfs_rename(
> * we can rely on either trans_commit or trans_cancel to unlock
> * them.
> */
> - xfs_trans_ijoin(tp, src_dp, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, src_dp, 0);
> if (new_parent)
> - xfs_trans_ijoin(tp, target_dp, XFS_ILOCK_EXCL);
> - xfs_trans_ijoin(tp, src_ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, target_dp, 0);
> + xfs_trans_ijoin(tp, src_ip, 0);
> if (target_ip)
> - xfs_trans_ijoin(tp, target_ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, target_ip, 0);
> if (wip)
> - xfs_trans_ijoin(tp, wip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, wip, 0);
>
> /*
> * If we are using project inheritance, we only allow renames
> @@ -2928,15 +2942,16 @@ xfs_rename(
> if (unlikely((target_dp->i_diflags & XFS_DIFLAG_PROJINHERIT) &&
> target_dp->i_projid != src_ip->i_projid)) {
> error = -EXDEV;
> - goto out_trans_cancel;
> + goto out_unlock;
> }
>
> /* RENAME_EXCHANGE is unique from here on. */
> - if (flags & RENAME_EXCHANGE)
> - return xfs_cross_rename(tp, src_dp, src_name, src_ip,
> + if (flags & RENAME_EXCHANGE) {
> + error = xfs_cross_rename(tp, src_dp, src_name, src_ip,
> target_dp, target_name, target_ip,
> spaceres);
> -
> + goto out_pptr;
> + }
> /*
> * Try to reserve quota to handle an expansion of the target directory.
> * We'll allow the rename to continue in reservationless mode if we hit
> @@ -3052,7 +3067,7 @@ xfs_rename(
> * to account for the ".." reference from the new entry.
> */
> error = xfs_dir_createname(tp, target_dp, target_name,
> - src_ip->i_ino, spaceres, NULL);
> + src_ip->i_ino, spaceres, &new_diroffset);
> if (error)
> goto out_trans_cancel;
>
> @@ -3073,10 +3088,14 @@ xfs_rename(
> * name at the destination directory, remove it first.
> */
> error = xfs_dir_replace(tp, target_dp, target_name,
> - src_ip->i_ino, spaceres, NULL);
> + src_ip->i_ino, spaceres, &new_diroffset);
> if (error)
> goto out_trans_cancel;
>
> + if (xfs_has_parent(mp))
> + error = xfs_parent_init(mp, target_ip, NULL,
> + &target_parent_ptr);
> +
> xfs_trans_ichgtime(tp, target_dp,
> XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
>
> @@ -3146,26 +3165,67 @@ xfs_rename(
> */
> if (wip)
> error = xfs_dir_replace(tp, src_dp, src_name, wip->i_ino,
> - spaceres, NULL);
> + spaceres, &old_diroffset);
> else
> error = xfs_dir_removename(tp, src_dp, src_name, src_ip->i_ino,
> - spaceres, NULL);
> + spaceres, &old_diroffset);
>
> if (error)
> goto out_trans_cancel;
>
> +out_pptr:
> + if (new_parent_ptr) {
> + error = xfs_parent_defer_add(tp, target_dp, new_parent_ptr,
> + new_diroffset);
> + if (error)
> + goto out_trans_cancel;
> + }
> +
> + if (old_parent_ptr) {
> + error = xfs_parent_defer_remove(tp, src_dp, old_parent_ptr,
> + old_diroffset);
> + if (error)
> + goto out_trans_cancel;
> + }
> +
> + if (target_parent_ptr) {
> + error = xfs_parent_defer_remove(tp, target_dp,
> + target_parent_ptr,
> + new_diroffset);
> + if (error)
> + goto out_trans_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)
> xfs_trans_log_inode(tp, target_dp, XFS_ILOG_CORE);
>
> error = xfs_finish_rename(tp);
> +
> +out_unlock:
> if (wip)
> xfs_irele(wip);
> + if (wip)
> + xfs_iunlock(wip, XFS_ILOCK_EXCL);
> + if (target_ip)
> + xfs_iunlock(target_ip, XFS_ILOCK_EXCL);
> + xfs_iunlock(src_ip, XFS_ILOCK_EXCL);
> + if (new_parent)
> + xfs_iunlock(target_dp, XFS_ILOCK_EXCL);
> + xfs_iunlock(src_dp, XFS_ILOCK_EXCL);
Sorry to be fussy, but could you separate the ILOCK unlocking changes
(and maybe the variable indentation part too) into a separate prep
patch, please?
Also, who frees the xfs_parent_defer objects?
--D
> +
> return error;
>
> out_trans_cancel:
> xfs_trans_cancel(tp);
> +drop_incompat:
> + if (new_parent_ptr)
> + xfs_parent_cancel(mp, new_parent_ptr);
> + if (old_parent_ptr)
> + xfs_parent_cancel(mp, old_parent_ptr);
> + if (target_parent_ptr)
> + xfs_parent_cancel(mp, target_parent_ptr);
> out_release_wip:
> if (wip)
> xfs_irele(wip);
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-08-09 19:07 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-04 19:39 [PATCH RESEND v2 00/18] Parent Pointers Allison Henderson
2022-08-04 19:39 ` [PATCH RESEND v2 01/18] xfs: Fix multi-transaction larp replay Allison Henderson
2022-08-09 16:52 ` Darrick J. Wong
2022-08-10 1:58 ` Dave Chinner
2022-08-10 5:01 ` Alli
2022-08-10 6:12 ` Dave Chinner
2022-08-10 15:52 ` Darrick J. Wong
2022-08-10 19:28 ` Alli
2022-08-12 1:55 ` Alli
2022-08-12 3:05 ` Darrick J. Wong
2022-08-16 0:54 ` Dave Chinner
2022-08-16 5:07 ` Darrick J. Wong
2022-08-16 20:41 ` Alli
2022-08-19 1:05 ` Alli
2022-08-23 15:07 ` Darrick J. Wong
2022-08-24 18:47 ` Alli
2022-08-10 3:08 ` Alli
2022-08-04 19:39 ` [PATCH RESEND v2 02/18] xfs: Increase XFS_DEFER_OPS_NR_INODES to 5 Allison Henderson
2022-08-09 16:38 ` Darrick J. Wong
2022-08-10 3:07 ` Alli
2022-08-04 19:39 ` [PATCH RESEND v2 03/18] xfs: Hold inode locks in xfs_ialloc Allison Henderson
2022-08-04 19:39 ` [PATCH RESEND v2 04/18] xfs: Hold inode locks in xfs_trans_alloc_dir Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 05/18] xfs: get directory offset when adding directory name Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 06/18] xfs: get directory offset when removing " Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 07/18] xfs: get directory offset when replacing a " Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 08/18] xfs: add parent pointer support to attribute code Allison Henderson
2022-08-09 16:54 ` Darrick J. Wong
2022-08-10 3:08 ` Alli
2022-08-04 19:40 ` [PATCH RESEND v2 09/18] xfs: define parent pointer xattr format Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 10/18] xfs: Add xfs_verify_pptr Allison Henderson
2022-08-09 16:59 ` Darrick J. Wong
2022-08-10 3:08 ` Alli
2022-08-04 19:40 ` [PATCH RESEND v2 11/18] xfs: extend transaction reservations for parent attributes Allison Henderson
2022-08-09 17:48 ` Darrick J. Wong
2022-08-10 3:08 ` Alli
2022-08-04 19:40 ` [PATCH RESEND v2 12/18] xfs: parent pointer attribute creation Allison Henderson
2022-08-09 18:01 ` Darrick J. Wong
2022-08-09 18:13 ` Darrick J. Wong
2022-08-10 3:09 ` Alli
2022-08-10 3:08 ` Alli
2022-08-04 19:40 ` [PATCH RESEND v2 13/18] xfs: add parent attributes to link Allison Henderson
2022-08-09 18:43 ` Darrick J. Wong
2022-08-10 3:09 ` Alli
2022-09-23 20:25 ` Darrick J. Wong
2022-08-04 19:40 ` [PATCH RESEND v2 14/18] xfs: remove parent pointers in unlink Allison Henderson
2022-08-09 18:45 ` Darrick J. Wong
2022-08-10 3:09 ` Alli
2022-08-04 19:40 ` [PATCH RESEND v2 15/18] xfs: Add parent pointers to rename Allison Henderson
2022-08-09 18:49 ` Darrick J. Wong [this message]
2022-08-10 3:09 ` Alli
2022-08-04 19:40 ` [PATCH RESEND v2 16/18] xfs: Add the parent pointer support to the superblock version 5 Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 17/18] xfs: Add helper function xfs_attr_list_context_init Allison Henderson
2022-08-04 19:40 ` [PATCH RESEND v2 18/18] xfs: Add parent pointer ioctl Allison Henderson
2022-08-09 19:26 ` Darrick J. Wong
2022-08-10 3:09 ` Alli
2022-09-24 0:01 ` Darrick J. Wong
2022-08-09 22:55 ` [RFC PATCH 19/18] xfs: fix unit conversion error in xfs_log_calc_max_attrsetm_res Darrick J. Wong
2022-08-09 22:56 ` [RFC PATCH 20/18] xfs: drop compatibility minimum log size computations for reflink Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YvKsUyj3Ob9lqFYh@magnolia \
--to=djwong@kernel.org \
--cc=allison.henderson@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox