On Wed, May 19, 2021 at 10:12:54PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> We currently set the log ticket client ID when we reserve a
> transaction. This client ID is only ever written to the log by
> a CIL checkpoint or unmount records, and so anything using a high
> level transaction allocated through xfs_trans_alloc() does not need
> a log ticket client ID to be set.
>
> For the CIL checkpoint, the client ID written to the journal is
> always XFS_TRANSACTION, and for the unmount record it is always
> XFS_LOG, and nothing else writes to the log. All of these operations
> tell xlog_write() exactly what they need to write to the log (the
> optype) and build their own opheaders for start, commit and unmount
> records. Hence we no longer need to set the client id in either the
> log ticket or the xfs_trans.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
Yay for removing never-used macros,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/libxfs/xfs_log_format.h | 1 -
> fs/xfs/xfs_log.c | 47 ++++++----------------------------
> fs/xfs/xfs_log.h | 16 +++++-------
> fs/xfs/xfs_log_cil.c | 2 +-
> fs/xfs/xfs_log_priv.h | 10 ++------
> fs/xfs/xfs_trans.c | 6 ++---
> 6 files changed, 19 insertions(+), 63 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index d548ea4b6aab..78d5368a7caa 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -69,7 +69,6 @@ static inline uint xlog_get_cycle(char *ptr)
>
> /* Log Clients */
> #define XFS_TRANSACTION 0x69
> -#define XFS_VOLUME 0x2
> #define XFS_LOG 0xaa
>
> #define XLOG_UNMOUNT_TYPE 0x556e /* Un for Unmount */
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 76a73f4b0f30..ccf584914b6a 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -433,10 +433,9 @@ xfs_log_regrant(
> int
> xfs_log_reserve(
> struct xfs_mount *mp,
> - int unit_bytes,
> - int cnt,
> + int unit_bytes,
> + int cnt,
> struct xlog_ticket **ticp,
> - uint8_t client,
> bool permanent)
> {
> struct xlog *log = mp->m_log;
> @@ -444,15 +443,13 @@ xfs_log_reserve(
> int need_bytes;
> int error = 0;
>
> - ASSERT(client == XFS_TRANSACTION || client == XFS_LOG);
> -
> if (XLOG_FORCED_SHUTDOWN(log))
> return -EIO;
>
> XFS_STATS_INC(mp, xs_try_logspace);
>
> ASSERT(*ticp == NULL);
> - tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent);
> + tic = xlog_ticket_alloc(log, unit_bytes, cnt, permanent);
> *ticp = tic;
>
> xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt
> @@ -853,7 +850,7 @@ xlog_unmount_write(
> struct xlog_ticket *tic = NULL;
> int error;
>
> - error = xfs_log_reserve(mp, 600, 1, &tic, XFS_LOG, 0);
> + error = xfs_log_reserve(mp, 600, 1, &tic, 0);
> if (error)
> goto out_err;
>
> @@ -2181,35 +2178,13 @@ xlog_write_calc_vec_length(
>
> static xlog_op_header_t *
> xlog_write_setup_ophdr(
> - struct xlog *log,
> struct xlog_op_header *ophdr,
> - struct xlog_ticket *ticket,
> - uint flags)
> + struct xlog_ticket *ticket)
> {
> ophdr->oh_tid = cpu_to_be32(ticket->t_tid);
> - ophdr->oh_clientid = ticket->t_clientid;
> + ophdr->oh_clientid = XFS_TRANSACTION;
> ophdr->oh_res2 = 0;
> -
> - /* are we copying a commit or unmount record? */
> - ophdr->oh_flags = flags;
> -
> - /*
> - * We've seen logs corrupted with bad transaction client ids. This
> - * makes sure that XFS doesn't generate them on. Turn this into an EIO
> - * and shut down the filesystem.
> - */
> - switch (ophdr->oh_clientid) {
> - case XFS_TRANSACTION:
> - case XFS_VOLUME:
> - case XFS_LOG:
> - break;
> - default:
> - xfs_warn(log->l_mp,
> - "Bad XFS transaction clientid 0x%x in ticket "PTR_FMT,
> - ophdr->oh_clientid, ticket);
> - return NULL;
> - }
> -
> + ophdr->oh_flags = 0;
> return ophdr;
> }
>
> @@ -2439,11 +2414,7 @@ xlog_write(
> if (index)
> optype &= ~XLOG_START_TRANS;
> } else {
> - ophdr = xlog_write_setup_ophdr(log, ptr,
> - ticket, optype);
> - if (!ophdr)
> - return -EIO;
> -
> + ophdr = xlog_write_setup_ophdr(ptr, ticket);
> xlog_write_adv_cnt(&ptr, &len, &log_offset,
> sizeof(struct xlog_op_header));
> added_ophdr = true;
> @@ -3499,7 +3470,6 @@ xlog_ticket_alloc(
> struct xlog *log,
> int unit_bytes,
> int cnt,
> - char client,
> bool permanent)
> {
> struct xlog_ticket *tic;
> @@ -3517,7 +3487,6 @@ xlog_ticket_alloc(
> tic->t_cnt = cnt;
> tic->t_ocnt = cnt;
> tic->t_tid = prandom_u32();
> - tic->t_clientid = client;
> if (permanent)
> tic->t_flags |= XLOG_TIC_PERM_RESERV;
>
> diff --git a/fs/xfs/xfs_log.h b/fs/xfs/xfs_log.h
> index 1bd080ce3a95..c0c3141944ea 100644
> --- a/fs/xfs/xfs_log.h
> +++ b/fs/xfs/xfs_log.h
> @@ -117,16 +117,12 @@ int xfs_log_mount_finish(struct xfs_mount *mp);
> void xfs_log_mount_cancel(struct xfs_mount *);
> xfs_lsn_t xlog_assign_tail_lsn(struct xfs_mount *mp);
> xfs_lsn_t xlog_assign_tail_lsn_locked(struct xfs_mount *mp);
> -void xfs_log_space_wake(struct xfs_mount *mp);
> -int xfs_log_reserve(struct xfs_mount *mp,
> - int length,
> - int count,
> - struct xlog_ticket **ticket,
> - uint8_t clientid,
> - bool permanent);
> -int xfs_log_regrant(struct xfs_mount *mp, struct xlog_ticket *tic);
> -void xfs_log_unmount(struct xfs_mount *mp);
> -int xfs_log_force_umount(struct xfs_mount *mp, int logerror);
> +void xfs_log_space_wake(struct xfs_mount *mp);
> +int xfs_log_reserve(struct xfs_mount *mp, int length, int count,
> + struct xlog_ticket **ticket, bool permanent);
> +int xfs_log_regrant(struct xfs_mount *mp, struct xlog_ticket *tic);
> +void xfs_log_unmount(struct xfs_mount *mp);
> +int xfs_log_force_umount(struct xfs_mount *mp, int logerror);
> bool xfs_log_writable(struct xfs_mount *mp);
>
> struct xlog_ticket *xfs_log_ticket_get(struct xlog_ticket *ticket);
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 2983adaed675..9d3a495f1c78 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -37,7 +37,7 @@ xlog_cil_ticket_alloc(
> {
> struct xlog_ticket *tic;
>
> - tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0);
> + tic = xlog_ticket_alloc(log, 0, 1, 0);
>
> /*
> * set the current reservation to zero so we know to steal the basic
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 87447fa34c43..e4e3e71b2b1b 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -158,7 +158,6 @@ typedef struct xlog_ticket {
> int t_unit_res; /* unit reservation in bytes : 4 */
> char t_ocnt; /* original count : 1 */
> char t_cnt; /* current count : 1 */
> - char t_clientid; /* who does this belong to; : 1 */
> char t_flags; /* properties of reservation : 1 */
>
> /* reservation array fields */
> @@ -465,13 +464,8 @@ extern __le32 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead,
> char *dp, int size);
>
> extern kmem_zone_t *xfs_log_ticket_zone;
> -struct xlog_ticket *
> -xlog_ticket_alloc(
> - struct xlog *log,
> - int unit_bytes,
> - int count,
> - char client,
> - bool permanent);
> +struct xlog_ticket *xlog_ticket_alloc(struct xlog *log, int unit_bytes,
> + int count, bool permanent);
>
> static inline void
> xlog_write_adv_cnt(void **ptr, int *len, int *off, size_t bytes)
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index c214a69b573d..bc72826d1f97 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -194,11 +194,9 @@ xfs_trans_reserve(
> ASSERT(resp->tr_logflags & XFS_TRANS_PERM_LOG_RES);
> error = xfs_log_regrant(mp, tp->t_ticket);
> } else {
> - error = xfs_log_reserve(mp,
> - resp->tr_logres,
> + error = xfs_log_reserve(mp, resp->tr_logres,
> resp->tr_logcount,
> - &tp->t_ticket, XFS_TRANSACTION,
> - permanent);
> + &tp->t_ticket, permanent);
> }
>
> if (error)
> --
> 2.31.1
>
