On Mon, Jul 02, 2018 at 01:38:36PM -0400, Brian Foster wrote:
> Log recovery passes down a central dfops structure to recovery
> handlers for bui and cui log items. Each of these handlers allocates
> and commits a transaction and defers any remaining operations to be
> completed by the main recovery sequence.
>
> Since dfops outlives the transaction in this context, set and clear
> ->t_dfops appropriately such that the *_finish_item() paths and
> below (i.e., xfs_bmapi*()) can expect to find the dfops in the
> transaction without it being committed with the dfops attached. This
> is required because transaction commit expects that an associated
> dfops is finished and in this context the dfops may be populated at
> commit time.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> ---
>
> v2:
> - Add comments to explain unique ->t_dfops usage in bui/cui log item
> recovery.
>
> fs/xfs/xfs_bmap_item.c | 8 ++++++++
> fs/xfs/xfs_refcount_item.c | 8 ++++++++
General note: realtime rmap (being rooted in an inode) log item replay
can generate further dfops, so I'll have to wire that up when I pass the
log recovery dfops collector into xfs_rui_recover, and I'll have to
remember to attach it to the transaction when I do that.
Otherwise I think this is fine,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> 2 files changed, 16 insertions(+)
>
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 956ebd583e27..478bfc798861 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -441,6 +441,7 @@ xfs_bui_recover(
> XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp);
> if (error)
> return error;
> + tp->t_dfops = dfops;
> budp = xfs_trans_get_bud(tp, buip);
>
> /* Grab the inode. */
> @@ -487,6 +488,12 @@ xfs_bui_recover(
> }
>
> set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
> + /*
> + * Recovery finishes all deferred ops once intent processing is
> + * complete. Reset the trans reference because commit expects a finished
> + * dfops or none at all.
> + */
> + tp->t_dfops = NULL;
> error = xfs_trans_commit(tp);
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> IRELE(ip);
> @@ -494,6 +501,7 @@ xfs_bui_recover(
> return error;
>
> err_inode:
> + tp->t_dfops = NULL;
> xfs_trans_cancel(tp);
> if (ip) {
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index 472a73e9d331..2064c689bc72 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -452,6 +452,7 @@ xfs_cui_recover(
> mp->m_refc_maxlevels * 2, 0, XFS_TRANS_RESERVE, &tp);
> if (error)
> return error;
> + tp->t_dfops = dfops;
> cudp = xfs_trans_get_cud(tp, cuip);
>
> for (i = 0; i < cuip->cui_format.cui_nextents; i++) {
> @@ -514,11 +515,18 @@ xfs_cui_recover(
>
> xfs_refcount_finish_one_cleanup(tp, rcur, error);
> set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
> + /*
> + * Recovery finishes all deferred ops once intent processing is
> + * complete. Reset the trans reference because commit expects a finished
> + * dfops or none at all.
> + */
> + tp->t_dfops = NULL;
> error = xfs_trans_commit(tp);
> return error;
>
> abort_error:
> xfs_refcount_finish_one_cleanup(tp, rcur, error);
> + tp->t_dfops = NULL;
> xfs_trans_cancel(tp);
> return error;
> }
> --
> 2.17.1
>
> --
> 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
--
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
