Re: [PATCH 5/8] xfs: factor callbacks out of xlog_state_do_callback()

Dave Chinner
2019-09-05T22:17:59Z

On Thu, Sep 05, 2019 at 08:39:07AM -0700, Darrick J. Wong wrote:
> On Thu, Sep 05, 2019 at 06:47:14PM +1000, Dave Chinner wrote:
> > @@ -2795,31 +2831,13 @@ xlog_state_do_callback(
> > } else
> > ioerrors++;
> >
> > - spin_unlock(&log->l_icloglock);
> > -
> > /*
> > - * Keep processing entries in the callback list until
> > - * we come around and it is empty. We need to
> > - * atomically see that the list is empty and change the
> > - * state to DIRTY so that we don't miss any more
> > - * callbacks being added.
> > + * Running callbacks will drop the icloglock which means
> > + * we'll have to run at least one more complete loop.
> > */
> > - spin_lock(&iclog->ic_callback_lock);
> > - while (!list_empty(&iclog->ic_callbacks)) {
> > - LIST_HEAD(tmp);
> > + cycled_icloglock = true;
> > + xlog_state_do_iclog_callbacks(log, iclog, aborted);
> >
> > - list_splice_init(&iclog->ic_callbacks, &tmp);
> > -
> > - spin_unlock(&iclog->ic_callback_lock);
> > - xlog_cil_process_committed(&tmp, aborted);
> > - spin_lock(&iclog->ic_callback_lock);
> > - }
> > -
> > - loopdidcallbacks++;
> > - funcdidcallbacks++;
> > -
> > - spin_lock(&log->l_icloglock);
> > - spin_unlock(&iclog->ic_callback_lock);
> > if (!(iclog->ic_state & XLOG_STATE_IOERROR))
> > iclog->ic_state = XLOG_STATE_DIRTY;
> >
> > @@ -2835,6 +2853,8 @@ xlog_state_do_callback(
> > iclog = iclog->ic_next;
> > } while (first_iclog != iclog);
> >
> > + funcdidcallbacks += cycled_icloglock;
>
> funcdidcallbacks is effectively a yes/no state flag, so maybe it should
> be turned into a boolean and this statement becomes:
>
> funcdidcallbacks |= cycled_icloglock;

Fixed. I renamed it to did_callbacks at the same time, just to be a
little less eye-bleedy...

> Though I guess we're not at huge risk of integer overflow and it
> controls whether or not we run a debugging check so maybe we don't care?

All that really matters is we don't need a branch to calculate it :P

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com