On Tue, Jun 22, 2021 at 02:06:01PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> It's completely unnecessary because callbacks are added to iclogs
> without holding the icloglock, hence no amount of ordering between
> the icloglock and ic_callback_lock will order the removal of
> callbacks from the iclog.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Seems reasonable so far -- we don't tangle the icloglock with the
callback lock anywhere like this.
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_log.c | 18 ++++--------------
> 1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index e93cac6b5378..bb4390942275 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2773,11 +2773,8 @@ static void
> xlog_state_do_iclog_callbacks(
> struct xlog *log,
> struct xlog_in_core *iclog)
> - __releases(&log->l_icloglock)
> - __acquires(&log->l_icloglock)
> {
> trace_xlog_iclog_callbacks_start(iclog, _RET_IP_);
> - spin_unlock(&log->l_icloglock);
> spin_lock(&iclog->ic_callback_lock);
> while (!list_empty(&iclog->ic_callbacks)) {
> LIST_HEAD(tmp);
> @@ -2789,12 +2786,6 @@ xlog_state_do_iclog_callbacks(
> spin_lock(&iclog->ic_callback_lock);
> }
>
> - /*
> - * Pick up the icloglock while still holding the callback lock so we
> - * serialise against anyone trying to add more callbacks to this iclog
> - * now we've finished processing.
> - */
> - spin_lock(&log->l_icloglock);
> spin_unlock(&iclog->ic_callback_lock);
> trace_xlog_iclog_callbacks_done(iclog, _RET_IP_);
> }
> @@ -2836,13 +2827,12 @@ xlog_state_do_callback(
> iclog = iclog->ic_next;
> continue;
> }
> + spin_unlock(&log->l_icloglock);
>
> - /*
> - * Running callbacks will drop the icloglock which means
> - * we'll have to run at least one more complete loop.
> - */
> - cycled_icloglock = true;
> xlog_state_do_iclog_callbacks(log, iclog);
> + cycled_icloglock = true;
> +
> + spin_lock(&log->l_icloglock);
> if (XLOG_FORCED_SHUTDOWN(log))
> wake_up_all(&iclog->ic_force_wait);
> else
> --
> 2.31.1
>
