Re: [PATCH 2/3] xfs: consolidate superblock logging functions

Brian Foster
2015-01-08T14:36:46Z

On Thu, Jan 08, 2015 at 08:51:04AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> We now have several superblock loggin functions that are identical
> except for the transaction reservation and whether it shoul dbe a
> synchronous transaction or not. Consolidate these all into a single
> function, a single reserveration and a sync flag and call it
> xfs_sync_sb().
>
> Also, xfs_mod_sb() is not really a modification function - it's the
> operation of logging the superblock buffer. hence change the name of
> it to reflect this.
>
> Note that we have to change the mp->m_update_flags that are passed
> around at mount time to a boolean simply to indicate a superblock
> update is needed.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

A question and one other little thing...

> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +-
> fs/xfs/libxfs/xfs_bmap.c | 10 +++---
> fs/xfs/libxfs/xfs_sb.c | 43 +++++++++++++++++++----
> fs/xfs/libxfs/xfs_sb.h | 3 +-
> fs/xfs/libxfs/xfs_shared.h | 33 ++++++++----------
> fs/xfs/libxfs/xfs_trans_resv.c | 14 --------
> fs/xfs/libxfs/xfs_trans_resv.h | 1 -
> fs/xfs/xfs_fsops.c | 29 ----------------
> fs/xfs/xfs_log.c | 18 ++++++++--
> fs/xfs/xfs_mount.c | 78 +++++++-----------------------------------
> fs/xfs/xfs_mount.h | 3 +-
> fs/xfs/xfs_qm.c | 27 ++-------------
> fs/xfs/xfs_qm_syscalls.c | 7 ++--
> fs/xfs/xfs_super.c | 13 +++----
> 14 files changed, 101 insertions(+), 180 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index c914422..15105db 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -403,7 +403,7 @@ xfs_sbversion_add_attr2(xfs_mount_t *mp, xfs_trans_t *tp)
> if (!xfs_sb_version_hasattr2(&mp->m_sb)) {
> xfs_sb_version_addattr2(&mp->m_sb);
> spin_unlock(&mp->m_sb_lock);
> - xfs_mod_sb(tp);
> + xfs_log_sb(tp);
> } else
> spin_unlock(&mp->m_sb_lock);
> }
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 8c39cc8..63a5bb9 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1221,20 +1221,20 @@ xfs_bmap_add_attrfork(
> goto bmap_cancel;
> if (!xfs_sb_version_hasattr(&mp->m_sb) ||
> (!xfs_sb_version_hasattr2(&mp->m_sb) && version == 2)) {
> - bool mod_sb = false;
> + bool log_sb = false;
>
> spin_lock(&mp->m_sb_lock);
> if (!xfs_sb_version_hasattr(&mp->m_sb)) {
> xfs_sb_version_addattr(&mp->m_sb);
> - mod_sb = true;
> + log_sb = true;
> }
> if (!xfs_sb_version_hasattr2(&mp->m_sb) && version == 2) {
> xfs_sb_version_addattr2(&mp->m_sb);
> - mod_sb = true;
> + log_sb = true;
> }
> spin_unlock(&mp->m_sb_lock);
> - if (mod_sb)
> - xfs_mod_sb(tp);
> + if (log_sb)
> + xfs_log_sb(tp);
> }
>
> error = xfs_bmap_finish(&tp, &flist, &committed);
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 4afbbce..6ee3af6 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -744,14 +744,13 @@ xfs_initialize_perag_data(
> }
>
> /*
> - * xfs_mod_sb() can be used to copy arbitrary changes to the
> - * in-core superblock into the superblock buffer to be logged.
> - * It does not provide the higher level of locking that is
> - * needed to protect the in-core superblock from concurrent
> - * access.
> + * xfs_log_sb() can be used to copy arbitrary changes to the in-core superblock
> + * into the superblock buffer to be logged. It does not provide the higher
> + * level of locking that is needed to protect the in-core superblock from
> + * concurrent access.
> */
> void
> -xfs_mod_sb(
> +xfs_log_sb(
> struct xfs_trans *tp)
> {
> struct xfs_mount *mp = tp->t_mountp;
> @@ -761,3 +760,35 @@ xfs_mod_sb(
> xfs_trans_buf_set_type(tp, bp, XFS_BLFT_SB_BUF);
> xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsb));
> }
> +
> +/*
> + * xfs_sync_sb
> + *
> + * Sync the superblock to disk.
> + *
> + * Note that the caller is responsible for checking the frozen state of the
> + * filesystem. This procedure uses the non-blocking transaction allocator and
> + * thus will allow modifications to a frozen fs. This is required because this
> + * code can be called during the process of freezing where use of the high-level
> + * allocator would deadlock.
> + */
> +int
> +xfs_sync_sb(
> + struct xfs_mount *mp,
> + bool wait)
> +{
> + struct xfs_trans *tp;
> + int error;
> +
> + tp = _xfs_trans_alloc(mp, XFS_TRANS_SB_CHANGE, KM_SLEEP);
> + error = xfs_trans_reserve(tp, &M_RES(mp)->tr_sb, 0, 0);
> + if (error) {
> + xfs_trans_cancel(tp, 0);
> + return error;
> + }
> +
> + xfs_log_sb(tp);
> + if (wait)
> + xfs_trans_set_sync(tp);
> + return xfs_trans_commit(tp, 0);
> +}
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index e193caa..b25bb9a 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -28,7 +28,8 @@ extern void xfs_perag_put(struct xfs_perag *pag);
> extern int xfs_initialize_perag_data(struct xfs_mount *, xfs_agnumber_t);
>
> extern void xfs_sb_calc_crc(struct xfs_buf *bp);
> -extern void xfs_mod_sb(struct xfs_trans *tp);
> +extern void xfs_log_sb(struct xfs_trans *tp);
> +extern int xfs_sync_sb(struct xfs_mount *mp, bool wait);
> extern void xfs_sb_mount_common(struct xfs_mount *mp, struct xfs_sb *sbp);
> extern void xfs_sb_from_disk(struct xfs_sb *to, struct xfs_dsb *from);
> extern void xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
> diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
> index 82404da..8dda4b3 100644
> --- a/fs/xfs/libxfs/xfs_shared.h
> +++ b/fs/xfs/libxfs/xfs_shared.h
> @@ -82,7 +82,7 @@ extern const struct xfs_buf_ops xfs_symlink_buf_ops;
> #define XFS_TRANS_ATTR_RM 23
> #define XFS_TRANS_ATTR_FLAG 24
> #define XFS_TRANS_CLEAR_AGI_BUCKET 25
> -#define XFS_TRANS_QM_SBCHANGE 26
> +#define XFS_TRANS_SB_CHANGE 26
> /*
> * Dummy entries since we use the transaction type to index into the
> * trans_type[] in xlog_recover_print_trans_head()
> @@ -95,17 +95,15 @@ extern const struct xfs_buf_ops xfs_symlink_buf_ops;
> #define XFS_TRANS_QM_DQCLUSTER 32
> #define XFS_TRANS_QM_QINOCREATE 33
> #define XFS_TRANS_QM_QUOTAOFF_END 34
> -#define XFS_TRANS_SB_UNIT 35
> -#define XFS_TRANS_FSYNC_TS 36
> -#define XFS_TRANS_GROWFSRT_ALLOC 37
> -#define XFS_TRANS_GROWFSRT_ZERO 38
> -#define XFS_TRANS_GROWFSRT_FREE 39
> -#define XFS_TRANS_SWAPEXT 40
> -#define XFS_TRANS_SB_COUNT 41
> -#define XFS_TRANS_CHECKPOINT 42
> -#define XFS_TRANS_ICREATE 43
> -#define XFS_TRANS_CREATE_TMPFILE 44
> -#define XFS_TRANS_TYPE_MAX 44
> +#define XFS_TRANS_FSYNC_TS 35
> +#define XFS_TRANS_GROWFSRT_ALLOC 36
> +#define XFS_TRANS_GROWFSRT_ZERO 37
> +#define XFS_TRANS_GROWFSRT_FREE 38
> +#define XFS_TRANS_SWAPEXT 39
> +#define XFS_TRANS_CHECKPOINT 40
> +#define XFS_TRANS_ICREATE 41
> +#define XFS_TRANS_CREATE_TMPFILE 42
> +#define XFS_TRANS_TYPE_MAX 43
> /* new transaction types need to be reflected in xfs_logprint(8) */
>
> #define XFS_TRANS_TYPES \
> @@ -113,7 +111,6 @@ extern const struct xfs_buf_ops xfs_symlink_buf_ops;
> { XFS_TRANS_SETATTR_SIZE, "SETATTR_SIZE" }, \
> { XFS_TRANS_INACTIVE, "INACTIVE" }, \
> { XFS_TRANS_CREATE, "CREATE" }, \
> - { XFS_TRANS_CREATE_TMPFILE, "CREATE_TMPFILE" }, \
> { XFS_TRANS_CREATE_TRUNC, "CREATE_TRUNC" }, \
> { XFS_TRANS_TRUNCATE_FILE, "TRUNCATE_FILE" }, \
> { XFS_TRANS_REMOVE, "REMOVE" }, \
> @@ -134,23 +131,23 @@ extern const struct xfs_buf_ops xfs_symlink_buf_ops;
> { XFS_TRANS_ATTR_RM, "ATTR_RM" }, \
> { XFS_TRANS_ATTR_FLAG, "ATTR_FLAG" }, \
> { XFS_TRANS_CLEAR_AGI_BUCKET, "CLEAR_AGI_BUCKET" }, \
> - { XFS_TRANS_QM_SBCHANGE, "QM_SBCHANGE" }, \
> + { XFS_TRANS_SB_CHANGE, "SBCHANGE" }, \
> + { XFS_TRANS_DUMMY1, "DUMMY1" }, \
> + { XFS_TRANS_DUMMY2, "DUMMY2" }, \

I take it we can't just remove these dummy types, for userspace
compatibility reasons, right? At least, it looks like logprint could get
confused if other transaction types inherited these values.

> { XFS_TRANS_QM_QUOTAOFF, "QM_QUOTAOFF" }, \
> { XFS_TRANS_QM_DQALLOC, "QM_DQALLOC" }, \
> { XFS_TRANS_QM_SETQLIM, "QM_SETQLIM" }, \
> { XFS_TRANS_QM_DQCLUSTER, "QM_DQCLUSTER" }, \
> { XFS_TRANS_QM_QINOCREATE, "QM_QINOCREATE" }, \
> { XFS_TRANS_QM_QUOTAOFF_END, "QM_QOFF_END" }, \
> - { XFS_TRANS_SB_UNIT, "SB_UNIT" }, \
> { XFS_TRANS_FSYNC_TS, "FSYNC_TS" }, \
> { XFS_TRANS_GROWFSRT_ALLOC, "GROWFSRT_ALLOC" }, \
> { XFS_TRANS_GROWFSRT_ZERO, "GROWFSRT_ZERO" }, \
> { XFS_TRANS_GROWFSRT_FREE, "GROWFSRT_FREE" }, \
> { XFS_TRANS_SWAPEXT, "SWAPEXT" }, \
> - { XFS_TRANS_SB_COUNT, "SB_COUNT" }, \
> { XFS_TRANS_CHECKPOINT, "CHECKPOINT" }, \
> - { XFS_TRANS_DUMMY1, "DUMMY1" }, \
> - { XFS_TRANS_DUMMY2, "DUMMY2" }, \
> + { XFS_TRANS_ICREATE, "ICREATE" }, \
> + { XFS_TRANS_CREATE_TMPFILE, "CREATE_TMPFILE" }, \
> { XLOG_UNMOUNT_REC_TYPE, "UNMOUNT" }
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
> index 6c1330f..68cb1e7 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.c
> +++ b/fs/xfs/libxfs/xfs_trans_resv.c
> @@ -716,17 +716,6 @@ xfs_calc_clear_agi_bucket_reservation(
> }
>
> /*
> - * Clearing the quotaflags in the superblock.
> - * the super block for changing quota flags: sector size
> - */
> -STATIC uint
> -xfs_calc_qm_sbchange_reservation(
> - struct xfs_mount *mp)
> -{
> - return xfs_calc_buf_res(1, mp->m_sb.sb_sectsize);
> -}
> -
> -/*
> * Adjusting quota limits.
> * the xfs_disk_dquot_t: sizeof(struct xfs_disk_dquot)
> */
> @@ -864,9 +853,6 @@ xfs_trans_resv_calc(
> * The following transactions are logged in logical format with
> * a default log count.
> */
> - resp->tr_qm_sbchange.tr_logres = xfs_calc_qm_sbchange_reservation(mp);
> - resp->tr_qm_sbchange.tr_logcount = XFS_DEFAULT_LOG_COUNT;
> -
> resp->tr_qm_setqlim.tr_logres = xfs_calc_qm_setqlim_reservation(mp);
> resp->tr_qm_setqlim.tr_logcount = XFS_DEFAULT_LOG_COUNT;
>
> diff --git a/fs/xfs/libxfs/xfs_trans_resv.h b/fs/xfs/libxfs/xfs_trans_resv.h
> index 1097d14..2d5bdfc 100644
> --- a/fs/xfs/libxfs/xfs_trans_resv.h
> +++ b/fs/xfs/libxfs/xfs_trans_resv.h
> @@ -56,7 +56,6 @@ struct xfs_trans_resv {
> struct xfs_trans_res tr_growrtalloc; /* grow realtime allocations */
> struct xfs_trans_res tr_growrtzero; /* grow realtime zeroing */
> struct xfs_trans_res tr_growrtfree; /* grow realtime freeing */
> - struct xfs_trans_res tr_qm_sbchange; /* change quota flags */
> struct xfs_trans_res tr_qm_setqlim; /* adjust quota limits */
> struct xfs_trans_res tr_qm_dqalloc; /* allocate quota on disk */
> struct xfs_trans_res tr_qm_quotaoff; /* turn quota off */
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 82af857..f711452 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -756,35 +756,6 @@ out:
> return 0;
> }
>
> -/*
> - * Dump a transaction into the log that contains no real change. This is needed
> - * to be able to make the log dirty or stamp the current tail LSN into the log
> - * during the covering operation.
> - *
> - * We cannot use an inode here for this - that will push dirty state back up
> - * into the VFS and then periodic inode flushing will prevent log covering from
> - * making progress. Hence we log a field in the superblock instead and use a
> - * synchronous transaction to ensure the superblock is immediately unpinned
> - * and can be written back.
> - */
> -int
> -xfs_fs_log_dummy(
> - xfs_mount_t *mp)
> -{
> - xfs_trans_t *tp;
> - int error;
> -
> - tp = _xfs_trans_alloc(mp, XFS_TRANS_DUMMY1, KM_SLEEP);
> - error = xfs_trans_reserve(tp, &M_RES(mp)->tr_sb, 0, 0);
> - if (error) {
> - xfs_trans_cancel(tp, 0);
> - return error;
> - }
> - xfs_mod_sb(tp);
> - xfs_trans_set_sync(tp);
> - return xfs_trans_commit(tp, 0);
> -}
> -
> int
> xfs_fs_goingdown(
> xfs_mount_t *mp,
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 8fbbfb2..bcc7cfa 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -33,6 +33,7 @@
> #include "xfs_fsops.h"
> #include "xfs_cksum.h"
> #include "xfs_sysfs.h"
> +#include "xfs_sb.h"
>
> kmem_zone_t *xfs_log_ticket_zone;
>
> @@ -1290,9 +1291,20 @@ xfs_log_worker(
> struct xfs_mount *mp = log->l_mp;
>
> /* dgc: errors ignored - not fatal and nowhere to report them */
> - if (xfs_log_need_covered(mp))
> - xfs_fs_log_dummy(mp);
> - else
> + if (xfs_log_need_covered(mp)) {
> + /*
> + * Dump a transaction into the log that contains no real change.
> + * This is needed to stamp the current tail LSN into the log
> + * during the covering operation.
> + *
> + * We cannot use an inode here for this - that will push dirty
> + * state back up into the VFS and then periodic inode flushing
> + * will prevent log covering from making progress. Hence we
> + * synchronously log the superblock instead to ensure the
> + * superblock is immediately unpinned and can be written back.
> + */
> + xfs_sync_sb(mp, true);
> + } else
> xfs_log_force(mp, 0);
>
> /* start pushing all the metadata that is currently dirty */
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index defcf69..5ef9aa2 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -408,11 +408,11 @@ xfs_update_alignment(xfs_mount_t *mp)
> if (xfs_sb_version_hasdalign(sbp)) {
> if (sbp->sb_unit != mp->m_dalign) {
> sbp->sb_unit = mp->m_dalign;
> - mp->m_update_flags |= XFS_SB_UNIT;
> + mp->m_update_sb = true;
> }
> if (sbp->sb_width != mp->m_swidth) {
> sbp->sb_width = mp->m_swidth;
> - mp->m_update_flags |= XFS_SB_WIDTH;
> + mp->m_update_sb = true;
> }
> } else {
> xfs_warn(mp,
> @@ -583,38 +583,19 @@ int
> xfs_mount_reset_sbqflags(
> struct xfs_mount *mp)
> {
> - int error;
> - struct xfs_trans *tp;
> -
> mp->m_qflags = 0;
>
> - /*
> - * It is OK to look at sb_qflags here in mount path,
> - * without m_sb_lock.
> - */
> + /* It is OK to look at sb_qflags in the mount path without m_sb_lock. */
> if (mp->m_sb.sb_qflags == 0)
> return 0;
> spin_lock(&mp->m_sb_lock);
> mp->m_sb.sb_qflags = 0;
> spin_unlock(&mp->m_sb_lock);
>
> - /*
> - * If the fs is readonly, let the incore superblock run
> - * with quotas off but don't flush the update out to disk
> - */
> - if (mp->m_flags & XFS_MOUNT_RDONLY)
> + if (!xfs_fs_writable(mp, SB_FREEZE_WRITE))
> return 0;
>
> - tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SBCHANGE);
> - error = xfs_trans_reserve(tp, &M_RES(mp)->tr_qm_sbchange, 0, 0);
> - if (error) {
> - xfs_trans_cancel(tp, 0);
> - xfs_alert(mp, "%s: Superblock update failed!", __func__);
> - return error;
> - }
> -
> - xfs_mod_sb(tp);
> - return xfs_trans_commit(tp, 0);
> + return xfs_sync_sb(mp, false);
> }
>
> __uint64_t
> @@ -678,7 +659,7 @@ xfs_mountfs(
> xfs_warn(mp, "correcting sb_features alignment problem");
> sbp->sb_features2 |= sbp->sb_bad_features2;
> sbp->sb_bad_features2 = sbp->sb_features2;
> - mp->m_update_flags |= XFS_SB_FEATURES2;
> + mp->m_update_sb = true;
>
> /*
> * Re-check for ATTR2 in case it was found in bad_features2
> @@ -692,17 +673,17 @@ xfs_mountfs(
> if (xfs_sb_version_hasattr2(&mp->m_sb) &&
> (mp->m_flags & XFS_MOUNT_NOATTR2)) {
> xfs_sb_version_removeattr2(&mp->m_sb);
> - mp->m_update_flags |= XFS_SB_FEATURES2;
> + mp->m_update_sb = true;
>
> /* update sb_versionnum for the clearing of the morebits */
> if (!sbp->sb_features2)
> - mp->m_update_flags |= XFS_SB_VERSIONNUM;
> + mp->m_update_sb = true;
> }
>
> /* always use v2 inodes by default now */
> if (!(mp->m_sb.sb_versionnum & XFS_SB_VERSION_NLINKBIT)) {
> mp->m_sb.sb_versionnum |= XFS_SB_VERSION_NLINKBIT;
> - mp->m_update_flags |= XFS_SB_VERSIONNUM;
> + mp->m_update_sb = true;
> }
>
> /*
> @@ -895,8 +876,8 @@ xfs_mountfs(
> * the next remount into writeable mode. Otherwise we would never
> * perform the update e.g. for the root filesystem.
> */
> - if (mp->m_update_flags && !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> - error = xfs_mount_log_sb(mp);
> + if (mp->m_update_sb && !(mp->m_flags & XFS_MOUNT_RDONLY)) {
> + error = xfs_sync_sb(mp, false);
> if (error) {
> xfs_warn(mp, "failed to write sb changes");
> goto out_rtunmount;
> @@ -1103,9 +1084,6 @@ xfs_fs_writable(
> int
> xfs_log_sbcount(xfs_mount_t *mp)
> {
> - xfs_trans_t *tp;
> - int error;
> -
> /* allow this to proceed during the freeze sequence... */
> if (!xfs_fs_writable(mp, SB_FREEZE_COMPLETE))
> return 0;
> @@ -1119,17 +1097,7 @@ xfs_log_sbcount(xfs_mount_t *mp)
> if (!xfs_sb_version_haslazysbcount(&mp->m_sb))
> return 0;
>
> - tp = _xfs_trans_alloc(mp, XFS_TRANS_SB_COUNT, KM_SLEEP);
> - error = xfs_trans_reserve(tp, &M_RES(mp)->tr_sb, 0, 0);
> - if (error) {
> - xfs_trans_cancel(tp, 0);
> - return error;
> - }
> -
> - xfs_mod_sb(tp);
> - xfs_trans_set_sync(tp);
> - error = xfs_trans_commit(tp, 0);
> - return error;
> + return xfs_sync_sb(mp, true);
> }
>
> /*
> @@ -1423,28 +1391,6 @@ xfs_freesb(
> }
>
> /*
> - * Used to log changes to the superblock unit and width fields which could
> - * be altered by the mount options, as well as any potential sb_features2
> - * fixup. Only the first superblock is updated.
> - */
> -int
> -xfs_mount_log_sb(
> - struct xfs_mount *mp)
> -{
> - struct xfs_trans *tp;
> - int error;
> -
> - tp = xfs_trans_alloc(mp, XFS_TRANS_SB_UNIT);
> - error = xfs_trans_reserve(tp, &M_RES(mp)->tr_sb, 0, 0);
> - if (error) {
> - xfs_trans_cancel(tp, 0);
> - return error;
> - }
> - xfs_mod_sb(tp);
> - return xfs_trans_commit(tp, 0);
> -}
> -
> -/*
> * If the underlying (data/log/rt) device is readonly, there are some
> * operations that cannot proceed.
> */
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 28b341b..a5b2ff8 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -162,8 +162,7 @@ typedef struct xfs_mount {
> struct delayed_work m_reclaim_work; /* background inode reclaim */
> struct delayed_work m_eofblocks_work; /* background eof blocks
> trimming */
> - __int64_t m_update_flags; /* sb flags we need to update
> - on the next remount,rw */
> + bool m_update_sb; /* sb needs update in mount */
> int64_t m_low_space[XFS_LOWSP_MAX];
> /* low free space thresholds */
> struct xfs_kobj m_kobj;
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index c815a80..3e81862 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -792,7 +792,7 @@ xfs_qm_qino_alloc(
> else
> mp->m_sb.sb_pquotino = (*ip)->i_ino;
> spin_unlock(&mp->m_sb_lock);
> - xfs_mod_sb(tp);
> + xfs_log_sb(tp);
>
> if ((error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES))) {
> xfs_alert(mp, "%s failed (error %d)!", __func__, error);
> @@ -1445,7 +1445,7 @@ xfs_qm_mount_quotas(
> spin_unlock(&mp->m_sb_lock);
>
> if (sbf != (mp->m_qflags & XFS_MOUNT_QUOTA_ALL)) {
> - if (xfs_qm_write_sb_changes(mp)) {
> + if (xfs_sync_sb(mp, false)) {
> /*
> * We could only have been turning quotas off.
> * We aren't in very good shape actually because
> @@ -1574,29 +1574,6 @@ xfs_qm_dqfree_one(
> xfs_qm_dqdestroy(dqp);
> }
>
> -/*
> - * Start a transaction and write the incore superblock changes to
> - * disk. flags parameter indicates which fields have changed.
> - */
> -int
> -xfs_qm_write_sb_changes(

This guy still has a function declaration in xfs_qm.h. The rest looks
good to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> - struct xfs_mount *mp)
> -{
> - xfs_trans_t *tp;
> - int error;
> -
> - tp = xfs_trans_alloc(mp, XFS_TRANS_QM_SBCHANGE);
> - error = xfs_trans_reserve(tp, &M_RES(mp)->tr_qm_sbchange, 0, 0);
> - if (error) {
> - xfs_trans_cancel(tp, 0);
> - return error;
> - }
> -
> - xfs_mod_sb(tp);
> - return xfs_trans_commit(tp, 0);
> -}
> -
> -
> /* --------------- utility functions for vnodeops ---------------- */
>
>
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 8d7e5f0..b8a565e 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -92,7 +92,7 @@ xfs_qm_scall_quotaoff(
> mutex_unlock(&q->qi_quotaofflock);
>
> /* XXX what to do if error ? Revert back to old vals incore ? */
> - return xfs_qm_write_sb_changes(mp);
> + return xfs_sync_sb(mp, false);
> }
>
> dqtype = 0;
> @@ -369,7 +369,8 @@ xfs_qm_scall_quotaon(
> if ((qf & flags) == flags)
> return -EEXIST;
>
> - if ((error = xfs_qm_write_sb_changes(mp)))
> + error = xfs_sync_sb(mp, false);
> + if (error)
> return error;
> /*
> * If we aren't trying to switch on quota enforcement, we are done.
> @@ -796,7 +797,7 @@ xfs_qm_log_quotaoff(
> mp->m_sb.sb_qflags = (mp->m_qflags & ~(flags)) & XFS_MOUNT_QUOTA_ALL;
> spin_unlock(&mp->m_sb_lock);
>
> - xfs_mod_sb(tp);
> + xfs_log_sb(tp);
>
> /*
> * We have to make sure that the transaction is secure on disk before we
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 3797a03..afd6bae 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1257,13 +1257,13 @@ xfs_fs_remount(
> * If this is the first remount to writeable state we
> * might have some superblock changes to update.
> */
> - if (mp->m_update_flags) {
> - error = xfs_mount_log_sb(mp);
> + if (mp->m_update_sb) {
> + error = xfs_sync_sb(mp, false);
> if (error) {
> xfs_warn(mp, "failed to write sb changes");
> return error;
> }
> - mp->m_update_flags = 0;
> + mp->m_update_sb = false;
> }
>
> /*
> @@ -1293,8 +1293,9 @@ xfs_fs_remount(
>
> /*
> * Second stage of a freeze. The data is already frozen so we only
> - * need to take care of the metadata. Once that's done write a dummy
> - * record to dirty the log in case of a crash while frozen.
> + * need to take care of the metadata. Once that's done sync the superblock
> + * to the log to dirty it in case of a crash while frozen. This ensures that we
> + * will recover the unlinked inode lists on the next mount.
> */
> STATIC int
> xfs_fs_freeze(
> @@ -1304,7 +1305,7 @@ xfs_fs_freeze(
>
> xfs_save_resvblks(mp);
> xfs_quiesce_attr(mp);
> - return xfs_fs_log_dummy(mp);
> + return xfs_sync_sb(mp, true);
> }
>
> STATIC int
> --
> 2.0.0
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs