| Message ID | 20200316144233.900390-14-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/14] xfs: merge xlog_cil_push into xlog_cil_push_work | expand |
On Mon, Mar 16, 2020 at 03:42:32PM +0100, Christoph Hellwig wrote: > Open code the xlog_state_want_sync logic in its two callers given that > this function is a trivial wrapper around xlog_state_switch_iclogs. > > Move the lockdep assert into xlog_state_switch_iclogs to not lose this > debugging aid, and improve the comment that documents > xlog_state_switch_iclogs as well. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_log.c | 50 +++++++++++++++++------------------------------- > 1 file changed, 18 insertions(+), 32 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index 865dd1e08679..761b138d97ec 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -62,11 +62,6 @@ xlog_state_switch_iclogs( > struct xlog_in_core *iclog, > int eventual_size); > STATIC void > -xlog_state_want_sync( > - struct xlog *log, > - struct xlog_in_core *iclog); > - > -STATIC void > xlog_grant_push_ail( > struct xlog *log, > int need_bytes); > @@ -938,7 +933,11 @@ xfs_log_write_unmount_record( > spin_lock(&log->l_icloglock); > iclog = log->l_iclog; > atomic_inc(&iclog->ic_refcnt); > - xlog_state_want_sync(log, iclog); > + if (iclog->ic_state == XLOG_STATE_ACTIVE) > + xlog_state_switch_iclogs(log, iclog, 0); > + else > + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || > + iclog->ic_state == XLOG_STATE_IOERROR); > error = xlog_state_release_iclog(log, iclog); > xlog_wait_on_iclog(iclog); > > @@ -2293,7 +2292,11 @@ xlog_write_copy_finish( > *record_cnt = 0; > *data_cnt = 0; > > - xlog_state_want_sync(log, iclog); > + if (iclog->ic_state == XLOG_STATE_ACTIVE) > + xlog_state_switch_iclogs(log, iclog, 0); > + else > + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || > + iclog->ic_state == XLOG_STATE_IOERROR); > if (!commit_iclog) > goto release_iclog; > spin_unlock(&log->l_icloglock); > @@ -3069,11 +3072,12 @@ xlog_ungrant_log_space( > } > > /* > - * This routine will mark the current iclog in the ring as WANT_SYNC > - * and move the current iclog pointer to the next iclog in the ring. > - * When this routine is called from xlog_state_get_iclog_space(), the > - * exact size of the iclog has not yet been determined. All we know is > - * that every data block. We have run out of space in this log record. > + * Mark the current iclog in the ring as WANT_SYNC and move the current iclog > + * pointer to the next iclog in the ring. > + * > + * When called from xlog_state_get_iclog_space(), the exact size of the iclog > + * has not yet been determined, all we know is that we have run out of space in > + * the current iclog. > */ > STATIC void > xlog_state_switch_iclogs( > @@ -3082,6 +3086,8 @@ xlog_state_switch_iclogs( > int eventual_size) > { > ASSERT(iclog->ic_state == XLOG_STATE_ACTIVE); > + assert_spin_locked(&log->l_icloglock); > + > if (!eventual_size) > eventual_size = iclog->ic_offset; > iclog->ic_state = XLOG_STATE_WANT_SYNC; > @@ -3323,26 +3329,6 @@ xfs_log_force_lsn( > return ret; > } > > -/* > - * Called when we want to mark the current iclog as being ready to sync to > - * disk. > - */ > -STATIC void > -xlog_state_want_sync( > - struct xlog *log, > - struct xlog_in_core *iclog) > -{ > - assert_spin_locked(&log->l_icloglock); > - > - if (iclog->ic_state == XLOG_STATE_ACTIVE) { > - xlog_state_switch_iclogs(log, iclog, 0); > - } else { > - ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || > - iclog->ic_state == XLOG_STATE_IOERROR); > - } > -} > - > - > /***************************************************************************** > * > * TICKET functions > -- > 2.24.1 >
On Mon, Mar 16, 2020 at 03:42:32PM +0100, Christoph Hellwig wrote: > Open code the xlog_state_want_sync logic in its two callers given that > this function is a trivial wrapper around xlog_state_switch_iclogs. > > Move the lockdep assert into xlog_state_switch_iclogs to not lose this > debugging aid, and improve the comment that documents > xlog_state_switch_iclogs as well. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/xfs_log.c | 50 +++++++++++++++++------------------------------- > 1 file changed, 18 insertions(+), 32 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index 865dd1e08679..761b138d97ec 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -62,11 +62,6 @@ xlog_state_switch_iclogs( > struct xlog_in_core *iclog, > int eventual_size); > STATIC void > -xlog_state_want_sync( > - struct xlog *log, > - struct xlog_in_core *iclog); > - > -STATIC void > xlog_grant_push_ail( > struct xlog *log, > int need_bytes); > @@ -938,7 +933,11 @@ xfs_log_write_unmount_record( > spin_lock(&log->l_icloglock); > iclog = log->l_iclog; > atomic_inc(&iclog->ic_refcnt); > - xlog_state_want_sync(log, iclog); > + if (iclog->ic_state == XLOG_STATE_ACTIVE) > + xlog_state_switch_iclogs(log, iclog, 0); > + else > + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || > + iclog->ic_state == XLOG_STATE_IOERROR); > error = xlog_state_release_iclog(log, iclog); > xlog_wait_on_iclog(iclog); > > @@ -2293,7 +2292,11 @@ xlog_write_copy_finish( > *record_cnt = 0; > *data_cnt = 0; > > - xlog_state_want_sync(log, iclog); > + if (iclog->ic_state == XLOG_STATE_ACTIVE) > + xlog_state_switch_iclogs(log, iclog, 0); > + else > + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || > + iclog->ic_state == XLOG_STATE_IOERROR); > if (!commit_iclog) > goto release_iclog; > spin_unlock(&log->l_icloglock); > @@ -3069,11 +3072,12 @@ xlog_ungrant_log_space( > } > > /* > - * This routine will mark the current iclog in the ring as WANT_SYNC > - * and move the current iclog pointer to the next iclog in the ring. > - * When this routine is called from xlog_state_get_iclog_space(), the > - * exact size of the iclog has not yet been determined. All we know is > - * that every data block. We have run out of space in this log record. > + * Mark the current iclog in the ring as WANT_SYNC and move the current iclog > + * pointer to the next iclog in the ring. > + * > + * When called from xlog_state_get_iclog_space(), the exact size of the iclog > + * has not yet been determined, all we know is that we have run out of space in > + * the current iclog. > */ > STATIC void > xlog_state_switch_iclogs( > @@ -3082,6 +3086,8 @@ xlog_state_switch_iclogs( > int eventual_size) > { > ASSERT(iclog->ic_state == XLOG_STATE_ACTIVE); > + assert_spin_locked(&log->l_icloglock); > + > if (!eventual_size) > eventual_size = iclog->ic_offset; > iclog->ic_state = XLOG_STATE_WANT_SYNC; > @@ -3323,26 +3329,6 @@ xfs_log_force_lsn( > return ret; > } > > -/* > - * Called when we want to mark the current iclog as being ready to sync to > - * disk. > - */ > -STATIC void > -xlog_state_want_sync( > - struct xlog *log, > - struct xlog_in_core *iclog) > -{ > - assert_spin_locked(&log->l_icloglock); > - > - if (iclog->ic_state == XLOG_STATE_ACTIVE) { > - xlog_state_switch_iclogs(log, iclog, 0); > - } else { > - ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || > - iclog->ic_state == XLOG_STATE_IOERROR); > - } > -} > - > - > /***************************************************************************** > * > * TICKET functions > -- > 2.24.1 >
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 865dd1e08679..761b138d97ec 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -62,11 +62,6 @@ xlog_state_switch_iclogs( struct xlog_in_core *iclog, int eventual_size); STATIC void -xlog_state_want_sync( - struct xlog *log, - struct xlog_in_core *iclog); - -STATIC void xlog_grant_push_ail( struct xlog *log, int need_bytes); @@ -938,7 +933,11 @@ xfs_log_write_unmount_record( spin_lock(&log->l_icloglock); iclog = log->l_iclog; atomic_inc(&iclog->ic_refcnt); - xlog_state_want_sync(log, iclog); + if (iclog->ic_state == XLOG_STATE_ACTIVE) + xlog_state_switch_iclogs(log, iclog, 0); + else + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || + iclog->ic_state == XLOG_STATE_IOERROR); error = xlog_state_release_iclog(log, iclog); xlog_wait_on_iclog(iclog); @@ -2293,7 +2292,11 @@ xlog_write_copy_finish( *record_cnt = 0; *data_cnt = 0; - xlog_state_want_sync(log, iclog); + if (iclog->ic_state == XLOG_STATE_ACTIVE) + xlog_state_switch_iclogs(log, iclog, 0); + else + ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || + iclog->ic_state == XLOG_STATE_IOERROR); if (!commit_iclog) goto release_iclog; spin_unlock(&log->l_icloglock); @@ -3069,11 +3072,12 @@ xlog_ungrant_log_space( } /* - * This routine will mark the current iclog in the ring as WANT_SYNC - * and move the current iclog pointer to the next iclog in the ring. - * When this routine is called from xlog_state_get_iclog_space(), the - * exact size of the iclog has not yet been determined. All we know is - * that every data block. We have run out of space in this log record. + * Mark the current iclog in the ring as WANT_SYNC and move the current iclog + * pointer to the next iclog in the ring. + * + * When called from xlog_state_get_iclog_space(), the exact size of the iclog + * has not yet been determined, all we know is that we have run out of space in + * the current iclog. */ STATIC void xlog_state_switch_iclogs( @@ -3082,6 +3086,8 @@ xlog_state_switch_iclogs( int eventual_size) { ASSERT(iclog->ic_state == XLOG_STATE_ACTIVE); + assert_spin_locked(&log->l_icloglock); + if (!eventual_size) eventual_size = iclog->ic_offset; iclog->ic_state = XLOG_STATE_WANT_SYNC; @@ -3323,26 +3329,6 @@ xfs_log_force_lsn( return ret; } -/* - * Called when we want to mark the current iclog as being ready to sync to - * disk. - */ -STATIC void -xlog_state_want_sync( - struct xlog *log, - struct xlog_in_core *iclog) -{ - assert_spin_locked(&log->l_icloglock); - - if (iclog->ic_state == XLOG_STATE_ACTIVE) { - xlog_state_switch_iclogs(log, iclog, 0); - } else { - ASSERT(iclog->ic_state == XLOG_STATE_WANT_SYNC || - iclog->ic_state == XLOG_STATE_IOERROR); - } -} - - /***************************************************************************** * * TICKET functions
Open code the xlog_state_want_sync logic in its two callers given that this function is a trivial wrapper around xlog_state_switch_iclogs. Move the lockdep assert into xlog_state_switch_iclogs to not lose this debugging aid, and improve the comment that documents xlog_state_switch_iclogs as well. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_log.c | 50 +++++++++++++++++------------------------------- 1 file changed, 18 insertions(+), 32 deletions(-)