diff mbox series

[v1,09/17] xfs: extent transaction reservations for parent attributes

Message ID 20220611094200.129502-10-allison.henderson@oracle.com (mailing list archive)
State Deferred, archived
Headers show
Series Return of the Parent Pointers | expand

Commit Message

Allison Henderson June 11, 2022, 9:41 a.m. UTC 
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(-)

Comments

Dave Chinner June 16, 2022, 5:38 a.m. UTC | #1 
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 */
> +}

Just noticed this in passing - I have not looked at the reservation
calculations at all yet. We don't use "xfs_sb_version" feature
checks anymore. This goes into xfs_mount.h as a feature flag and we
use xfs_has_parent_pointers(mp) to check for the feature rather that
superblock bits.

Cheers,

Dave.
Allison Henderson June 18, 2022, 12:31 a.m. UTC | #2 
On Thu, 2022-06-16 at 15:38 +1000, Dave Chinner wrote:
> 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 */
> > +}
> 
> Just noticed this in passing - I have not looked at the reservation
> calculations at all yet. We don't use "xfs_sb_version" feature
> checks anymore. This goes into xfs_mount.h as a feature flag and we
> use xfs_has_parent_pointers(mp) to check for the feature rather that
> superblock bits.
Sure, will move.  Thx for the reviews!

Allison
> 
> Cheers,
> 
> Dave.
Darrick J. Wong June 29, 2022, 6:37 p.m. UTC | #3 
> 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
>
Allison Henderson June 29, 2022, 7:23 p.m. UTC | #4 
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@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?

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
> >
diff mbox series

Patch

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)
+{
+	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__ */