Re: [RFC v5 PATCH 7/9] xfs: buffer relogging support prototype

[Dave Chinner <david@fromorbit.com>]

On Thu, Feb 27, 2020 at 08:43:19AM -0500, Brian Foster wrote:
> Add a quick and dirty implementation of buffer relogging support.
> There is currently no use case for buffer relogging. This is for
> experimental use only and serves as an example to demonstrate the
> ability to relog arbitrary items in the future, if necessary.
> 
> Add a hook to enable relogging a buffer in a transaction, update the
> buffer log item handlers to support relogged BLIs and update the
> relog handler to join the relogged buffer to the relog transaction.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
.....
>  /*
> @@ -187,9 +188,21 @@ xfs_ail_relog(
>  			xfs_log_ticket_put(ailp->ail_relog_tic);
>  		spin_unlock(&ailp->ail_lock);
>  
> -		xfs_trans_add_item(tp, lip);
> -		set_bit(XFS_LI_DIRTY, &lip->li_flags);
> -		tp->t_flags |= XFS_TRANS_DIRTY;
> +		/*
> +		 * TODO: Ideally, relog transaction management would be pushed
> +		 * down into the ->iop_push() callbacks rather than playing
> +		 * games with ->li_trans and looking at log item types here.
> +		 */
> +		if (lip->li_type == XFS_LI_BUF) {
> +			struct xfs_buf_log_item	*bli = (struct xfs_buf_log_item *) lip;
> +			xfs_buf_hold(bli->bli_buf);

What is this for? The bli already has a reference to the buffer.

> +			xfs_trans_bjoin(tp, bli->bli_buf);
> +			xfs_trans_dirty_buf(tp, bli->bli_buf);
> +		} else {
> +			xfs_trans_add_item(tp, lip);
> +			set_bit(XFS_LI_DIRTY, &lip->li_flags);
> +			tp->t_flags |= XFS_TRANS_DIRTY;
> +		}

Really, this should be a xfs_item_ops callout. i.e.

		lip->li_ops->iop_relog(lip);

And then a) it doesn't matter really where we call it from, and b)
it becomes fully generic and we can implement the callout
as future functionality requires.

However, we have to make sure that the current transaction we are
running has the correct space usage accounted to it, so I think this
callout really does need to be done in a tight loop iterating and
accounting all the relog items into the transaction without outside
interference.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com