On Thu, Jan 08, 2015 at 09:36:33AM -0500, Brian Foster wrote:
> On Thu, Jan 08, 2015 at 08:51:04AM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
>
> A question and one other little thing...
[snip]
> > #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.
With delayed logging, userspace will never see these as they never
get to disk. Userspace will only ever see checkpoints and unmount
records. My plan is to eventually remove this stuff from the kernel
(it isn['t used anymore except in tracing) and define it in
xfs_logprint in a way that works for current and legacy filesystems.
So for the moment, I'm just making the kernel/user values
consistent.
> > @@ -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:
Good catch, I'll kill it.
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
Thanks!
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
