| Message ID | 20200316144233.900390-11-hch@lst.de (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [01/14] xfs: merge xlog_cil_push into xlog_cil_push_work | expand |
On Mon, Mar 16, 2020 at 03:42:29PM +0100, Christoph Hellwig wrote: > Move all state checks into the caller to make the loop flow more clear, > and instead move the callback processing together with marking the iclog > for callbacks. > > This also allows to easily indicate when we actually dropped the > icloglock instead of assuming we do so for any iclog processed. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Ok, I think this is a pretty straightfoward hoisting of the the state checksk, as promised... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_log.c | 82 ++++++++++++++++++++++-------------------------- > 1 file changed, 37 insertions(+), 45 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index 4efaa248a03d..a38d495b6e81 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -2738,47 +2738,28 @@ xlog_state_do_iclog_callbacks( > spin_unlock(&iclog->ic_callback_lock); > } > > -/* > - * Return true if we need to stop processing, false to continue to the next > - * iclog. The caller will need to run callbacks if the iclog is returned in the > - * XLOG_STATE_CALLBACK state. > - */ > static bool > xlog_state_iodone_process_iclog( > struct xlog *log, > struct xlog_in_core *iclog) > { > - xfs_lsn_t lowest_lsn; > - xfs_lsn_t header_lsn; > + xfs_lsn_t header_lsn, lowest_lsn; > > - switch (iclog->ic_state) { > - case XLOG_STATE_ACTIVE: > - case XLOG_STATE_DIRTY: > - /* > - * Skip all iclogs in the ACTIVE & DIRTY states: > - */ > - return false; > - case XLOG_STATE_DONE_SYNC: > - /* > - * Now that we have an iclog that is in the DONE_SYNC state, do > - * one more check here to see if we have chased our tail around. > - * If this is not the lowest lsn iclog, then we will leave it > - * for another completion to process. > - */ > - header_lsn = be64_to_cpu(iclog->ic_header.h_lsn); > - lowest_lsn = xlog_get_lowest_lsn(log); > - if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0) > - return false; > - xlog_state_set_callback(log, iclog, header_lsn); > + /* > + * Now that we have an iclog that is in the DONE_SYNC state, do one more > + * check here to see if we have chased our tail around. If this is not > + * the lowest lsn iclog, then we will leave it for another completion to > + * process. > + */ > + header_lsn = be64_to_cpu(iclog->ic_header.h_lsn); > + lowest_lsn = xlog_get_lowest_lsn(log); > + if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0) > return false; > - default: > - /* > - * Can only perform callbacks in order. Since this iclog is not > - * in the DONE_SYNC state, we skip the rest and just try to > - * clean up. > - */ > - return true; > - } > + > + xlog_state_set_callback(log, iclog, header_lsn); > + xlog_state_do_iclog_callbacks(log, iclog); > + xlog_state_clean_iclog(log, iclog); > + return true; > } > > STATIC void > @@ -2795,10 +2776,6 @@ xlog_state_do_callback( > * > * Keep looping through iclogs until one full pass is made without > * running any callbacks. > - * > - * If the log has been shut down, still perform the callbacks once per > - * iclog to abort all log items, but don't bother to restart the loop > - * after dropping the log as no new callbacks can show up. > */ > spin_lock(&log->l_icloglock); > do { > @@ -2809,25 +2786,40 @@ xlog_state_do_callback( > repeats++; > > do { > + /* > + * If the log has been shut down, still perform the > + * callbacks to abort all log items to clean up any > + * allocate resource, but don't bother to restart the > + * loop after dropping the log as no new callbacks can > + * be attached now. > + */ > if (XLOG_FORCED_SHUTDOWN(log)) { > xlog_state_do_iclog_callbacks(log, iclog); > wake_up_all(&iclog->ic_force_wait); > continue; > } > > - if (xlog_state_iodone_process_iclog(log, iclog)) > - break; > - > - if (iclog->ic_state != XLOG_STATE_CALLBACK) > + /* > + * Skip all iclogs in the ACTIVE & DIRTY states: > + */ > + if (iclog->ic_state == XLOG_STATE_ACTIVE || > + iclog->ic_state == XLOG_STATE_DIRTY) > continue; > > + /* > + * We can only perform callbacks in order. If this > + * iclog is not in the DONE_SYNC state, we skip the rest > + * and just try to clean up. > + */ > + if (iclog->ic_state != XLOG_STATE_DONE_SYNC) > + break; > + > /* > * 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); > - xlog_state_clean_iclog(log, iclog); > + if (xlog_state_iodone_process_iclog(log, iclog)) > + cycled_icloglock = true; > } while ((iclog = iclog->ic_next) != first_iclog); > > if (repeats > 5000) { > -- > 2.24.1 >
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 4efaa248a03d..a38d495b6e81 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -2738,47 +2738,28 @@ xlog_state_do_iclog_callbacks( spin_unlock(&iclog->ic_callback_lock); } -/* - * Return true if we need to stop processing, false to continue to the next - * iclog. The caller will need to run callbacks if the iclog is returned in the - * XLOG_STATE_CALLBACK state. - */ static bool xlog_state_iodone_process_iclog( struct xlog *log, struct xlog_in_core *iclog) { - xfs_lsn_t lowest_lsn; - xfs_lsn_t header_lsn; + xfs_lsn_t header_lsn, lowest_lsn; - switch (iclog->ic_state) { - case XLOG_STATE_ACTIVE: - case XLOG_STATE_DIRTY: - /* - * Skip all iclogs in the ACTIVE & DIRTY states: - */ - return false; - case XLOG_STATE_DONE_SYNC: - /* - * Now that we have an iclog that is in the DONE_SYNC state, do - * one more check here to see if we have chased our tail around. - * If this is not the lowest lsn iclog, then we will leave it - * for another completion to process. - */ - header_lsn = be64_to_cpu(iclog->ic_header.h_lsn); - lowest_lsn = xlog_get_lowest_lsn(log); - if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0) - return false; - xlog_state_set_callback(log, iclog, header_lsn); + /* + * Now that we have an iclog that is in the DONE_SYNC state, do one more + * check here to see if we have chased our tail around. If this is not + * the lowest lsn iclog, then we will leave it for another completion to + * process. + */ + header_lsn = be64_to_cpu(iclog->ic_header.h_lsn); + lowest_lsn = xlog_get_lowest_lsn(log); + if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0) return false; - default: - /* - * Can only perform callbacks in order. Since this iclog is not - * in the DONE_SYNC state, we skip the rest and just try to - * clean up. - */ - return true; - } + + xlog_state_set_callback(log, iclog, header_lsn); + xlog_state_do_iclog_callbacks(log, iclog); + xlog_state_clean_iclog(log, iclog); + return true; } STATIC void @@ -2795,10 +2776,6 @@ xlog_state_do_callback( * * Keep looping through iclogs until one full pass is made without * running any callbacks. - * - * If the log has been shut down, still perform the callbacks once per - * iclog to abort all log items, but don't bother to restart the loop - * after dropping the log as no new callbacks can show up. */ spin_lock(&log->l_icloglock); do { @@ -2809,25 +2786,40 @@ xlog_state_do_callback( repeats++; do { + /* + * If the log has been shut down, still perform the + * callbacks to abort all log items to clean up any + * allocate resource, but don't bother to restart the + * loop after dropping the log as no new callbacks can + * be attached now. + */ if (XLOG_FORCED_SHUTDOWN(log)) { xlog_state_do_iclog_callbacks(log, iclog); wake_up_all(&iclog->ic_force_wait); continue; } - if (xlog_state_iodone_process_iclog(log, iclog)) - break; - - if (iclog->ic_state != XLOG_STATE_CALLBACK) + /* + * Skip all iclogs in the ACTIVE & DIRTY states: + */ + if (iclog->ic_state == XLOG_STATE_ACTIVE || + iclog->ic_state == XLOG_STATE_DIRTY) continue; + /* + * We can only perform callbacks in order. If this + * iclog is not in the DONE_SYNC state, we skip the rest + * and just try to clean up. + */ + if (iclog->ic_state != XLOG_STATE_DONE_SYNC) + break; + /* * 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); - xlog_state_clean_iclog(log, iclog); + if (xlog_state_iodone_process_iclog(log, iclog)) + cycled_icloglock = true; } while ((iclog = iclog->ic_next) != first_iclog); if (repeats > 5000) {
Move all state checks into the caller to make the loop flow more clear, and instead move the callback processing together with marking the iclog for callbacks. This also allows to easily indicate when we actually dropped the icloglock instead of assuming we do so for any iclog processed. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_log.c | 82 ++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 45 deletions(-)