From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 16/39] xfs: log tickets don't need log client id
Date: Thu, 20 May 2021 17:38:14 -0700 [thread overview]
Message-ID: <20210521003814.GJ9675@magnolia> (raw)
In-Reply-To: <20210519121317.585244-17-david@fromorbit.com>
On Wed, May 19, 2021 at 10:12:54PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> 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@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
Yay for removing never-used macros,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--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
>
next prev parent reply other threads:[~2021-05-21 0:38 UTC|newest]
Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-19 12:12 [PATCH 00/39 v4] xfs: CIL and log optimisations Dave Chinner
2021-05-19 12:12 ` [PATCH 01/39] xfs: log stripe roundoff is a property of the log Dave Chinner
2021-05-28 0:54 ` Allison Henderson
2021-05-19 12:12 ` [PATCH 02/39] xfs: separate CIL commit record IO Dave Chinner
2021-05-28 0:54 ` Allison Henderson
2021-05-19 12:12 ` [PATCH 03/39] xfs: remove xfs_blkdev_issue_flush Dave Chinner
2021-05-28 0:54 ` Allison Henderson
2021-05-19 12:12 ` [PATCH 04/39] xfs: async blkdev cache flush Dave Chinner
2021-05-20 23:53 ` Darrick J. Wong
2021-05-28 0:54 ` Allison Henderson
2021-05-19 12:12 ` [PATCH 05/39] xfs: CIL checkpoint flushes caches unconditionally Dave Chinner
2021-05-28 0:54 ` Allison Henderson
2021-05-19 12:12 ` [PATCH 06/39] xfs: remove need_start_rec parameter from xlog_write() Dave Chinner
2021-05-19 12:12 ` [PATCH 07/39] xfs: journal IO cache flush reductions Dave Chinner
2021-05-21 0:16 ` Darrick J. Wong
2021-05-19 12:12 ` [PATCH 08/39] xfs: Fix CIL throttle hang when CIL space used going backwards Dave Chinner
2021-05-19 12:12 ` [PATCH 09/39] xfs: xfs_log_force_lsn isn't passed a LSN Dave Chinner
2021-05-21 0:20 ` Darrick J. Wong
2021-05-19 12:12 ` [PATCH 10/39] xfs: AIL needs asynchronous CIL forcing Dave Chinner
2021-05-21 0:33 ` Darrick J. Wong
2021-05-19 12:12 ` [PATCH 11/39] xfs: CIL work is serialised, not pipelined Dave Chinner
2021-05-21 0:32 ` Darrick J. Wong
2021-05-19 12:12 ` [PATCH 12/39] xfs: factor out the CIL transaction header building Dave Chinner
2021-05-19 12:12 ` [PATCH 13/39] xfs: only CIL pushes require a start record Dave Chinner
2021-05-19 12:12 ` [PATCH 14/39] xfs: embed the xlog_op_header in the unmount record Dave Chinner
2021-05-21 0:35 ` Darrick J. Wong
2021-05-19 12:12 ` [PATCH 15/39] xfs: embed the xlog_op_header in the commit record Dave Chinner
2021-05-19 12:12 ` [PATCH 16/39] xfs: log tickets don't need log client id Dave Chinner
2021-05-21 0:38 ` Darrick J. Wong [this message]
2021-05-19 12:12 ` [PATCH 17/39] xfs: move log iovec alignment to preparation function Dave Chinner
2021-05-19 12:12 ` [PATCH 18/39] xfs: reserve space and initialise xlog_op_header in item formatting Dave Chinner
2021-05-19 12:12 ` [PATCH 19/39] xfs: log ticket region debug is largely useless Dave Chinner
2021-05-19 12:12 ` [PATCH 20/39] xfs: pass lv chain length into xlog_write() Dave Chinner
2021-05-27 17:20 ` Darrick J. Wong
2021-06-02 22:18 ` Dave Chinner
2021-06-02 22:24 ` Darrick J. Wong
2021-06-02 22:58 ` [PATCH 20/39 V2] " Dave Chinner
2021-06-02 23:01 ` Darrick J. Wong
2021-05-19 12:12 ` [PATCH 21/39] xfs: introduce xlog_write_single() Dave Chinner
2021-05-27 17:27 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 22/39] xfs:_introduce xlog_write_partial() Dave Chinner
2021-05-27 18:06 ` Darrick J. Wong
2021-06-02 22:21 ` Dave Chinner
2021-05-19 12:13 ` [PATCH 23/39] xfs: xlog_write() no longer needs contwr state Dave Chinner
2021-05-19 12:13 ` [PATCH 24/39] xfs: xlog_write() doesn't need optype anymore Dave Chinner
2021-05-27 18:07 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 25/39] xfs: CIL context doesn't need to count iovecs Dave Chinner
2021-05-27 18:08 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 26/39] xfs: use the CIL space used counter for emptiness checks Dave Chinner
2021-05-19 12:13 ` [PATCH 27/39] xfs: lift init CIL reservation out of xc_cil_lock Dave Chinner
2021-05-19 12:13 ` [PATCH 28/39] xfs: rework per-iclog header CIL reservation Dave Chinner
2021-05-27 18:17 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 29/39] xfs: introduce per-cpu CIL tracking structure Dave Chinner
2021-05-27 18:31 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 30/39] xfs: implement percpu cil space used calculation Dave Chinner
2021-05-27 18:41 ` Darrick J. Wong
2021-06-02 23:47 ` Dave Chinner
2021-06-03 1:26 ` Darrick J. Wong
2021-06-03 2:28 ` Dave Chinner
2021-06-03 3:01 ` Darrick J. Wong
2021-06-03 3:56 ` Dave Chinner
2021-05-19 12:13 ` [PATCH 31/39] xfs: track CIL ticket reservation in percpu structure Dave Chinner
2021-05-27 18:48 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 32/39] xfs: convert CIL busy extents to per-cpu Dave Chinner
2021-05-27 18:49 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 33/39] xfs: Add order IDs to log items in CIL Dave Chinner
2021-05-27 19:00 ` Darrick J. Wong
2021-06-03 0:16 ` Dave Chinner
2021-06-03 0:49 ` Darrick J. Wong
2021-06-03 2:13 ` Dave Chinner
2021-06-03 3:02 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 34/39] xfs: convert CIL to unordered per cpu lists Dave Chinner
2021-05-27 19:03 ` Darrick J. Wong
2021-06-03 0:27 ` Dave Chinner
2021-05-19 12:13 ` [PATCH 35/39] xfs: convert log vector chain to use list heads Dave Chinner
2021-05-27 19:13 ` Darrick J. Wong
2021-06-03 0:38 ` Dave Chinner
2021-06-03 0:50 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 36/39] xfs: move CIL ordering to the logvec chain Dave Chinner
2021-05-27 19:14 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 37/39] xfs: avoid cil push lock if possible Dave Chinner
2021-05-27 19:18 ` Darrick J. Wong
2021-05-19 12:13 ` [PATCH 38/39] xfs: xlog_sync() manually adjusts grant head space Dave Chinner
2021-05-19 12:13 ` [PATCH 39/39] xfs: expanding delayed logging design with background material Dave Chinner
2021-05-27 20:38 ` Darrick J. Wong
2021-06-03 0:57 ` Dave Chinner
-- strict thread matches above, loose matches on Subject: below --
2021-06-03 5:22 [PATCH 00/39 v5] xfs: CIL and log optimisations Dave Chinner
2021-06-03 5:22 ` [PATCH 16/39] xfs: log tickets don't need log client id Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210521003814.GJ9675@magnolia \
--to=djwong@kernel.org \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).