From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Carlos Maiolino <cem@kernel.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/9] xfs: add a on-disk log header cycle array accessor
Date: Tue, 14 Oct 2025 14:27:17 -0700 [thread overview]
Message-ID: <20251014212717.GH6188@frogsfrogsfrogs> (raw)
In-Reply-To: <20251013024228.4109032-3-hch@lst.de>
On Mon, Oct 13, 2025 at 11:42:06AM +0900, Christoph Hellwig wrote:
> Accessing the cycle arrays in the original log record header vs the
> extended header is messy and duplicated in multiple places.
>
> Add a xlog_cycle_data helper to abstract it out.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_log.c | 63 ++++++++++------------------------------
> fs/xfs/xfs_log_priv.h | 18 ++++++++++++
> fs/xfs/xfs_log_recover.c | 17 ++---------
> 3 files changed, 37 insertions(+), 61 deletions(-)
>
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index e09e5f71ed8c..a569a4320a3a 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1524,18 +1524,13 @@ xlog_pack_data(
> struct xlog_in_core *iclog,
> int roundoff)
> {
> - int i, j, k;
> - int size = iclog->ic_offset + roundoff;
> - __be32 cycle_lsn;
> - char *dp;
> -
> - cycle_lsn = CYCLE_LSN_DISK(iclog->ic_header.h_lsn);
> + struct xlog_rec_header *rhead = &iclog->ic_header;
> + __be32 cycle_lsn = CYCLE_LSN_DISK(rhead->h_lsn);
> + char *dp = iclog->ic_datap;
> + int i;
>
> - dp = iclog->ic_datap;
> - for (i = 0; i < BTOBB(size); i++) {
> - if (i >= XLOG_CYCLE_DATA_SIZE)
> - break;
> - iclog->ic_header.h_cycle_data[i] = *(__be32 *)dp;
> + for (i = 0; i < BTOBB(iclog->ic_offset + roundoff); i++) {
> + *xlog_cycle_data(rhead, i) = *(__be32 *)dp;
> *(__be32 *)dp = cycle_lsn;
> dp += BBSIZE;
> }
> @@ -1543,14 +1538,6 @@ xlog_pack_data(
> if (xfs_has_logv2(log->l_mp)) {
> xlog_in_core_2_t *xhdr = iclog->ic_data;
>
> - for ( ; i < BTOBB(size); i++) {
> - j = i / XLOG_CYCLE_DATA_SIZE;
> - k = i % XLOG_CYCLE_DATA_SIZE;
> - xhdr[j].hic_xheader.xh_cycle_data[k] = *(__be32 *)dp;
> - *(__be32 *)dp = cycle_lsn;
> - dp += BBSIZE;
> - }
Just to orient myself -- this is the code that stamps (swazzled) LSN
cycle numbers into the log headers, right?
> -
> for (i = 1; i < log->l_iclog_heads; i++)
> xhdr[i].hic_xheader.xh_cycle = cycle_lsn;
> }
> @@ -3322,13 +3309,12 @@ xlog_verify_iclog(
> struct xlog_in_core *iclog,
> int count)
> {
> - struct xlog_op_header *ophead;
> + struct xlog_rec_header *rhead = &iclog->ic_header;
> xlog_in_core_t *icptr;
> - xlog_in_core_2_t *xhdr;
> - void *base_ptr, *ptr, *p;
> + void *base_ptr, *ptr;
> ptrdiff_t field_offset;
> uint8_t clientid;
> - int len, i, j, k, op_len;
> + int len, i, op_len;
> int idx;
>
> /* check validity of iclog pointers */
> @@ -3342,11 +3328,10 @@ xlog_verify_iclog(
> spin_unlock(&log->l_icloglock);
>
> /* check log magic numbers */
> - if (iclog->ic_header.h_magicno != cpu_to_be32(XLOG_HEADER_MAGIC_NUM))
> + if (rhead->h_magicno != cpu_to_be32(XLOG_HEADER_MAGIC_NUM))
> xfs_emerg(log->l_mp, "%s: invalid magic num", __func__);
>
> - base_ptr = ptr = &iclog->ic_header;
> - p = &iclog->ic_header;
> + base_ptr = ptr = rhead;
> for (ptr += BBSIZE; ptr < base_ptr + count; ptr += BBSIZE) {
> if (*(__be32 *)ptr == cpu_to_be32(XLOG_HEADER_MAGIC_NUM))
> xfs_emerg(log->l_mp, "%s: unexpected magic num",
> @@ -3354,29 +3339,19 @@ xlog_verify_iclog(
> }
>
> /* check fields */
> - len = be32_to_cpu(iclog->ic_header.h_num_logops);
> + len = be32_to_cpu(rhead->h_num_logops);
> base_ptr = ptr = iclog->ic_datap;
> - ophead = ptr;
> - xhdr = iclog->ic_data;
> for (i = 0; i < len; i++) {
> - ophead = ptr;
> + struct xlog_op_header *ophead = ptr;
> + void *p = &ophead->oh_clientid;
>
> /* clientid is only 1 byte */
> - p = &ophead->oh_clientid;
> field_offset = p - base_ptr;
> if (field_offset & 0x1ff) {
> clientid = ophead->oh_clientid;
> } else {
> idx = BTOBBT((void *)&ophead->oh_clientid - iclog->ic_datap);
> - if (idx >= XLOG_CYCLE_DATA_SIZE) {
> - j = idx / XLOG_CYCLE_DATA_SIZE;
> - k = idx % XLOG_CYCLE_DATA_SIZE;
> - clientid = xlog_get_client_id(
> - xhdr[j].hic_xheader.xh_cycle_data[k]);
> - } else {
> - clientid = xlog_get_client_id(
> - iclog->ic_header.h_cycle_data[idx]);
> - }
> + clientid = xlog_get_client_id(*xlog_cycle_data(rhead, idx));
> }
> if (clientid != XFS_TRANSACTION && clientid != XFS_LOG) {
> xfs_warn(log->l_mp,
> @@ -3392,13 +3367,7 @@ xlog_verify_iclog(
> op_len = be32_to_cpu(ophead->oh_len);
> } else {
> idx = BTOBBT((void *)&ophead->oh_len - iclog->ic_datap);
> - if (idx >= XLOG_CYCLE_DATA_SIZE) {
> - j = idx / XLOG_CYCLE_DATA_SIZE;
> - k = idx % XLOG_CYCLE_DATA_SIZE;
> - op_len = be32_to_cpu(xhdr[j].hic_xheader.xh_cycle_data[k]);
> - } else {
> - op_len = be32_to_cpu(iclog->ic_header.h_cycle_data[idx]);
> - }
> + op_len = be32_to_cpu(*xlog_cycle_data(rhead, idx));
> }
> ptr += sizeof(struct xlog_op_header) + op_len;
> }
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 0cfc654d8e87..d2f17691ecca 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -711,4 +711,22 @@ xlog_item_space(
> return round_up(nbytes, sizeof(uint64_t));
> }
>
> +/*
> + * Cycles over XLOG_CYCLE_DATA_SIZE overflow into the extended header that was
> + * added for v2 logs. Addressing for the cycles array there is off by one,
> + * because the first batch of cycles is in the original header.
> + */
> +static inline __be32 *xlog_cycle_data(struct xlog_rec_header *rhead, unsigned i)
> +{
> + if (i >= XLOG_CYCLE_DATA_SIZE) {
Does this need a xfs_has_logv2() check? The old callsites all seem to
have them.
What should happen if i >= XLOG_CYCLE_DATA_SIZE && !logv2 ? Should
this helper return NULL so that callers can return EFSCORRUPTED or
something like that?
--D
> + xlog_in_core_2_t *xhdr = (xlog_in_core_2_t *)rhead;
> + unsigned j = i / XLOG_CYCLE_DATA_SIZE;
> + unsigned k = i % XLOG_CYCLE_DATA_SIZE;
> +
> + return &xhdr[j].hic_xheader.xh_cycle_data[k];
> + }
> +
> + return &rhead->h_cycle_data[i];
> +}
> +
> #endif /* __XFS_LOG_PRIV_H__ */
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index bb2b3f976deb..ef0f6efc4381 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2863,23 +2863,12 @@ xlog_unpack_data(
> char *dp,
> struct xlog *log)
> {
> - int i, j, k;
> + int i;
>
> - for (i = 0; i < BTOBB(be32_to_cpu(rhead->h_len)) &&
> - i < XLOG_CYCLE_DATA_SIZE; i++) {
> - *(__be32 *)dp = *(__be32 *)&rhead->h_cycle_data[i];
> + for (i = 0; i < BTOBB(be32_to_cpu(rhead->h_len)); i++) {
> + *(__be32 *)dp = *xlog_cycle_data(rhead, i);
> dp += BBSIZE;
> }
> -
> - if (xfs_has_logv2(log->l_mp)) {
> - xlog_in_core_2_t *xhdr = (xlog_in_core_2_t *)rhead;
> - for ( ; i < BTOBB(be32_to_cpu(rhead->h_len)); i++) {
> - j = i / XLOG_CYCLE_DATA_SIZE;
> - k = i % XLOG_CYCLE_DATA_SIZE;
> - *(__be32 *)dp = xhdr[j].hic_xheader.xh_cycle_data[k];
> - dp += BBSIZE;
> - }
> - }
> }
>
> /*
> --
> 2.47.3
>
>
next prev parent reply other threads:[~2025-10-14 21:27 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-13 2:42 kill xlog_in_core_2_t v2 Christoph Hellwig
2025-10-13 2:42 ` [PATCH 1/9] xfs: add a XLOG_CYCLE_DATA_SIZE constant Christoph Hellwig
2025-10-14 21:16 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 2/9] xfs: add a on-disk log header cycle array accessor Christoph Hellwig
2025-10-14 21:27 ` Darrick J. Wong [this message]
2025-10-15 4:32 ` Christoph Hellwig
2025-10-15 20:25 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 3/9] xfs: don't use xlog_in_core_2_t in struct xlog_in_core Christoph Hellwig
2025-10-14 21:47 ` Darrick J. Wong
2025-10-15 4:34 ` Christoph Hellwig
2025-10-15 20:26 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 4/9] xfs: cleanup xlog_alloc_log a bit Christoph Hellwig
2025-10-14 21:48 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 5/9] xfs: remove a very outdated comment from xlog_alloc_log Christoph Hellwig
2025-10-14 21:52 ` Darrick J. Wong
2025-10-15 4:37 ` Christoph Hellwig
2025-10-13 2:42 ` [PATCH 6/9] xfs: remove xlog_in_core_2_t Christoph Hellwig
2025-10-14 22:07 ` Darrick J. Wong
2025-10-15 4:41 ` Christoph Hellwig
2025-10-15 20:27 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 7/9] xfs: remove the xlog_rec_header_t typedef Christoph Hellwig
2025-10-14 22:08 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 8/9] xfs: remove l_iclog_heads Christoph Hellwig
2025-10-14 22:12 ` Darrick J. Wong
2025-10-13 2:42 ` [PATCH 9/9] xfs: remove the xlog_in_core_t typedef Christoph Hellwig
2025-10-14 22:12 ` Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2025-10-27 7:05 kill xlog_in_core_2_t v3 Christoph Hellwig
2025-10-27 7:05 ` [PATCH 2/9] xfs: add a on-disk log header cycle array accessor Christoph Hellwig
2025-10-31 13:38 ` Carlos Maiolino
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=20251014212717.GH6188@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=hch@lst.de \
--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).