> + * Note that SYNCING|IOABORT is a valid state so we cannot just check for > + * ic_state == SYNCING. I've removed the IOABORT flag recently, but it seems I forgot to remove this comment. That beeing said the IOERR flag is still a complete mess as we sometimes use it as a flag and sometimes as a separate state. I've been wanting to sort that out, but always got preempted. > + */ > +static void > +xlog_state_callback_check_state( > + struct xlog *log, > + int did_callbacks) > +{ > + struct xlog_in_core *iclog; > + struct xlog_in_core *first_iclog; > + > + if (!did_callbacks) > + return; > + > + first_iclog = iclog = log->l_iclog; I'd keep the did_callbacks check in the caller. For the non-debug case it will be optimized away, but it saves an argument, and allows initializing the iclog variables on the declaration line. > + do { > + ASSERT(iclog->ic_state != XLOG_STATE_DO_CALLBACK); > + /* > + * Terminate the loop if iclogs are found in states > + * which will cause other threads to clean up iclogs. > + * > + * SYNCING - i/o completion will go through logs > + * DONE_SYNC - interrupt thread should be waiting for > + * l_icloglock > + * IOERROR - give up hope all ye who enter here > + */ > + if (iclog->ic_state == XLOG_STATE_WANT_SYNC || > + iclog->ic_state & XLOG_STATE_SYNCING || > + iclog->ic_state == XLOG_STATE_DONE_SYNC || > + iclog->ic_state == XLOG_STATE_IOERROR ) > + break; > + iclog = iclog->ic_next; No new, but if we list the states we should not miss one..