On Fri, May 01, 2020 at 01:07:03AM -0700, Christoph Hellwig wrote:
> Any comments?
>
> On Thu, Apr 09, 2020 at 09:36:50AM +0200, Christoph Hellwig wrote:
> > Instead of marking the transaction sync using a flag, often based on
> > copy and pasted mount options checks, pass an enum of sync modes
> > to xfs_trans_commit that clearly documents the intent.
> >
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > ---
I recall looking at this when it was first posted and my first reaction
was that I didn't really like the interface. I decided to think about it
to see if it grew on me and then just lost track (sorry). It's not so
much passing a flag to commit as opposed to the flags not directly
controlling behavior (i.e., one flag means sync if <something> is true,
another flag means sync if <something else> is true, etc.) tends to
confuse me. I don't feel terribly strongly about it if others prefer
this pattern, but I still find the existing code more readable.
I vaguely recall thinking it might be nice if we could dump this into
transaction state to avoid the aforementioned logic warts, but IIRC that
might not have been possible for all users of this functionality..
Brian
> > fs/xfs/libxfs/xfs_attr.c | 15 ++--------
> > fs/xfs/libxfs/xfs_bmap.c | 4 +--
> > fs/xfs/libxfs/xfs_refcount.c | 2 +-
> > fs/xfs/libxfs/xfs_sb.c | 7 ++---
> > fs/xfs/libxfs/xfs_shared.h | 1 -
> > fs/xfs/scrub/scrub.c | 2 +-
> > fs/xfs/xfs_aops.c | 2 +-
> > fs/xfs/xfs_attr_inactive.c | 2 +-
> > fs/xfs/xfs_bmap_item.c | 2 +-
> > fs/xfs/xfs_bmap_util.c | 19 ++++--------
> > fs/xfs/xfs_dquot.c | 2 +-
> > fs/xfs/xfs_extfree_item.c | 2 +-
> > fs/xfs/xfs_file.c | 4 +--
> > fs/xfs/xfs_fsops.c | 6 ++--
> > fs/xfs/xfs_inode.c | 58 +++++-------------------------------
> > fs/xfs/xfs_ioctl.c | 9 ++----
> > fs/xfs/xfs_iomap.c | 4 +--
> > fs/xfs/xfs_iops.c | 11 ++-----
> > fs/xfs/xfs_log_recover.c | 4 +--
> > fs/xfs/xfs_pnfs.c | 3 +-
> > fs/xfs/xfs_qm.c | 2 +-
> > fs/xfs/xfs_qm_syscalls.c | 10 +++----
> > fs/xfs/xfs_refcount_item.c | 2 +-
> > fs/xfs/xfs_reflink.c | 14 ++++-----
> > fs/xfs/xfs_rmap_item.c | 2 +-
> > fs/xfs/xfs_rtalloc.c | 6 ++--
> > fs/xfs/xfs_super.c | 2 +-
> > fs/xfs/xfs_symlink.c | 13 ++------
> > fs/xfs/xfs_trans.c | 32 +++++++++++++++-----
> > fs/xfs/xfs_trans.h | 15 +++++-----
> > 30 files changed, 94 insertions(+), 163 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> > index e4fe3dca9883..a6a94bbea8a7 100644
> > --- a/fs/xfs/libxfs/xfs_attr.c
> > +++ b/fs/xfs/libxfs/xfs_attr.c
> > @@ -175,7 +175,6 @@ xfs_attr_try_sf_addname(
> > struct xfs_da_args *args)
> > {
> >
> > - struct xfs_mount *mp = dp->i_mount;
> > int error, error2;
> >
> > error = xfs_attr_shortform_addname(args);
> > @@ -189,10 +188,7 @@ xfs_attr_try_sf_addname(
> > if (!error && !(args->op_flags & XFS_DA_OP_NOTIME))
> > xfs_trans_ichgtime(args->trans, dp, XFS_ICHGTIME_CHG);
> >
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(args->trans);
> > -
> > - error2 = xfs_trans_commit(args->trans);
> > + error2 = xfs_trans_commit(args->trans, XFS_TRANS_COMMIT_WSYNC);
> > args->trans = NULL;
> > return error ? error : error2;
> > }
> > @@ -382,13 +378,6 @@ xfs_attr_set(
> > goto out_trans_cancel;
> > }
> >
> > - /*
> > - * If this is a synchronous mount, make sure that the
> > - * transaction goes to disk before returning to the user.
> > - */
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(args->trans);
> > -
> > if (!(args->op_flags & XFS_DA_OP_NOTIME))
> > xfs_trans_ichgtime(args->trans, dp, XFS_ICHGTIME_CHG);
> >
> > @@ -396,7 +385,7 @@ xfs_attr_set(
> > * Commit the last in the sequence of transactions.
> > */
> > xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE);
> > - error = xfs_trans_commit(args->trans);
> > + error = xfs_trans_commit(args->trans, XFS_TRANS_COMMIT_WSYNC);
> > out_unlock:
> > xfs_iunlock(dp, XFS_ILOCK_EXCL);
> > return error;
> > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> > index fda13cd7add0..aa16990e9b5f 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -1148,7 +1148,7 @@ xfs_bmap_add_attrfork(
> > xfs_log_sb(tp);
> > }
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > return error;
> >
> > @@ -4644,7 +4644,7 @@ xfs_bmapi_convert_delalloc(
> > goto out_finish;
> >
> > xfs_bmapi_finish(&bma, whichfork, 0);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > return error;
> >
> > diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> > index 2076627243b0..095ab1202643 100644
> > --- a/fs/xfs/libxfs/xfs_refcount.c
> > +++ b/fs/xfs/libxfs/xfs_refcount.c
> > @@ -1749,7 +1749,7 @@ xfs_refcount_recover_cow_leftovers(
> > /* Free the block. */
> > xfs_bmap_add_free(tp, fsb, rr->rr_rrec.rc_blockcount, NULL);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto out_free;
> >
> > diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> > index c526c5e5ab76..c30ba1a6e819 100644
> > --- a/fs/xfs/libxfs/xfs_sb.c
> > +++ b/fs/xfs/libxfs/xfs_sb.c
> > @@ -990,9 +990,7 @@ xfs_sync_sb(
> > return error;
> >
> > xfs_log_sb(tp);
> > - if (wait)
> > - xfs_trans_set_sync(tp);
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, wait ? XFS_TRANS_COMMIT_SYNC : 0);
> > }
> >
> > /*
> > @@ -1087,8 +1085,7 @@ xfs_sync_sb_buf(
> > bp = xfs_trans_getsb(tp, mp);
> > xfs_log_sb(tp);
> > xfs_trans_bhold(tp, bp);
> > - xfs_trans_set_sync(tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> > if (error)
> > goto out;
> > /*
> > diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
> > index c45acbd3add9..586d5c0c912b 100644
> > --- a/fs/xfs/libxfs/xfs_shared.h
> > +++ b/fs/xfs/libxfs/xfs_shared.h
> > @@ -61,7 +61,6 @@ void xfs_log_get_max_trans_res(struct xfs_mount *mp,
> > #define XFS_TRANS_DIRTY 0x01 /* something needs to be logged */
> > #define XFS_TRANS_SB_DIRTY 0x02 /* superblock is modified */
> > #define XFS_TRANS_PERM_LOG_RES 0x04 /* xact took a permanent log res */
> > -#define XFS_TRANS_SYNC 0x08 /* make commit synchronous */
> > #define XFS_TRANS_DQ_DIRTY 0x10 /* at least one dquot in trx dirty */
> > #define XFS_TRANS_RESERVE 0x20 /* OK to use reserved data blocks */
> > #define XFS_TRANS_NO_WRITECOUNT 0x40 /* do not elevate SB writecount */
> > diff --git a/fs/xfs/scrub/scrub.c b/fs/xfs/scrub/scrub.c
> > index 8ebf35b115ce..67bb71c98003 100644
> > --- a/fs/xfs/scrub/scrub.c
> > +++ b/fs/xfs/scrub/scrub.c
> > @@ -155,7 +155,7 @@ xchk_teardown(
> > xchk_ag_free(sc, &sc->sa);
> > if (sc->tp) {
> > if (error == 0 && (sc->sm->sm_flags & XFS_SCRUB_IFLAG_REPAIR))
> > - error = xfs_trans_commit(sc->tp);
> > + error = xfs_trans_commit(sc->tp, 0);
> > else
> > xfs_trans_cancel(sc->tp);
> > sc->tp = NULL;
> > diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> > index 9d9cebf18726..c0504d5144ee 100644
> > --- a/fs/xfs/xfs_aops.c
> > +++ b/fs/xfs/xfs_aops.c
> > @@ -92,7 +92,7 @@ __xfs_setfilesize(
> > xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> > xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> >
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, 0);
> > }
> >
> > int
> > diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> > index c42f90e16b4f..c07389d07724 100644
> > --- a/fs/xfs/xfs_attr_inactive.c
> > +++ b/fs/xfs/xfs_attr_inactive.c
> > @@ -380,7 +380,7 @@ xfs_attr_inactive(
> > /* Reset the attribute fork - this also destroys the in-core fork */
> > xfs_attr_fork_remove(dp, trans);
> >
> > - error = xfs_trans_commit(trans);
> > + error = xfs_trans_commit(trans, 0);
> > xfs_iunlock(dp, lock_mode);
> > return error;
> >
> > diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> > index ee6f4229cebc..6d07581f8fc0 100644
> > --- a/fs/xfs/xfs_bmap_item.c
> > +++ b/fs/xfs/xfs_bmap_item.c
> > @@ -548,7 +548,7 @@ xfs_bui_recover(
> >
> > set_bit(XFS_BUI_RECOVERED, &buip->bui_flags);
> > xfs_defer_move(parent_tp, tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > xfs_irele(ip);
> >
> > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> > index 4f800f7fe888..f1b24fa01432 100644
> > --- a/fs/xfs/xfs_bmap_util.c
> > +++ b/fs/xfs/xfs_bmap_util.c
> > @@ -702,7 +702,7 @@ xfs_free_eofblocks(
> > */
> > xfs_trans_cancel(tp);
> > } else {
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (!error)
> > xfs_inode_clear_eofblocks_tag(ip);
> > }
> > @@ -833,7 +833,7 @@ xfs_alloc_file_space(
> > /*
> > * Complete the transaction
> > */
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > if (error)
> > break;
> > @@ -890,7 +890,7 @@ xfs_unmap_extent(
> > if (error)
> > goto out_trans_cancel;
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > out_unlock:
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > return error;
> > @@ -1098,7 +1098,7 @@ xfs_collapse_file_space(
> > goto out_trans_cancel;
> > }
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > return error;
> >
> > @@ -1175,7 +1175,7 @@ xfs_insert_file_space(
> > goto out_trans_cancel;
> > } while (!done);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > return error;
> >
> > @@ -1753,14 +1753,7 @@ xfs_swap_extents(
> > goto out_trans_cancel;
> > }
> >
> > - /*
> > - * If this is a synchronous mount, make sure that the
> > - * transaction goes to disk before returning to the user.
> > - */
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(tp);
> > -
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_WSYNC);
> >
> > trace_xfs_swap_extent_after(ip, 0);
> > trace_xfs_swap_extent_after(tip, 1);
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index af2c8e5ceea0..fec166e2dd11 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -530,7 +530,7 @@ xfs_qm_dqread_alloc(
> > if (error)
> > goto err_cancel;
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error) {
> > /*
> > * Buffer was held to the transaction, so we have to unlock it
> > diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> > index 6ea847f6e298..7572cddca074 100644
> > --- a/fs/xfs/xfs_extfree_item.c
> > +++ b/fs/xfs/xfs_extfree_item.c
> > @@ -645,7 +645,7 @@ xfs_efi_recover(
> > }
> >
> > set_bit(XFS_EFI_RECOVERED, &efip->efi_flags);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > return error;
> >
> > abort_error:
> > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> > index 4b8bdecc3863..45343e6d1643 100644
> > --- a/fs/xfs/xfs_file.c
> > +++ b/fs/xfs/xfs_file.c
> > @@ -61,9 +61,7 @@ xfs_update_prealloc_flags(
> > ip->i_d.di_flags &= ~XFS_DIFLAG_PREALLOC;
> >
> > xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> > - if (flags & XFS_PREALLOC_SYNC)
> > - xfs_trans_set_sync(tp);
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> > }
> >
> > /*
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index 3e61d0cc23f8..a45ab9397901 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -128,8 +128,7 @@ xfs_growfs_data_private(
> > nb - mp->m_sb.sb_dblocks);
> > if (id.nfree)
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_FDBLOCKS, id.nfree);
> > - xfs_trans_set_sync(tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> > if (error)
> > return error;
> >
> > @@ -209,8 +208,7 @@ xfs_growfs_imaxpct(
> >
> > dpct = imaxpct - mp->m_sb.sb_imax_pct;
> > xfs_trans_mod_sb(tp, XFS_TRANS_SB_IMAXPCT, dpct);
> > - xfs_trans_set_sync(tp);
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> > }
> >
> > /*
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index d1772786af29..e04587a23bfa 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -1216,14 +1216,6 @@ xfs_create(
> > xfs_bumplink(tp, dp);
> > }
> >
> > - /*
> > - * If this is a synchronous mount, make sure that the
> > - * create transaction goes to disk before returning to
> > - * the user.
> > - */
> > - if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
> > - xfs_trans_set_sync(tp);
> > -
> > /*
> > * Attach the dquot(s) to the inodes and modify them incore.
> > * These ids of the inode couldn't have changed since the new
> > @@ -1231,7 +1223,7 @@ xfs_create(
> > */
> > xfs_qm_vop_create_dqattach(tp, ip, udqp, gdqp, pdqp);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_DIRSYNC);
> > if (error)
> > goto out_release_inode;
> >
> > @@ -1311,9 +1303,6 @@ xfs_create_tmpfile(
> > if (error)
> > goto out_trans_cancel;
> >
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(tp);
> > -
> > /*
> > * Attach the dquot(s) to the inodes and modify them incore.
> > * These ids of the inode couldn't have changed since the new
> > @@ -1325,7 +1314,7 @@ xfs_create_tmpfile(
> > if (error)
> > goto out_trans_cancel;
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_WSYNC);
> > if (error)
> > goto out_release_inode;
> >
> > @@ -1430,16 +1419,7 @@ xfs_link(
> > xfs_trans_log_inode(tp, tdp, XFS_ILOG_CORE);
> >
> > xfs_bumplink(tp, sip);
> > -
> > - /*
> > - * If this is a synchronous mount, make sure that the
> > - * link transaction goes to disk before returning to
> > - * the user.
> > - */
> > - if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
> > - xfs_trans_set_sync(tp);
> > -
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, XFS_TRANS_COMMIT_DIRSYNC);
> >
> > error_return:
> > xfs_trans_cancel(tp);
> > @@ -1688,7 +1668,7 @@ xfs_inactive_truncate(
> >
> > ASSERT(ip->i_d.di_nextents == 0);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto error_unlock;
> >
> > @@ -1773,7 +1753,7 @@ xfs_inactive_ifree(
> > * Just ignore errors at this point. There is nothing we can do except
> > * to try to keep going. Make sure it's not a silent error.
> > */
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > xfs_notice(mp, "%s: xfs_trans_commit returned error %d",
> > __func__, error);
> > @@ -2957,15 +2937,7 @@ xfs_remove(
> > goto out_trans_cancel;
> > }
> >
> > - /*
> > - * If this is a synchronous mount, make sure that the
> > - * remove transaction goes to disk before returning to
> > - * the user.
> > - */
> > - if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
> > - xfs_trans_set_sync(tp);
> > -
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_DIRSYNC);
> > if (error)
> > goto std_return;
> >
> > @@ -3031,20 +3003,6 @@ xfs_sort_for_rename(
> > }
> > }
> >
> > -static int
> > -xfs_finish_rename(
> > - struct xfs_trans *tp)
> > -{
> > - /*
> > - * If this is a synchronous mount, make sure that the rename transaction
> > - * goes to disk before returning to the user.
> > - */
> > - if (tp->t_mountp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
> > - xfs_trans_set_sync(tp);
> > -
> > - return xfs_trans_commit(tp);
> > -}
> > -
> > /*
> > * xfs_cross_rename()
> > *
> > @@ -3147,7 +3105,7 @@ xfs_cross_rename(
> > }
> > xfs_trans_ichgtime(tp, dp1, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> > xfs_trans_log_inode(tp, dp1, XFS_ILOG_CORE);
> > - return xfs_finish_rename(tp);
> > + return xfs_trans_commit(tp, XFS_TRANS_COMMIT_DIRSYNC);
> >
> > out_trans_abort:
> > xfs_trans_cancel(tp);
> > @@ -3471,7 +3429,7 @@ xfs_rename(
> > if (new_parent)
> > xfs_trans_log_inode(tp, target_dp, XFS_ILOG_CORE);
> >
> > - error = xfs_finish_rename(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_DIRSYNC);
> > if (wip)
> > xfs_irele(wip);
> > return error;
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index cdfb3cd9a25b..c2e3d8181dd8 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1381,10 +1381,6 @@ xfs_ioctl_setattr_get_trans(
> > error = -EPERM;
> > goto out_cancel;
> > }
> > -
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(tp);
> > -
> > return tp;
> >
> > out_cancel:
> > @@ -1620,7 +1616,8 @@ xfs_ioctl_setattr(
> > else
> > ip->i_d.di_cowextsize = 0;
> >
> > - code = xfs_trans_commit(tp);
> > +
> > + code = xfs_trans_commit(tp, XFS_TRANS_COMMIT_WSYNC);
> >
> > /*
> > * Release any dquot(s) the inode had kept before chown.
> > @@ -1729,7 +1726,7 @@ xfs_ioc_setxflags(
> > goto out_drop_write;
> > }
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_WSYNC);
> > out_drop_write:
> > mnt_drop_write_file(filp);
> > return error;
> > diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> > index bb590a267a7f..52c0fabe9117 100644
> > --- a/fs/xfs/xfs_iomap.c
> > +++ b/fs/xfs/xfs_iomap.c
> > @@ -265,7 +265,7 @@ xfs_iomap_write_direct(
> > /*
> > * Complete the transaction
> > */
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto out_unlock;
> >
> > @@ -593,7 +593,7 @@ xfs_iomap_write_unwritten(
> > xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> > }
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > if (error)
> > return error;
> > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > index f7a99b3bbcf7..e95edd550ca8 100644
> > --- a/fs/xfs/xfs_iops.c
> > +++ b/fs/xfs/xfs_iops.c
> > @@ -789,9 +789,7 @@ xfs_setattr_nonsize(
> >
> > XFS_STATS_INC(mp, xs_ig_attrchg);
> >
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_WSYNC);
> >
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> >
> > @@ -1028,10 +1026,7 @@ xfs_setattr_size(
> >
> > XFS_STATS_INC(mp, xs_ig_attrchg);
> >
> > - if (mp->m_flags & XFS_MOUNT_WSYNC)
> > - xfs_trans_set_sync(tp);
> > -
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_WSYNC);
> > out_unlock:
> > if (lock_flags)
> > xfs_iunlock(ip, lock_flags);
> > @@ -1125,7 +1120,7 @@ xfs_vn_update_time(
> >
> > xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> > xfs_trans_log_inode(tp, ip, log_flags);
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, 0);
> > }
> >
> > STATIC int
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 11c3502b07b1..0e7c10911345 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -4768,7 +4768,7 @@ xlog_finish_defer_ops(
> > /* transfer all collected dfops to this transaction */
> > xfs_defer_move(tp, parent_tp);
> >
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, 0);
> > }
> >
> > /*
> > @@ -4954,7 +4954,7 @@ xlog_recover_clear_agi_bucket(
> > xfs_trans_log_buf(tp, agibp, offset,
> > (offset + sizeof(xfs_agino_t) - 1));
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto out_error;
> > return;
> > diff --git a/fs/xfs/xfs_pnfs.c b/fs/xfs/xfs_pnfs.c
> > index bb3008d390aa..c24a760f5194 100644
> > --- a/fs/xfs/xfs_pnfs.c
> > +++ b/fs/xfs/xfs_pnfs.c
> > @@ -290,8 +290,7 @@ xfs_fs_commit_blocks(
> > ip->i_d.di_size = iattr->ia_size;
> > }
> >
> > - xfs_trans_set_sync(tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> >
> > out_drop_iolock:
> > xfs_iunlock(ip, XFS_IOLOCK_EXCL);
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index c225691fad15..6eb0d44429de 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -818,7 +818,7 @@ xfs_qm_qino_alloc(
> > spin_unlock(&mp->m_sb_lock);
> > xfs_log_sb(tp);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error) {
> > ASSERT(XFS_FORCED_SHUTDOWN(mp));
> > xfs_alert(mp, "%s failed (error %d)!", __func__, error);
> > diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
> > index 5d5ac65aa1cc..982083b1c142 100644
> > --- a/fs/xfs/xfs_qm_syscalls.c
> > +++ b/fs/xfs/xfs_qm_syscalls.c
> > @@ -47,8 +47,7 @@ xfs_qm_log_quotaoff(
> > * return and actually stop quota accounting. So, make it synchronous.
> > * We don't care about quotoff's performance.
> > */
> > - xfs_trans_set_sync(tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> > if (error)
> > goto out;
> >
> > @@ -81,8 +80,7 @@ xfs_qm_log_quotaoff_end(
> > * return and actually stop quota accounting. So, make it synchronous.
> > * We don't care about quotoff's performance.
> > */
> > - xfs_trans_set_sync(tp);
> > - return xfs_trans_commit(tp);
> > + return xfs_trans_commit(tp, XFS_TRANS_COMMIT_SYNC);
> > }
> >
> > /*
> > @@ -305,7 +303,7 @@ xfs_qm_scall_trunc_qfile(
> > ASSERT(ip->i_d.di_nextents == 0);
> >
> > xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> >
> > out_unlock:
> > xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
> > @@ -593,7 +591,7 @@ xfs_qm_scall_setqlim(
> > dqp->dq_flags |= XFS_DQ_DIRTY;
> > xfs_trans_log_dquot(tp, dqp);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> >
> > out_rele:
> > xfs_qm_dqrele(dqp);
> > diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> > index 8eeed73928cd..9786c1e22ca4 100644
> > --- a/fs/xfs/xfs_refcount_item.c
> > +++ b/fs/xfs/xfs_refcount_item.c
> > @@ -581,7 +581,7 @@ xfs_cui_recover(
> > xfs_refcount_finish_one_cleanup(tp, rcur, error);
> > set_bit(XFS_CUI_RECOVERED, &cuip->cui_flags);
> > xfs_defer_move(parent_tp, tp);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > return error;
> >
> > abort_error:
> > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> > index b0ce04ffd3cd..01e6691a1f61 100644
> > --- a/fs/xfs/xfs_reflink.c
> > +++ b/fs/xfs/xfs_reflink.c
> > @@ -414,7 +414,7 @@ xfs_reflink_allocate_cow(
> > goto out_unreserve;
> >
> > xfs_inode_set_cowblocks_tag(ip);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > return error;
> >
> > @@ -570,7 +570,7 @@ xfs_reflink_cancel_cow_range(
> > if (error)
> > goto out_cancel;
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> >
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > return error;
> > @@ -683,7 +683,7 @@ xfs_reflink_end_cow_extent(
> > /* Remove the mapping from the CoW fork. */
> > xfs_bmap_del_extent_cow(ip, &icur, &got, &del);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > if (error)
> > return error;
> > @@ -901,7 +901,7 @@ xfs_reflink_set_inode_flag(
> > xfs_iunlock(dest, XFS_ILOCK_EXCL);
> >
> > commit_flags:
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto out_error;
> > return error;
> > @@ -948,7 +948,7 @@ xfs_reflink_update_dest(
> >
> > xfs_trans_log_inode(tp, dest, XFS_ILOG_CORE);
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto out_error;
> > return error;
> > @@ -1088,7 +1088,7 @@ xfs_reflink_remap_extent(
> > goto out_cancel;
> > }
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > xfs_iunlock(ip, XFS_ILOCK_EXCL);
> > if (error)
> > goto out;
> > @@ -1493,7 +1493,7 @@ xfs_reflink_try_clear_inode_flag(
> > if (error)
> > goto cancel;
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > goto out;
> >
> > diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> > index 4911b68f95dd..29931d4fa0e7 100644
> > --- a/fs/xfs/xfs_rmap_item.c
> > +++ b/fs/xfs/xfs_rmap_item.c
> > @@ -598,7 +598,7 @@ xfs_rui_recover(
> >
> > xfs_rmap_finish_one_cleanup(tp, rcur, error);
> > set_bit(XFS_RUI_RECOVERED, &ruip->rui_flags);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > return error;
> >
> > abort_error:
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index 6209e7b6b895..6039fca2b34f 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> > @@ -800,7 +800,7 @@ xfs_growfs_rt_alloc(
> > /*
> > * Free any blocks freed up in the transaction, then commit.
> > */
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > return error;
> > /*
> > @@ -835,7 +835,7 @@ xfs_growfs_rt_alloc(
> > /*
> > * Commit the transaction.
> > */
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > return error;
> > }
> > @@ -1072,7 +1072,7 @@ xfs_growfs_rt(
> > mp->m_rsumlevels = nrsumlevels;
> > mp->m_rsumsize = nrsumsize;
> >
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error)
> > break;
> > }
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index abf06bf9c3f3..e652ca8a1ef7 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -633,7 +633,7 @@ xfs_fs_dirty_inode(
> > xfs_ilock(ip, XFS_ILOCK_EXCL);
> > xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> > xfs_trans_log_inode(tp, ip, XFS_ILOG_TIMESTAMP);
> > - xfs_trans_commit(tp);
> > + xfs_trans_commit(tp, 0);
> > }
> >
> > /*
> > diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
> > index 13fb4b919648..24e228c0f477 100644
> > --- a/fs/xfs/xfs_symlink.c
> > +++ b/fs/xfs/xfs_symlink.c
> > @@ -312,16 +312,7 @@ xfs_symlink(
> > xfs_trans_ichgtime(tp, dp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
> > xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
> >
> > - /*
> > - * If this is a synchronous mount, make sure that the
> > - * symlink transaction goes to disk before returning to
> > - * the user.
> > - */
> > - if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC)) {
> > - xfs_trans_set_sync(tp);
> > - }
> > -
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, XFS_TRANS_COMMIT_DIRSYNC);
> > if (error)
> > goto out_release_inode;
> >
> > @@ -439,7 +430,7 @@ xfs_inactive_symlink_rmt(
> > * rolls and commits the transaction that frees the extents.
> > */
> > xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> > - error = xfs_trans_commit(tp);
> > + error = xfs_trans_commit(tp, 0);
> > if (error) {
> > ASSERT(XFS_FORCED_SHUTDOWN(mp));
> > goto error_unlock;
> > diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> > index 28b983ff8b11..8aad8b3572ed 100644
> > --- a/fs/xfs/xfs_trans.c
> > +++ b/fs/xfs/xfs_trans.c
> > @@ -918,6 +918,23 @@ xfs_trans_committed_bulk(
> > spin_unlock(&ailp->ail_lock);
> > }
> >
> > +static inline bool
> > +xfs_trans_is_sync(
> > + struct xfs_mount *mp,
> > + enum xfs_commit_mode mode)
> > +{
> > + switch (mode) {
> > + case XFS_TRANS_COMMIT_SYNC:
> > + return true;
> > + case XFS_TRANS_COMMIT_DIRSYNC:
> > + return mp->m_flags & (XFS_MOUNT_WSYNC | XFS_MOUNT_DIRSYNC);
> > + case XFS_TRANS_COMMIT_WSYNC:
> > + return mp->m_flags & XFS_MOUNT_WSYNC;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > /*
> > * Commit the given transaction to the log.
> > *
> > @@ -933,12 +950,12 @@ xfs_trans_committed_bulk(
> > static int
> > __xfs_trans_commit(
> > struct xfs_trans *tp,
> > + enum xfs_commit_mode mode,
> > bool regrant)
> > {
> > struct xfs_mount *mp = tp->t_mountp;
> > xfs_lsn_t commit_lsn = -1;
> > int error = 0;
> > - int sync = tp->t_flags & XFS_TRANS_SYNC;
> >
> > trace_xfs_trans_commit(tp, _RET_IP_);
> >
> > @@ -984,10 +1001,10 @@ __xfs_trans_commit(
> > xfs_trans_free(tp);
> >
> > /*
> > - * If the transaction needs to be synchronous, then force the
> > - * log out now and wait for it.
> > + * If the transaction needs to be synchronous, then force the log out
> > + * now and wait for it.
> > */
> > - if (sync) {
> > + if (xfs_trans_is_sync(mp, mode)) {
> > error = xfs_log_force_lsn(mp, commit_lsn, XFS_LOG_SYNC, NULL);
> > XFS_STATS_INC(mp, xs_trans_sync);
> > } else {
> > @@ -1022,9 +1039,10 @@ __xfs_trans_commit(
> >
> > int
> > xfs_trans_commit(
> > - struct xfs_trans *tp)
> > + struct xfs_trans *tp,
> > + enum xfs_commit_mode mode)
> > {
> > - return __xfs_trans_commit(tp, false);
> > + return __xfs_trans_commit(tp, mode, false);
> > }
> >
> > /*
> > @@ -1111,7 +1129,7 @@ xfs_trans_roll(
> > * is in progress. The caller takes the responsibility to cancel
> > * the duplicate transaction that gets returned.
> > */
> > - error = __xfs_trans_commit(trans, true);
> > + error = __xfs_trans_commit(trans, 0, true);
> > if (error)
> > return error;
> >
> > diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> > index 752c7fef9de7..499b5da72df0 100644
> > --- a/fs/xfs/xfs_trans.h
> > +++ b/fs/xfs/xfs_trans.h
> > @@ -143,12 +143,6 @@ typedef struct xfs_trans {
> > unsigned long t_pflags; /* saved process flags state */
> > } xfs_trans_t;
> >
> > -/*
> > - * XFS transaction mechanism exported interfaces that are
> > - * actually macros.
> > - */
> > -#define xfs_trans_set_sync(tp) ((tp)->t_flags |= XFS_TRANS_SYNC)
> > -
> > #if defined(DEBUG) || defined(XFS_WARN)
> > #define xfs_trans_agblocks_delta(tp, d) ((tp)->t_ag_freeblks_delta += (int64_t)d)
> > #define xfs_trans_agflist_delta(tp, d) ((tp)->t_ag_flist_delta += (int64_t)d)
> > @@ -230,7 +224,14 @@ void xfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
> > bool xfs_trans_buf_is_dirty(struct xfs_buf *bp);
> > void xfs_trans_log_inode(xfs_trans_t *, struct xfs_inode *, uint);
> >
> > -int xfs_trans_commit(struct xfs_trans *);
> > +enum xfs_commit_mode {
> > + /* 0 means no sync required */
> > + XFS_TRANS_COMMIT_SYNC = 1, /* always commit synchronously */
> > + XFS_TRANS_COMMIT_WSYNC, /* ... only for wsync mounts */
> > + XFS_TRANS_COMMIT_DIRSYNC, /* ... for dirsync and wsync mounts */
> > +};
> > +
> > +int xfs_trans_commit(struct xfs_trans *, enum xfs_commit_mode mode);
> > int xfs_trans_roll(struct xfs_trans **);
> > int xfs_trans_roll_inode(struct xfs_trans **, struct xfs_inode *);
> > void xfs_trans_cancel(xfs_trans_t *);
> > --
> > 2.25.1
> >
> ---end quoted text---
>
