On Wed, 2022-06-29 at 11:37 -0700, Darrick J. Wong wrote:
> > Subject: xfs: extent transaction reservations for parent
> > attributeso
>
> s/extent/extend/?
Yes, will fix
>
>
> 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@xxxxxxxxxx>
> > Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> > ---
> > 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?
Yes, I think it can be made static, nothing else needs to call it. Thx
for the review!
Allison
>
> --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
> >
