From: "Darrick J. Wong" <djwong@kernel.org>
To: Allison Henderson <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH v1 09/17] xfs: extent transaction reservations for parent attributes
Date: Wed, 29 Jun 2022 11:37:21 -0700 [thread overview]
Message-ID: <Yryb4TYRKm+g0iBv@magnolia> (raw)
In-Reply-To: <20220611094200.129502-10-allison.henderson@oracle.com>
> Subject: xfs: extent transaction reservations for parent attributeso
s/extent/extend/?
On Sat, Jun 11, 2022 at 02:41:52AM -0700, Allison Henderson wrote:
> We need to add, remove or modify parent pointer attributes during
> create/link/unlink/rename operations atomically with the dirents in the
> parent directories being modified. This means they need to be modified
> in the same transaction as the parent directories, and so we need to add
> the required space for the attribute modifications to the transaction
> reservations.
>
> [achender: rebased, added xfs_sb_version_hasparent stub]
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
> ---
> fs/xfs/libxfs/xfs_format.h | 5 ++
> fs/xfs/libxfs/xfs_trans_resv.c | 103 +++++++++++++++++++++++++++------
> fs/xfs/libxfs/xfs_trans_resv.h | 1 +
> 3 files changed, 90 insertions(+), 19 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index afdfc8108c5f..96976497306c 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -390,6 +390,11 @@ xfs_sb_has_incompat_feature(
> return (sbp->sb_features_incompat & feature) != 0;
> }
>
> +static inline bool xfs_sb_version_hasparent(struct xfs_sb *sbp)
> +{
> + return false; /* We'll enable this at the end of the set */
> +}
> +
> #define XFS_SB_FEAT_INCOMPAT_LOG_XATTRS (1 << 0) /* Delayed Attributes */
> #define XFS_SB_FEAT_INCOMPAT_LOG_ALL \
> (XFS_SB_FEAT_INCOMPAT_LOG_XATTRS)
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index e9913c2c5a24..fbe46fd3b722 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -909,24 +909,30 @@ xfs_calc_sb_reservation(
> return xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
> }
>
> -void
> -xfs_trans_resv_calc(
> +/*
> + * Namespace reservations.
> + *
> + * These get tricky when parent pointers are enabled as we have attribute
> + * modifications occurring from within these transactions. Rather than confuse
> + * each of these reservation calculations with the conditional attribute
> + * reservations, add them here in a clear and concise manner. This assumes that
> + * the attribute reservations have already been calculated.
> + *
> + * Note that we only include the static attribute reservation here; the runtime
> + * reservation will have to be modified by the size of the attributes being
> + * added/removed/modified. See the comments on the attribute reservation
> + * calculations for more details.
> + *
> + * Note for rename: rename will vastly overestimate requirements. This will be
> + * addressed later when modifications are made to ensure parent attribute
> + * modifications can be done atomically with the rename operation.
> + */
> +STATIC void
> +xfs_calc_namespace_reservations(
> struct xfs_mount *mp,
> struct xfs_trans_resv *resp)
> {
> - int logcount_adj = 0;
> -
> - /*
> - * The following transactions are logged in physical format and
> - * require a permanent reservation on space.
> - */
> - resp->tr_write.tr_logres = xfs_calc_write_reservation(mp, false);
> - resp->tr_write.tr_logcount = XFS_WRITE_LOG_COUNT;
> - resp->tr_write.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> -
> - resp->tr_itruncate.tr_logres = xfs_calc_itruncate_reservation(mp, false);
> - resp->tr_itruncate.tr_logcount = XFS_ITRUNCATE_LOG_COUNT;
> - resp->tr_itruncate.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> + ASSERT(resp->tr_attrsetm.tr_logres > 0);
>
> resp->tr_rename.tr_logres = xfs_calc_rename_reservation(mp);
> resp->tr_rename.tr_logcount = XFS_RENAME_LOG_COUNT;
> @@ -948,15 +954,72 @@ xfs_trans_resv_calc(
> resp->tr_create.tr_logcount = XFS_CREATE_LOG_COUNT;
> resp->tr_create.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
>
> + resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp);
> + resp->tr_mkdir.tr_logcount = XFS_MKDIR_LOG_COUNT;
> + resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> +
> + xfs_calc_parent_ptr_reservations(mp);
> +}
> +
> +void xfs_calc_parent_ptr_reservations(struct xfs_mount *mp)
Indenting, etc.
Also vaguely wondering if this should be static inline? Or does
something else call this?
--D
> +{
> + struct xfs_trans_resv *resp = M_RES(mp);
> +
> + /* Calculate extra space needed for parent pointer attributes */
> + if (!xfs_sb_version_hasparent(&mp->m_sb))
> + return;
> +
> + /* rename can add/remove/modify 4 parent attributes */
> + resp->tr_rename.tr_logres += 4 * max(resp->tr_attrsetm.tr_logres,
> + resp->tr_attrrm.tr_logres);
> + resp->tr_rename.tr_logcount += 4 * max(resp->tr_attrsetm.tr_logcount,
> + resp->tr_attrrm.tr_logcount);
> +
> + /* create will add 1 parent attribute */
> + resp->tr_create.tr_logres += resp->tr_attrsetm.tr_logres;
> + resp->tr_create.tr_logcount += resp->tr_attrsetm.tr_logcount;
> +
> + /* mkdir will add 1 parent attribute */
> + resp->tr_mkdir.tr_logres += resp->tr_attrsetm.tr_logres;
> + resp->tr_mkdir.tr_logcount += resp->tr_attrsetm.tr_logcount;
> +
> + /* link will add 1 parent attribute */
> + resp->tr_link.tr_logres += resp->tr_attrsetm.tr_logres;
> + resp->tr_link.tr_logcount += resp->tr_attrsetm.tr_logcount;
> +
> + /* symlink will add 1 parent attribute */
> + resp->tr_symlink.tr_logres += resp->tr_attrsetm.tr_logres;
> + resp->tr_symlink.tr_logcount += resp->tr_attrsetm.tr_logcount;
> +
> + /* remove will remove 1 parent attribute */
> + resp->tr_remove.tr_logres += resp->tr_attrrm.tr_logres;
> + resp->tr_remove.tr_logcount += resp->tr_attrrm.tr_logcount;
> +}
> +
> +void
> +xfs_trans_resv_calc(
> + struct xfs_mount *mp,
> + struct xfs_trans_resv *resp)
> +{
> + int logcount_adj = 0;
> +
> + /*
> + * The following transactions are logged in physical format and
> + * require a permanent reservation on space.
> + */
> + resp->tr_write.tr_logres = xfs_calc_write_reservation(mp, false);
> + resp->tr_write.tr_logcount = XFS_WRITE_LOG_COUNT;
> + resp->tr_write.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> +
> + resp->tr_itruncate.tr_logres = xfs_calc_itruncate_reservation(mp, false);
> + resp->tr_itruncate.tr_logcount = XFS_ITRUNCATE_LOG_COUNT;
> + resp->tr_itruncate.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> +
> resp->tr_create_tmpfile.tr_logres =
> xfs_calc_create_tmpfile_reservation(mp);
> resp->tr_create_tmpfile.tr_logcount = XFS_CREATE_TMPFILE_LOG_COUNT;
> resp->tr_create_tmpfile.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
>
> - resp->tr_mkdir.tr_logres = xfs_calc_mkdir_reservation(mp);
> - resp->tr_mkdir.tr_logcount = XFS_MKDIR_LOG_COUNT;
> - resp->tr_mkdir.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> -
> resp->tr_ifree.tr_logres = xfs_calc_ifree_reservation(mp);
> resp->tr_ifree.tr_logcount = XFS_INACTIVE_LOG_COUNT;
> resp->tr_ifree.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
> @@ -986,6 +1049,8 @@ xfs_trans_resv_calc(
> resp->tr_qm_dqalloc.tr_logcount = XFS_WRITE_LOG_COUNT;
> resp->tr_qm_dqalloc.tr_logflags |= XFS_TRANS_PERM_LOG_RES;
>
> + xfs_calc_namespace_reservations(mp, resp);
> +
> /*
> * The following transactions are logged in logical format with
> * a default log count.
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
> index 0554b9d775d2..cab8084a84d6 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.h
> +++ b/fs/xfs/libxfs/xfs_trans_resv.h
> @@ -101,5 +101,6 @@ uint xfs_allocfree_block_count(struct xfs_mount *mp, uint num_ops);
> unsigned int xfs_calc_itruncate_reservation_minlogsize(struct xfs_mount *mp);
> unsigned int xfs_calc_write_reservation_minlogsize(struct xfs_mount *mp);
> unsigned int xfs_calc_qm_dqalloc_reservation_minlogsize(struct xfs_mount *mp);
> +void xfs_calc_parent_ptr_reservations(struct xfs_mount *mp);
>
> #endif /* __XFS_TRANS_RESV_H__ */
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-06-29 18:37 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-11 9:41 [PATCH v1 00/17] Return of the Parent Pointers Allison Henderson
2022-06-11 9:41 ` [PATCH v1 01/17] xfs: Add larp state XFS_DAS_CREATE_FORK Allison Henderson
2022-06-15 1:09 ` Dave Chinner
2022-06-15 23:40 ` Alli
2022-06-16 2:08 ` Dave Chinner
2022-06-16 5:32 ` Dave Chinner
2022-06-29 6:33 ` Alli
2022-06-30 0:40 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 02/17] xfs: Hold inode locks in xfs_ialloc Allison Henderson
2022-06-29 18:28 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 03/17] xfs: get directory offset when adding directory name Allison Henderson
2022-06-29 18:29 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 04/17] xfs: get directory offset when removing " Allison Henderson
2022-06-29 18:30 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 05/17] xfs: get directory offset when replacing a " Allison Henderson
2022-06-29 18:30 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 06/17] xfs: add parent pointer support to attribute code Allison Henderson
2022-06-29 18:33 ` Darrick J. Wong
2022-06-29 18:58 ` Alli
2022-06-11 9:41 ` [PATCH v1 07/17] xfs: define parent pointer xattr format Allison Henderson
2022-06-29 18:34 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 08/17] xfs: Add xfs_verify_pptr Allison Henderson
2022-06-29 18:35 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 09/17] xfs: extent transaction reservations for parent attributes Allison Henderson
2022-06-16 5:38 ` Dave Chinner
2022-06-18 0:31 ` Alli
2022-06-29 18:37 ` Darrick J. Wong [this message]
2022-06-29 19:23 ` Alli
2022-06-11 9:41 ` [PATCH v1 10/17] xfs: parent pointer attribute creation Allison Henderson
2022-06-16 5:49 ` Dave Chinner
2022-06-18 0:32 ` Alli
2022-06-29 18:41 ` Darrick J. Wong
2022-06-30 1:29 ` Alli
2022-06-11 9:41 ` [PATCH v1 11/17] xfs: add parent attributes to link Allison Henderson
2022-06-16 22:39 ` Dave Chinner
2022-06-18 0:32 ` Alli
2022-06-29 18:09 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 12/17] xfs: remove parent pointers in unlink Allison Henderson
2022-06-29 17:35 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 13/17] xfs: Add parent pointers to rename Allison Henderson
2022-06-29 18:02 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 14/17] xfs: Add the parent pointer support to the superblock version 5 Allison Henderson
2022-06-16 6:03 ` Dave Chinner
2022-06-18 0:32 ` Alli
2022-06-20 0:21 ` Dave Chinner
2022-06-29 18:16 ` Darrick J. Wong
2022-06-11 9:41 ` [PATCH v1 15/17] xfs: Add helper function xfs_attr_list_context_init Allison Henderson
2022-06-29 18:42 ` Darrick J. Wong
2022-06-30 1:30 ` Alli
2022-06-11 9:41 ` [PATCH v1 16/17] xfs: Increase XFS_DEFER_OPS_NR_INODES to 4 Allison Henderson
2022-06-16 21:54 ` Dave Chinner
2022-06-18 0:32 ` Alli
2022-06-29 18:43 ` Darrick J. Wong
2022-06-30 1:30 ` Alli
2022-06-11 9:42 ` [PATCH v1 17/17] xfs: Add parent pointer ioctl Allison Henderson
2022-06-29 18:52 ` Darrick J. Wong
2022-06-30 1:30 ` Alli
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=Yryb4TYRKm+g0iBv@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