| Message ID | 20210121154526.1852176-7-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | xfs: rework log quiesce to cover the log | expand |
On 1/21/21 8:45 AM, Brian Foster wrote: > xfs_log_sbcount() calls xfs_sync_sb() to sync superblock counters to > disk when lazy superblock accounting is enabled. This occurs on > unmount, freeze, and read-only (re)mount and ensures the final > values are calculated and persisted to disk before each form of > quiesce completes. > > Now that log covering occurs in all of these contexts and uses the > same xfs_sync_sb() mechanism to update log state, there is no need > to log the superblock separately for any reason. Update the log > quiesce path to sync the superblock at least once for any mount > where lazy superblock accounting is enabled. If the log is already > covered, it will remain in the covered state. Otherwise, the next > sync as part of the normal covering sequence will carry the > associated superblock update with it. Remove xfs_log_sbcount() now > that it is no longer needed. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Ok, looks ok Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/xfs/xfs_log.c | 20 ++++++++++++++++++-- > fs/xfs/xfs_mount.c | 31 ------------------------------- > fs/xfs/xfs_mount.h | 1 - > fs/xfs/xfs_super.c | 8 -------- > 4 files changed, 18 insertions(+), 42 deletions(-) > > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c > index 6db65a4513a6..9479a5d0d785 100644 > --- a/fs/xfs/xfs_log.c > +++ b/fs/xfs/xfs_log.c > @@ -1108,6 +1108,7 @@ xfs_log_cover( > { > struct xlog *log = mp->m_log; > int error = 0; > + bool need_covered; > > ASSERT((xlog_cil_empty(log) && xlog_iclogs_empty(log) && > !xfs_ail_min_lsn(log->l_ailp)) || > @@ -1116,6 +1117,21 @@ xfs_log_cover( > if (!xfs_log_writable(mp)) > return 0; > > + /* > + * xfs_log_need_covered() is not idempotent because it progresses the > + * state machine if the log requires covering. Therefore, we must call > + * this function once and use the result until we've issued an sb sync. > + * Do so first to make that abundantly clear. > + * > + * Fall into the covering sequence if the log needs covering or the > + * mount has lazy superblock accounting to sync to disk. The sb sync > + * used for covering accumulates the in-core counters, so covering > + * handles this for us. > + */ > + need_covered = xfs_log_need_covered(mp); > + if (!need_covered && !xfs_sb_version_haslazysbcount(&mp->m_sb)) > + return 0; > + > /* > * To cover the log, commit the superblock twice (at most) in > * independent checkpoints. The first serves as a reference for the > @@ -1125,12 +1141,12 @@ xfs_log_cover( > * covering the log. Push the AIL one more time to leave it empty, as > * we found it. > */ > - while (xfs_log_need_covered(mp)) { > + do { > error = xfs_sync_sb(mp, true); > if (error) > break; > xfs_ail_push_all_sync(mp->m_ail); > - } > + } while (xfs_log_need_covered(mp)); > > return error; > } > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index a62b8a574409..f97b82d0e30f 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -1124,12 +1124,6 @@ xfs_unmountfs( > xfs_warn(mp, "Unable to free reserved block pool. " > "Freespace may not be correct on next mount."); > > - error = xfs_log_sbcount(mp); > - if (error) > - xfs_warn(mp, "Unable to update superblock counters. " > - "Freespace may not be correct on next mount."); > - > - > xfs_log_unmount(mp); > xfs_da_unmount(mp); > xfs_uuid_unmount(mp); > @@ -1164,31 +1158,6 @@ xfs_fs_writable( > return true; > } > > -/* > - * xfs_log_sbcount > - * > - * Sync the superblock counters to disk. > - * > - * Note this code can be called during the process of freezing, so we use the > - * transaction allocator that does not block when the transaction subsystem is > - * in its frozen state. > - */ > -int > -xfs_log_sbcount(xfs_mount_t *mp) > -{ > - if (!xfs_log_writable(mp)) > - return 0; > - > - /* > - * we don't need to do this if we are updating the superblock > - * counters on every modification. > - */ > - if (!xfs_sb_version_haslazysbcount(&mp->m_sb)) > - return 0; > - > - return xfs_sync_sb(mp, true); > -} > - > /* > * Deltas for the block count can vary from 1 to very large, but lock contention > * only occurs on frequent small block count updates such as in the delayed > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h > index dfa429b77ee2..452ca7654dc5 100644 > --- a/fs/xfs/xfs_mount.h > +++ b/fs/xfs/xfs_mount.h > @@ -399,7 +399,6 @@ int xfs_buf_hash_init(xfs_perag_t *pag); > void xfs_buf_hash_destroy(xfs_perag_t *pag); > > extern void xfs_uuid_table_free(void); > -extern int xfs_log_sbcount(xfs_mount_t *); > extern uint64_t xfs_default_resblks(xfs_mount_t *mp); > extern int xfs_mountfs(xfs_mount_t *mp); > extern int xfs_initialize_perag(xfs_mount_t *mp, xfs_agnumber_t agcount, > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 09d956e30fd8..75ada867c665 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -884,19 +884,11 @@ void > xfs_quiesce_attr( > struct xfs_mount *mp) > { > - int error = 0; > - > cancel_delayed_work_sync(&mp->m_log->l_work); > > /* force the log to unpin objects from the now complete transactions */ > xfs_log_force(mp, XFS_LOG_SYNC); > > - > - /* Push the superblock and write an unmount record */ > - error = xfs_log_sbcount(mp); > - if (error) > - xfs_warn(mp, "xfs_attr_quiesce: failed to log sb changes. " > - "Frozen image may not be consistent."); > xfs_log_clean(mp); > } > >
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 6db65a4513a6..9479a5d0d785 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -1108,6 +1108,7 @@ xfs_log_cover( { struct xlog *log = mp->m_log; int error = 0; + bool need_covered; ASSERT((xlog_cil_empty(log) && xlog_iclogs_empty(log) && !xfs_ail_min_lsn(log->l_ailp)) || @@ -1116,6 +1117,21 @@ xfs_log_cover( if (!xfs_log_writable(mp)) return 0; + /* + * xfs_log_need_covered() is not idempotent because it progresses the + * state machine if the log requires covering. Therefore, we must call + * this function once and use the result until we've issued an sb sync. + * Do so first to make that abundantly clear. + * + * Fall into the covering sequence if the log needs covering or the + * mount has lazy superblock accounting to sync to disk. The sb sync + * used for covering accumulates the in-core counters, so covering + * handles this for us. + */ + need_covered = xfs_log_need_covered(mp); + if (!need_covered && !xfs_sb_version_haslazysbcount(&mp->m_sb)) + return 0; + /* * To cover the log, commit the superblock twice (at most) in * independent checkpoints. The first serves as a reference for the @@ -1125,12 +1141,12 @@ xfs_log_cover( * covering the log. Push the AIL one more time to leave it empty, as * we found it. */ - while (xfs_log_need_covered(mp)) { + do { error = xfs_sync_sb(mp, true); if (error) break; xfs_ail_push_all_sync(mp->m_ail); - } + } while (xfs_log_need_covered(mp)); return error; } diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index a62b8a574409..f97b82d0e30f 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -1124,12 +1124,6 @@ xfs_unmountfs( xfs_warn(mp, "Unable to free reserved block pool. " "Freespace may not be correct on next mount."); - error = xfs_log_sbcount(mp); - if (error) - xfs_warn(mp, "Unable to update superblock counters. " - "Freespace may not be correct on next mount."); - - xfs_log_unmount(mp); xfs_da_unmount(mp); xfs_uuid_unmount(mp); @@ -1164,31 +1158,6 @@ xfs_fs_writable( return true; } -/* - * xfs_log_sbcount - * - * Sync the superblock counters to disk. - * - * Note this code can be called during the process of freezing, so we use the - * transaction allocator that does not block when the transaction subsystem is - * in its frozen state. - */ -int -xfs_log_sbcount(xfs_mount_t *mp) -{ - if (!xfs_log_writable(mp)) - return 0; - - /* - * we don't need to do this if we are updating the superblock - * counters on every modification. - */ - if (!xfs_sb_version_haslazysbcount(&mp->m_sb)) - return 0; - - return xfs_sync_sb(mp, true); -} - /* * Deltas for the block count can vary from 1 to very large, but lock contention * only occurs on frequent small block count updates such as in the delayed diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h index dfa429b77ee2..452ca7654dc5 100644 --- a/fs/xfs/xfs_mount.h +++ b/fs/xfs/xfs_mount.h @@ -399,7 +399,6 @@ int xfs_buf_hash_init(xfs_perag_t *pag); void xfs_buf_hash_destroy(xfs_perag_t *pag); extern void xfs_uuid_table_free(void); -extern int xfs_log_sbcount(xfs_mount_t *); extern uint64_t xfs_default_resblks(xfs_mount_t *mp); extern int xfs_mountfs(xfs_mount_t *mp); extern int xfs_initialize_perag(xfs_mount_t *mp, xfs_agnumber_t agcount, diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 09d956e30fd8..75ada867c665 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -884,19 +884,11 @@ void xfs_quiesce_attr( struct xfs_mount *mp) { - int error = 0; - cancel_delayed_work_sync(&mp->m_log->l_work); /* force the log to unpin objects from the now complete transactions */ xfs_log_force(mp, XFS_LOG_SYNC); - - /* Push the superblock and write an unmount record */ - error = xfs_log_sbcount(mp); - if (error) - xfs_warn(mp, "xfs_attr_quiesce: failed to log sb changes. " - "Frozen image may not be consistent."); xfs_log_clean(mp); }