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@xxxxxxxxxxxxx