On Thu, Jan 08, 2015 at 08:51:03AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> When we log changes to the superblock, we first have to write them
> to the on-disk buffer, and then log that. Right now we have a
> complex bitfield based arrangement to only write the modified field
> to the buffer before we log it.
>
> This used to be necessary as a performance optimisation because we
> logged the superblock buffer in every extent or inode allocation or
> freeing, and so performance was extremely important. We haven't done
> this for years, however, ever since the lazy superblock counters
> pulled the superblock logging out of the transaction commit
> fast path.
>
> Hence we have a bunch of complexity that is not necessary that makes
> writing the in-core superblock to disk much more complex than it
> needs to be. We only need to log the superblock now during
> management operations (e.g. during mount, unmount or quota control
> operations) so it is not a performance critical path anymore.
>
> As such, remove the complex field based logging mechanism and
> replace it with a simple conversion function similar to what we use
> for all other on-disk structures.
>
> This means we always log the entirity of the superblock, but again
> because we rarely modify the superblock this is not an issue for log
> bandwidth or CPU time. Indeed, if we do log the superblock
> frequently, delayed logging will minimise the impact of this
> overhead.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 2 +-
> fs/xfs/libxfs/xfs_bmap.c | 14 +-
> fs/xfs/libxfs/xfs_sb.c | 290 ++++++++++++++----------------------------
> fs/xfs/libxfs/xfs_sb.h | 10 +-
> fs/xfs/xfs_fsops.c | 6 +-
> fs/xfs/xfs_mount.c | 21 ++-
> fs/xfs/xfs_mount.h | 2 +-
> fs/xfs/xfs_qm.c | 26 +---
> fs/xfs/xfs_qm.h | 2 +-
> fs/xfs/xfs_qm_syscalls.c | 13 +-
> fs/xfs/xfs_super.c | 2 +-
> 11 files changed, 132 insertions(+), 256 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 5d38e8b..c914422 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_SB_VERSIONNUM | XFS_SB_FEATURES2);
> + xfs_mod_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 b5eb474..8c39cc8 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1221,22 +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)) {
> - __int64_t sbfields = 0;
> + bool mod_sb = false;
>
> spin_lock(&mp->m_sb_lock);
> if (!xfs_sb_version_hasattr(&mp->m_sb)) {
> xfs_sb_version_addattr(&mp->m_sb);
> - sbfields |= XFS_SB_VERSIONNUM;
> + mod_sb = true;
> }
> if (!xfs_sb_version_hasattr2(&mp->m_sb) && version == 2) {
> xfs_sb_version_addattr2(&mp->m_sb);
> - sbfields |= (XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);
> + mod_sb = true;
> }
> - if (sbfields) {
> - spin_unlock(&mp->m_sb_lock);
> - xfs_mod_sb(tp, sbfields);
> - } else
> - spin_unlock(&mp->m_sb_lock);
> + spin_unlock(&mp->m_sb_lock);
> + if (mod_sb)
> + xfs_mod_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 752915f..4afbbce 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -40,69 +40,6 @@
> * Physical superblock buffer manipulations. Shared with libxfs in userspace.
> */
>
> -static const struct {
> - short offset;
> - short type; /* 0 = integer
> - * 1 = binary / string (no translation)
> - */
> -} xfs_sb_info[] = {
> - { offsetof(xfs_sb_t, sb_magicnum), 0 },
> - { offsetof(xfs_sb_t, sb_blocksize), 0 },
> - { offsetof(xfs_sb_t, sb_dblocks), 0 },
> - { offsetof(xfs_sb_t, sb_rblocks), 0 },
> - { offsetof(xfs_sb_t, sb_rextents), 0 },
> - { offsetof(xfs_sb_t, sb_uuid), 1 },
> - { offsetof(xfs_sb_t, sb_logstart), 0 },
> - { offsetof(xfs_sb_t, sb_rootino), 0 },
> - { offsetof(xfs_sb_t, sb_rbmino), 0 },
> - { offsetof(xfs_sb_t, sb_rsumino), 0 },
> - { offsetof(xfs_sb_t, sb_rextsize), 0 },
> - { offsetof(xfs_sb_t, sb_agblocks), 0 },
> - { offsetof(xfs_sb_t, sb_agcount), 0 },
> - { offsetof(xfs_sb_t, sb_rbmblocks), 0 },
> - { offsetof(xfs_sb_t, sb_logblocks), 0 },
> - { offsetof(xfs_sb_t, sb_versionnum), 0 },
> - { offsetof(xfs_sb_t, sb_sectsize), 0 },
> - { offsetof(xfs_sb_t, sb_inodesize), 0 },
> - { offsetof(xfs_sb_t, sb_inopblock), 0 },
> - { offsetof(xfs_sb_t, sb_fname[0]), 1 },
> - { offsetof(xfs_sb_t, sb_blocklog), 0 },
> - { offsetof(xfs_sb_t, sb_sectlog), 0 },
> - { offsetof(xfs_sb_t, sb_inodelog), 0 },
> - { offsetof(xfs_sb_t, sb_inopblog), 0 },
> - { offsetof(xfs_sb_t, sb_agblklog), 0 },
> - { offsetof(xfs_sb_t, sb_rextslog), 0 },
> - { offsetof(xfs_sb_t, sb_inprogress), 0 },
> - { offsetof(xfs_sb_t, sb_imax_pct), 0 },
> - { offsetof(xfs_sb_t, sb_icount), 0 },
> - { offsetof(xfs_sb_t, sb_ifree), 0 },
> - { offsetof(xfs_sb_t, sb_fdblocks), 0 },
> - { offsetof(xfs_sb_t, sb_frextents), 0 },
> - { offsetof(xfs_sb_t, sb_uquotino), 0 },
> - { offsetof(xfs_sb_t, sb_gquotino), 0 },
> - { offsetof(xfs_sb_t, sb_qflags), 0 },
> - { offsetof(xfs_sb_t, sb_flags), 0 },
> - { offsetof(xfs_sb_t, sb_shared_vn), 0 },
> - { offsetof(xfs_sb_t, sb_inoalignmt), 0 },
> - { offsetof(xfs_sb_t, sb_unit), 0 },
> - { offsetof(xfs_sb_t, sb_width), 0 },
> - { offsetof(xfs_sb_t, sb_dirblklog), 0 },
> - { offsetof(xfs_sb_t, sb_logsectlog), 0 },
> - { offsetof(xfs_sb_t, sb_logsectsize), 0 },
> - { offsetof(xfs_sb_t, sb_logsunit), 0 },
> - { offsetof(xfs_sb_t, sb_features2), 0 },
> - { offsetof(xfs_sb_t, sb_bad_features2), 0 },
> - { offsetof(xfs_sb_t, sb_features_compat), 0 },
> - { offsetof(xfs_sb_t, sb_features_ro_compat), 0 },
> - { offsetof(xfs_sb_t, sb_features_incompat), 0 },
> - { offsetof(xfs_sb_t, sb_features_log_incompat), 0 },
> - { offsetof(xfs_sb_t, sb_crc), 0 },
> - { offsetof(xfs_sb_t, sb_pad), 0 },
> - { offsetof(xfs_sb_t, sb_pquotino), 0 },
> - { offsetof(xfs_sb_t, sb_lsn), 0 },
> - { sizeof(xfs_sb_t), 0 }
> -};
> -
> /*
> * Reference counting access wrappers to the perag structures.
> * Because we never free per-ag structures, the only thing we
> @@ -461,128 +398,119 @@ xfs_sb_from_disk(
> __xfs_sb_from_disk(to, from, true);
> }
>
> -static inline void
> +static void
> xfs_sb_quota_to_disk(
> - xfs_dsb_t *to,
> - xfs_sb_t *from,
> - __int64_t *fields)
> + struct xfs_dsb *to,
> + struct xfs_sb *from)
> {
> __uint16_t qflags = from->sb_qflags;
>
> + to->sb_uquotino = cpu_to_be64(from->sb_uquotino);
> + if (xfs_sb_version_has_pquotino(from)) {
> + to->sb_qflags = cpu_to_be16(from->sb_qflags);
> + to->sb_gquotino = cpu_to_be64(from->sb_gquotino);
> + to->sb_pquotino = cpu_to_be64(from->sb_pquotino);
> + return;
> + }
> +
> /*
> - * We need to do these manipilations only if we are working
> - * with an older version of on-disk superblock.
> + * The in-core version of sb_qflags do not have XFS_OQUOTA_*
> + * flags, whereas the on-disk version does. So, convert incore
> + * XFS_{PG}QUOTA_* flags to on-disk XFS_OQUOTA_* flags.
> */
> - if (xfs_sb_version_has_pquotino(from))
> - return;
> + qflags &= ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
> + XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
>
> - if (*fields & XFS_SB_QFLAGS) {
> - /*
> - * The in-core version of sb_qflags do not have
> - * XFS_OQUOTA_* flags, whereas the on-disk version
> - * does. So, convert incore XFS_{PG}QUOTA_* flags
> - * to on-disk XFS_OQUOTA_* flags.
> - */
> - qflags &= ~(XFS_PQUOTA_ENFD | XFS_PQUOTA_CHKD |
> - XFS_GQUOTA_ENFD | XFS_GQUOTA_CHKD);
> -
> - if (from->sb_qflags &
> - (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
> - qflags |= XFS_OQUOTA_ENFD;
> - if (from->sb_qflags &
> - (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
> - qflags |= XFS_OQUOTA_CHKD;
> - to->sb_qflags = cpu_to_be16(qflags);
> - *fields &= ~XFS_SB_QFLAGS;
> - }
> + if (from->sb_qflags &
> + (XFS_PQUOTA_ENFD | XFS_GQUOTA_ENFD))
> + qflags |= XFS_OQUOTA_ENFD;
> + if (from->sb_qflags &
> + (XFS_PQUOTA_CHKD | XFS_GQUOTA_CHKD))
> + qflags |= XFS_OQUOTA_CHKD;
> + to->sb_qflags = cpu_to_be16(qflags);
>
> /*
> - * GQUOTINO and PQUOTINO cannot be used together in versions of
> - * superblock that do not have pquotino. from->sb_flags tells us which
> - * quota is active and should be copied to disk. If neither are active,
> - * make sure we write NULLFSINO to the sb_gquotino field as a quota
> - * inode value of "0" is invalid when the XFS_SB_VERSION_QUOTA feature
> - * bit is set.
> + * GQUOTINO and PQUOTINO cannot be used together in versions
> + * of superblock that do not have pquotino. from->sb_flags
> + * tells us which quota is active and should be copied to
> + * disk. If neither are active, we should NULL the inode.
> *
> - * Note that we don't need to handle the sb_uquotino or sb_pquotino here
> - * as they do not require any translation. Hence the main sb field loop
> - * will write them appropriately from the in-core superblock.
> + * In all cases, the separate pquotino must remain 0 because it
> + * it beyond the "end" of the valid non-pquotino superblock.
> */
> - if ((*fields & XFS_SB_GQUOTINO) &&
> - (from->sb_qflags & XFS_GQUOTA_ACCT))
> + if (from->sb_qflags & XFS_GQUOTA_ACCT)
> to->sb_gquotino = cpu_to_be64(from->sb_gquotino);
> - else if ((*fields & XFS_SB_PQUOTINO) &&
> - (from->sb_qflags & XFS_PQUOTA_ACCT))
> + else if (from->sb_qflags & XFS_PQUOTA_ACCT)
> to->sb_gquotino = cpu_to_be64(from->sb_pquotino);
> - else {
> - /*
> - * We can't rely on just the fields being logged to tell us
> - * that it is safe to write NULLFSINO - we should only do that
> - * if quotas are not actually enabled. Hence only write
> - * NULLFSINO if both in-core quota inodes are NULL.
> - */
> - if (from->sb_gquotino == NULLFSINO &&
> - from->sb_pquotino == NULLFSINO)
> - to->sb_gquotino = cpu_to_be64(NULLFSINO);
> - }
> + else
> + to->sb_gquotino = cpu_to_be64(NULLFSINO);
FYI... it looks like the above hunk causes a regression due to resetting
sb_gquotaino when one of the in-core inodes is set. I'm seeing
disconnected quota inode messages on some xfstests (e.g., xfs/108) on v4
filesystems. I'm about to put a patch on the list to go back to the
original logic...
Brian
>
> - *fields &= ~(XFS_SB_PQUOTINO | XFS_SB_GQUOTINO);
> + to->sb_pquotino = 0;
> }
>
> -/*
> - * Copy in core superblock to ondisk one.
> - *
> - * The fields argument is mask of superblock fields to copy.
> - */
> void
> xfs_sb_to_disk(
> - xfs_dsb_t *to,
> - xfs_sb_t *from,
> - __int64_t fields)
> + struct xfs_dsb *to,
> + struct xfs_sb *from)
> {
> - xfs_caddr_t to_ptr = (xfs_caddr_t)to;
> - xfs_caddr_t from_ptr = (xfs_caddr_t)from;
> - xfs_sb_field_t f;
> - int first;
> - int size;
> -
> - ASSERT(fields);
> - if (!fields)
> - return;
> + xfs_sb_quota_to_disk(to, from);
>
> - /* We should never write the crc here, it's updated in the IO path */
> - fields &= ~XFS_SB_CRC;
> -
> - xfs_sb_quota_to_disk(to, from, &fields);
> - while (fields) {
> - f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
> - first = xfs_sb_info[f].offset;
> - size = xfs_sb_info[f + 1].offset - first;
> -
> - ASSERT(xfs_sb_info[f].type == 0 || xfs_sb_info[f].type == 1);
> -
> - if (size == 1 || xfs_sb_info[f].type == 1) {
> - memcpy(to_ptr + first, from_ptr + first, size);
> - } else {
> - switch (size) {
> - case 2:
> - *(__be16 *)(to_ptr + first) =
> - cpu_to_be16(*(__u16 *)(from_ptr + first));
> - break;
> - case 4:
> - *(__be32 *)(to_ptr + first) =
> - cpu_to_be32(*(__u32 *)(from_ptr + first));
> - break;
> - case 8:
> - *(__be64 *)(to_ptr + first) =
> - cpu_to_be64(*(__u64 *)(from_ptr + first));
> - break;
> - default:
> - ASSERT(0);
> - }
> - }
> + to->sb_magicnum = cpu_to_be32(from->sb_magicnum);
> + to->sb_blocksize = cpu_to_be32(from->sb_blocksize);
> + to->sb_dblocks = cpu_to_be64(from->sb_dblocks);
> + to->sb_rblocks = cpu_to_be64(from->sb_rblocks);
> + to->sb_rextents = cpu_to_be64(from->sb_rextents);
> + memcpy(&to->sb_uuid, &from->sb_uuid, sizeof(to->sb_uuid));
> + to->sb_logstart = cpu_to_be64(from->sb_logstart);
> + to->sb_rootino = cpu_to_be64(from->sb_rootino);
> + to->sb_rbmino = cpu_to_be64(from->sb_rbmino);
> + to->sb_rsumino = cpu_to_be64(from->sb_rsumino);
> + to->sb_rextsize = cpu_to_be32(from->sb_rextsize);
> + to->sb_agblocks = cpu_to_be32(from->sb_agblocks);
> + to->sb_agcount = cpu_to_be32(from->sb_agcount);
> + to->sb_rbmblocks = cpu_to_be32(from->sb_rbmblocks);
> + to->sb_logblocks = cpu_to_be32(from->sb_logblocks);
> + to->sb_versionnum = cpu_to_be16(from->sb_versionnum);
> + to->sb_sectsize = cpu_to_be16(from->sb_sectsize);
> + to->sb_inodesize = cpu_to_be16(from->sb_inodesize);
> + to->sb_inopblock = cpu_to_be16(from->sb_inopblock);
> + memcpy(&to->sb_fname, &from->sb_fname, sizeof(to->sb_fname));
> + to->sb_blocklog = from->sb_blocklog;
> + to->sb_sectlog = from->sb_sectlog;
> + to->sb_inodelog = from->sb_inodelog;
> + to->sb_inopblog = from->sb_inopblog;
> + to->sb_agblklog = from->sb_agblklog;
> + to->sb_rextslog = from->sb_rextslog;
> + to->sb_inprogress = from->sb_inprogress;
> + to->sb_imax_pct = from->sb_imax_pct;
> + to->sb_icount = cpu_to_be64(from->sb_icount);
> + to->sb_ifree = cpu_to_be64(from->sb_ifree);
> + to->sb_fdblocks = cpu_to_be64(from->sb_fdblocks);
> + to->sb_frextents = cpu_to_be64(from->sb_frextents);
>
> - fields &= ~(1LL << f);
> +
> + to->sb_flags = from->sb_flags;
> + to->sb_shared_vn = from->sb_shared_vn;
> + to->sb_inoalignmt = cpu_to_be32(from->sb_inoalignmt);
> + to->sb_unit = cpu_to_be32(from->sb_unit);
> + to->sb_width = cpu_to_be32(from->sb_width);
> + to->sb_dirblklog = from->sb_dirblklog;
> + to->sb_logsectlog = from->sb_logsectlog;
> + to->sb_logsectsize = cpu_to_be16(from->sb_logsectsize);
> + to->sb_logsunit = cpu_to_be32(from->sb_logsunit);
> + to->sb_features2 = cpu_to_be32(from->sb_features2);
> + to->sb_bad_features2 = cpu_to_be32(from->sb_bad_features2);
> +
> + if (xfs_sb_version_hascrc(from)) {
> + to->sb_features_compat = cpu_to_be32(from->sb_features_compat);
> + to->sb_features_ro_compat =
> + cpu_to_be32(from->sb_features_ro_compat);
> + to->sb_features_incompat =
> + cpu_to_be32(from->sb_features_incompat);
> + to->sb_features_log_incompat =
> + cpu_to_be32(from->sb_features_log_incompat);
> + to->sb_pad = 0;
> + to->sb_lsn = cpu_to_be64(from->sb_lsn);
> }
> }
>
> @@ -823,35 +751,13 @@ xfs_initialize_perag_data(
> * access.
> */
> void
> -xfs_mod_sb(xfs_trans_t *tp, __int64_t fields)
> +xfs_mod_sb(
> + struct xfs_trans *tp)
> {
> - xfs_buf_t *bp;
> - int first;
> - int last;
> - xfs_mount_t *mp;
> - xfs_sb_field_t f;
> -
> - ASSERT(fields);
> - if (!fields)
> - return;
> - mp = tp->t_mountp;
> - bp = xfs_trans_getsb(tp, mp, 0);
> - first = sizeof(xfs_sb_t);
> - last = 0;
> -
> - /* translate/copy */
> -
> - xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, fields);
> -
> - /* find modified range */
> - f = (xfs_sb_field_t)xfs_highbit64((__uint64_t)fields);
> - ASSERT((1LL << f) & XFS_SB_MOD_BITS);
> - last = xfs_sb_info[f + 1].offset - 1;
> -
> - f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
> - ASSERT((1LL << f) & XFS_SB_MOD_BITS);
> - first = xfs_sb_info[f].offset;
> + struct xfs_mount *mp = tp->t_mountp;
> + struct xfs_buf *bp = xfs_trans_getsb(tp, mp, 0);
>
> + xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
> xfs_trans_buf_set_type(tp, bp, XFS_BLFT_SB_BUF);
> - xfs_trans_log_buf(tp, bp, first, last);
> + xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsb));
> }
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 8eb1c54..e193caa 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -27,11 +27,11 @@ extern struct xfs_perag *xfs_perag_get_tag(struct xfs_mount *, xfs_agnumber_t,
> 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 *);
> -extern void xfs_mod_sb(struct xfs_trans *, __int64_t);
> -extern void xfs_sb_mount_common(struct xfs_mount *, struct xfs_sb *);
> -extern void xfs_sb_from_disk(struct xfs_sb *, struct xfs_dsb *);
> -extern void xfs_sb_to_disk(struct xfs_dsb *, struct xfs_sb *, __int64_t);
> +extern void xfs_sb_calc_crc(struct xfs_buf *bp);
> +extern void xfs_mod_sb(struct xfs_trans *tp);
> +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);
> extern void xfs_sb_quota_from_disk(struct xfs_sb *sbp);
>
> #endif /* __XFS_SB_H__ */
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index fdc6422..82af857 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -541,7 +541,7 @@ xfs_growfs_data_private(
> saved_error = error;
> continue;
> }
> - xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, XFS_SB_ALL_BITS);
> + xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
>
> error = xfs_bwrite(bp);
> xfs_buf_relse(bp);
> @@ -780,9 +780,7 @@ xfs_fs_log_dummy(
> xfs_trans_cancel(tp, 0);
> return error;
> }
> -
> - /* log the UUID because it is an unchanging field */
> - xfs_mod_sb(tp, XFS_SB_UUID);
> + xfs_mod_sb(tp);
> xfs_trans_set_sync(tp);
> return xfs_trans_commit(tp, 0);
> }
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 71d2c97..defcf69 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -613,7 +613,7 @@ xfs_mount_reset_sbqflags(
> return error;
> }
>
> - xfs_mod_sb(tp, XFS_SB_QFLAGS);
> + xfs_mod_sb(tp);
> return xfs_trans_commit(tp, 0);
> }
>
> @@ -896,7 +896,7 @@ xfs_mountfs(
> * 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, mp->m_update_flags);
> + error = xfs_mount_log_sb(mp);
> if (error) {
> xfs_warn(mp, "failed to write sb changes");
> goto out_rtunmount;
> @@ -1126,7 +1126,7 @@ xfs_log_sbcount(xfs_mount_t *mp)
> return error;
> }
>
> - xfs_mod_sb(tp, XFS_SB_IFREE | XFS_SB_ICOUNT | XFS_SB_FDBLOCKS);
> + xfs_mod_sb(tp);
> xfs_trans_set_sync(tp);
> error = xfs_trans_commit(tp, 0);
> return error;
> @@ -1429,14 +1429,10 @@ xfs_freesb(
> */
> int
> xfs_mount_log_sb(
> - xfs_mount_t *mp,
> - __int64_t fields)
> + struct xfs_mount *mp)
> {
> - xfs_trans_t *tp;
> - int error;
> -
> - ASSERT(fields & (XFS_SB_UNIT | XFS_SB_WIDTH | XFS_SB_UUID |
> - XFS_SB_FEATURES2 | XFS_SB_VERSIONNUM));
> + 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);
> @@ -1444,9 +1440,8 @@ xfs_mount_log_sb(
> xfs_trans_cancel(tp, 0);
> return error;
> }
> - xfs_mod_sb(tp, fields);
> - error = xfs_trans_commit(tp, 0);
> - return error;
> + xfs_mod_sb(tp);
> + return xfs_trans_commit(tp, 0);
> }
>
> /*
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 22ccf69..28b341b 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -378,7 +378,7 @@ extern void xfs_unmountfs(xfs_mount_t *);
> extern int xfs_mod_incore_sb(xfs_mount_t *, xfs_sb_field_t, int64_t, int);
> extern int xfs_mod_incore_sb_batch(xfs_mount_t *, xfs_mod_sb_t *,
> uint, int);
> -extern int xfs_mount_log_sb(xfs_mount_t *, __int64_t);
> +extern int xfs_mount_log_sb(xfs_mount_t *);
> extern struct xfs_buf *xfs_getsb(xfs_mount_t *, int);
> extern int xfs_readsb(xfs_mount_t *, int);
> extern void xfs_freesb(xfs_mount_t *);
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 79fb19d..c815a80 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -714,7 +714,6 @@ STATIC int
> xfs_qm_qino_alloc(
> xfs_mount_t *mp,
> xfs_inode_t **ip,
> - __int64_t sbfields,
> uint flags)
> {
> xfs_trans_t *tp;
> @@ -777,11 +776,6 @@ xfs_qm_qino_alloc(
> spin_lock(&mp->m_sb_lock);
> if (flags & XFS_QMOPT_SBVERSION) {
> ASSERT(!xfs_sb_version_hasquota(&mp->m_sb));
> - ASSERT((sbfields & (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> - XFS_SB_GQUOTINO | XFS_SB_PQUOTINO | XFS_SB_QFLAGS)) ==
> - (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> - XFS_SB_GQUOTINO | XFS_SB_PQUOTINO |
> - XFS_SB_QFLAGS));
>
> xfs_sb_version_addquota(&mp->m_sb);
> mp->m_sb.sb_uquotino = NULLFSINO;
> @@ -798,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, sbfields);
> + xfs_mod_sb(tp);
>
> if ((error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES))) {
> xfs_alert(mp, "%s failed (error %d)!", __func__, error);
> @@ -1451,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, XFS_SB_QFLAGS)) {
> + if (xfs_qm_write_sb_changes(mp)) {
> /*
> * We could only have been turning quotas off.
> * We aren't in very good shape actually because
> @@ -1482,7 +1476,6 @@ xfs_qm_init_quotainos(
> struct xfs_inode *gip = NULL;
> struct xfs_inode *pip = NULL;
> int error;
> - __int64_t sbflags = 0;
> uint flags = 0;
>
> ASSERT(mp->m_quotainfo);
> @@ -1517,9 +1510,6 @@ xfs_qm_init_quotainos(
> }
> } else {
> flags |= XFS_QMOPT_SBVERSION;
> - sbflags |= (XFS_SB_VERSIONNUM | XFS_SB_UQUOTINO |
> - XFS_SB_GQUOTINO | XFS_SB_PQUOTINO |
> - XFS_SB_QFLAGS);
> }
>
> /*
> @@ -1530,7 +1520,6 @@ xfs_qm_init_quotainos(
> */
> if (XFS_IS_UQUOTA_ON(mp) && uip == NULL) {
> error = xfs_qm_qino_alloc(mp, &uip,
> - sbflags | XFS_SB_UQUOTINO,
> flags | XFS_QMOPT_UQUOTA);
> if (error)
> goto error_rele;
> @@ -1539,7 +1528,6 @@ xfs_qm_init_quotainos(
> }
> if (XFS_IS_GQUOTA_ON(mp) && gip == NULL) {
> error = xfs_qm_qino_alloc(mp, &gip,
> - sbflags | XFS_SB_GQUOTINO,
> flags | XFS_QMOPT_GQUOTA);
> if (error)
> goto error_rele;
> @@ -1548,7 +1536,6 @@ xfs_qm_init_quotainos(
> }
> if (XFS_IS_PQUOTA_ON(mp) && pip == NULL) {
> error = xfs_qm_qino_alloc(mp, &pip,
> - sbflags | XFS_SB_PQUOTINO,
> flags | XFS_QMOPT_PQUOTA);
> if (error)
> goto error_rele;
> @@ -1593,8 +1580,7 @@ xfs_qm_dqfree_one(
> */
> int
> xfs_qm_write_sb_changes(
> - xfs_mount_t *mp,
> - __int64_t flags)
> + struct xfs_mount *mp)
> {
> xfs_trans_t *tp;
> int error;
> @@ -1606,10 +1592,8 @@ xfs_qm_write_sb_changes(
> return error;
> }
>
> - xfs_mod_sb(tp, flags);
> - error = xfs_trans_commit(tp, 0);
> -
> - return error;
> + xfs_mod_sb(tp);
> + return xfs_trans_commit(tp, 0);
> }
>
>
> diff --git a/fs/xfs/xfs_qm.h b/fs/xfs/xfs_qm.h
> index 3a07a93..bddd23f 100644
> --- a/fs/xfs/xfs_qm.h
> +++ b/fs/xfs/xfs_qm.h
> @@ -157,7 +157,7 @@ struct xfs_dquot_acct {
> #define XFS_QM_RTBWARNLIMIT 5
>
> extern void xfs_qm_destroy_quotainfo(struct xfs_mount *);
> -extern int xfs_qm_write_sb_changes(struct xfs_mount *, __int64_t);
> +extern int xfs_qm_write_sb_changes(struct xfs_mount *);
>
> /* dquot stuff */
> extern void xfs_qm_dqpurge_all(struct xfs_mount *, uint);
> diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> index 74fca68..8d7e5f0 100644
> --- a/fs/xfs/xfs_qm_syscalls.c
> +++ b/fs/xfs/xfs_qm_syscalls.c
> @@ -92,8 +92,7 @@ xfs_qm_scall_quotaoff(
> mutex_unlock(&q->qi_quotaofflock);
>
> /* XXX what to do if error ? Revert back to old vals incore ? */
> - error = xfs_qm_write_sb_changes(mp, XFS_SB_QFLAGS);
> - return error;
> + return xfs_qm_write_sb_changes(mp);
> }
>
> dqtype = 0;
> @@ -314,7 +313,6 @@ xfs_qm_scall_quotaon(
> {
> int error;
> uint qf;
> - __int64_t sbflags;
>
> flags &= (XFS_ALL_QUOTA_ACCT | XFS_ALL_QUOTA_ENFD);
> /*
> @@ -322,8 +320,6 @@ xfs_qm_scall_quotaon(
> */
> flags &= ~(XFS_ALL_QUOTA_ACCT);
>
> - sbflags = 0;
> -
> if (flags == 0) {
> xfs_debug(mp, "%s: zero flags, m_qflags=%x",
> __func__, mp->m_qflags);
> @@ -370,11 +366,10 @@ xfs_qm_scall_quotaon(
> /*
> * There's nothing to change if it's the same.
> */
> - if ((qf & flags) == flags && sbflags == 0)
> + if ((qf & flags) == flags)
> return -EEXIST;
> - sbflags |= XFS_SB_QFLAGS;
>
> - if ((error = xfs_qm_write_sb_changes(mp, sbflags)))
> + if ((error = xfs_qm_write_sb_changes(mp)))
> return error;
> /*
> * If we aren't trying to switch on quota enforcement, we are done.
> @@ -801,7 +796,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_SB_QFLAGS);
> + xfs_mod_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 40bad75..3797a03 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1258,7 +1258,7 @@ xfs_fs_remount(
> * might have some superblock changes to update.
> */
> if (mp->m_update_flags) {
> - error = xfs_mount_log_sb(mp, mp->m_update_flags);
> + error = xfs_mount_log_sb(mp);
> if (error) {
> xfs_warn(mp, "failed to write sb changes");
> return error;
> --
> 2.0.0
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
