kill xlog_in_core_2_t v2

kill xlog_in_core_2_t v2

[Christoph Hellwig]

Hi all,

xlog_in_core_2_t is probably one of the most confusing types in the
kernel. Not only does it describe an on-disk format despite claiming
to be in-core in the name, but it is also has an extremely confusing
layout.  This series revamps our C representation of the underlying
data structures so that they hopefully make a lot more sense while
killing off xlog_in_core_2_t for good.

Changes since v1:
 - use struct_size to calculate the total header size
 - add a patch in to kill the last remaining struct typedef in the log
   code

Diffstat:
 libxfs/xfs_log_format.h |   38 ++++----
 libxfs/xfs_ondisk.h     |    6 -
 xfs_log.c               |  206 ++++++++++++++++++------------------------------
 xfs_log_cil.c           |    6 -
 xfs_log_priv.h          |   33 +++++--
 xfs_log_recover.c       |   45 +++-------
 xfs_trace.h             |    2 
 7 files changed, 146 insertions(+), 190 deletions(-)

[PATCH 1/9] xfs: add a XLOG_CYCLE_DATA_SIZE constant

[Christoph Hellwig]

The XLOG_HEADER_CYCLE_SIZE / BBSIZE expression is used a lot
in the log code, give it a symbolic name.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_log_format.h |  5 +++--
 fs/xfs/xfs_log.c               | 18 +++++++++---------
 fs/xfs/xfs_log_recover.c       |  6 +++---
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 6c50cb2ece19..91a841ea5bb3 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -31,6 +31,7 @@ typedef uint32_t xlog_tid_t;
 #define XLOG_BIG_RECORD_BSIZE	(32*1024)	/* 32k buffers */
 #define XLOG_MAX_RECORD_BSIZE	(256*1024)
 #define XLOG_HEADER_CYCLE_SIZE	(32*1024)	/* cycle data in header */
+#define XLOG_CYCLE_DATA_SIZE	(XLOG_HEADER_CYCLE_SIZE / BBSIZE)
 #define XLOG_MIN_RECORD_BSHIFT	14		/* 16384 == 1 << 14 */
 #define XLOG_BIG_RECORD_BSHIFT	15		/* 32k == 1 << 15 */
 #define XLOG_MAX_RECORD_BSHIFT	18		/* 256k == 1 << 18 */
@@ -135,7 +136,7 @@ typedef struct xlog_rec_header {
 	__le32	  h_crc;	/* crc of log record                    :  4 */
 	__be32	  h_prev_block; /* block number to previous LR		:  4 */
 	__be32	  h_num_logops;	/* number of log operations in this LR	:  4 */
-	__be32	  h_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE];
+	__be32	  h_cycle_data[XLOG_CYCLE_DATA_SIZE];
 
 	/* fields added by the Linux port: */
 	__be32    h_fmt;        /* format of log record                 :  4 */
@@ -172,7 +173,7 @@ typedef struct xlog_rec_header {
 
 typedef struct xlog_rec_ext_header {
 	__be32	  xh_cycle;	/* write cycle of log			: 4 */
-	__be32	  xh_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE]; /*	: 256 */
+	__be32	  xh_cycle_data[XLOG_CYCLE_DATA_SIZE];		/*	: 256 */
 } xlog_rec_ext_header_t;
 
 /*
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 603e85c1ab4c..e09e5f71ed8c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1533,7 +1533,7 @@ xlog_pack_data(
 
 	dp = iclog->ic_datap;
 	for (i = 0; i < BTOBB(size); i++) {
-		if (i >= (XLOG_HEADER_CYCLE_SIZE / BBSIZE))
+		if (i >= XLOG_CYCLE_DATA_SIZE)
 			break;
 		iclog->ic_header.h_cycle_data[i] = *(__be32 *)dp;
 		*(__be32 *)dp = cycle_lsn;
@@ -1544,8 +1544,8 @@ xlog_pack_data(
 		xlog_in_core_2_t *xhdr = iclog->ic_data;
 
 		for ( ; i < BTOBB(size); i++) {
-			j = i / (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
-			k = i % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
+			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;
@@ -3368,9 +3368,9 @@ xlog_verify_iclog(
 			clientid = ophead->oh_clientid;
 		} else {
 			idx = BTOBBT((void *)&ophead->oh_clientid - iclog->ic_datap);
-			if (idx >= (XLOG_HEADER_CYCLE_SIZE / BBSIZE)) {
-				j = idx / (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
-				k = idx % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
+			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 {
@@ -3392,9 +3392,9 @@ xlog_verify_iclog(
 			op_len = be32_to_cpu(ophead->oh_len);
 		} else {
 			idx = BTOBBT((void *)&ophead->oh_len - iclog->ic_datap);
-			if (idx >= (XLOG_HEADER_CYCLE_SIZE / BBSIZE)) {
-				j = idx / (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
-				k = idx % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
+			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]);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 549d60959aee..bb2b3f976deb 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2866,7 +2866,7 @@ xlog_unpack_data(
 	int			i, j, k;
 
 	for (i = 0; i < BTOBB(be32_to_cpu(rhead->h_len)) &&
-		  i < (XLOG_HEADER_CYCLE_SIZE / BBSIZE); i++) {
+		  i < XLOG_CYCLE_DATA_SIZE; i++) {
 		*(__be32 *)dp = *(__be32 *)&rhead->h_cycle_data[i];
 		dp += BBSIZE;
 	}
@@ -2874,8 +2874,8 @@ xlog_unpack_data(
 	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_HEADER_CYCLE_SIZE / BBSIZE);
-			k = i % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
+			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

[PATCH 2/9] xfs: add a on-disk log header cycle array accessor

[Christoph Hellwig]

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;
-		}
-
 		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) {
+		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

[PATCH 3/9] xfs: don't use xlog_in_core_2_t in struct xlog_in_core

[Christoph Hellwig]

Most accessed to the on-disk log record header are for the original
xlog_rec_header.  Make that the main structure, and case for the
single remaining place using other union legs.

This prepares for removing xlog_in_core_2_t entirely.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c      | 74 +++++++++++++++++++++----------------------
 fs/xfs/xfs_log_cil.c  |  6 ++--
 fs/xfs/xfs_log_priv.h |  9 ++----
 fs/xfs/xfs_trace.h    |  2 +-
 4 files changed, 44 insertions(+), 47 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index a569a4320a3a..d9476124def6 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -534,8 +534,8 @@ xlog_state_release_iclog(
 	 */
 	if ((iclog->ic_state == XLOG_STATE_WANT_SYNC ||
 	     (iclog->ic_flags & XLOG_ICL_NEED_FUA)) &&
-	    !iclog->ic_header.h_tail_lsn) {
-		iclog->ic_header.h_tail_lsn =
+	    !iclog->ic_header->h_tail_lsn) {
+		iclog->ic_header->h_tail_lsn =
 				cpu_to_be64(atomic64_read(&log->l_tail_lsn));
 	}
 
@@ -1457,11 +1457,11 @@ xlog_alloc_log(
 		iclog->ic_prev = prev_iclog;
 		prev_iclog = iclog;
 
-		iclog->ic_data = kvzalloc(log->l_iclog_size,
+		iclog->ic_header = kvzalloc(log->l_iclog_size,
 				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
-		if (!iclog->ic_data)
+		if (!iclog->ic_header)
 			goto out_free_iclog;
-		head = &iclog->ic_header;
+		head = iclog->ic_header;
 		memset(head, 0, sizeof(xlog_rec_header_t));
 		head->h_magicno = cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
 		head->h_version = cpu_to_be32(
@@ -1476,7 +1476,7 @@ xlog_alloc_log(
 		iclog->ic_log = log;
 		atomic_set(&iclog->ic_refcnt, 0);
 		INIT_LIST_HEAD(&iclog->ic_callbacks);
-		iclog->ic_datap = (void *)iclog->ic_data + log->l_iclog_hsize;
+		iclog->ic_datap = (void *)iclog->ic_header + log->l_iclog_hsize;
 
 		init_waitqueue_head(&iclog->ic_force_wait);
 		init_waitqueue_head(&iclog->ic_write_wait);
@@ -1504,7 +1504,7 @@ xlog_alloc_log(
 out_free_iclog:
 	for (iclog = log->l_iclog; iclog; iclog = prev_iclog) {
 		prev_iclog = iclog->ic_next;
-		kvfree(iclog->ic_data);
+		kvfree(iclog->ic_header);
 		kfree(iclog);
 		if (prev_iclog == log->l_iclog)
 			break;
@@ -1524,7 +1524,7 @@ xlog_pack_data(
 	struct xlog_in_core	*iclog,
 	int			roundoff)
 {
-	struct xlog_rec_header	*rhead = &iclog->ic_header;
+	struct xlog_rec_header	*rhead = iclog->ic_header;
 	__be32			cycle_lsn = CYCLE_LSN_DISK(rhead->h_lsn);
 	char			*dp = iclog->ic_datap;
 	int			i;
@@ -1536,7 +1536,7 @@ xlog_pack_data(
 	}
 
 	if (xfs_has_logv2(log->l_mp)) {
-		xlog_in_core_2_t *xhdr = iclog->ic_data;
+		xlog_in_core_2_t *xhdr = (xlog_in_core_2_t *)iclog->ic_header;
 
 		for (i = 1; i < log->l_iclog_heads; i++)
 			xhdr[i].hic_xheader.xh_cycle = cycle_lsn;
@@ -1658,11 +1658,11 @@ xlog_write_iclog(
 
 	iclog->ic_flags &= ~(XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA);
 
-	if (is_vmalloc_addr(iclog->ic_data)) {
-		if (!bio_add_vmalloc(&iclog->ic_bio, iclog->ic_data, count))
+	if (is_vmalloc_addr(iclog->ic_header)) {
+		if (!bio_add_vmalloc(&iclog->ic_bio, iclog->ic_header, count))
 			goto shutdown;
 	} else {
-		bio_add_virt_nofail(&iclog->ic_bio, iclog->ic_data, count);
+		bio_add_virt_nofail(&iclog->ic_bio, iclog->ic_header, count);
 	}
 
 	/*
@@ -1791,19 +1791,19 @@ xlog_sync(
 	size = iclog->ic_offset;
 	if (xfs_has_logv2(log->l_mp))
 		size += roundoff;
-	iclog->ic_header.h_len = cpu_to_be32(size);
+	iclog->ic_header->h_len = cpu_to_be32(size);
 
 	XFS_STATS_INC(log->l_mp, xs_log_writes);
 	XFS_STATS_ADD(log->l_mp, xs_log_blocks, BTOBB(count));
 
-	bno = BLOCK_LSN(be64_to_cpu(iclog->ic_header.h_lsn));
+	bno = BLOCK_LSN(be64_to_cpu(iclog->ic_header->h_lsn));
 
 	/* Do we need to split this write into 2 parts? */
 	if (bno + BTOBB(count) > log->l_logBBsize)
-		xlog_split_iclog(log, &iclog->ic_header, bno, count);
+		xlog_split_iclog(log, iclog->ic_header, bno, count);
 
 	/* calculcate the checksum */
-	iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header,
+	iclog->ic_header->h_crc = xlog_cksum(log, iclog->ic_header,
 			iclog->ic_datap, XLOG_REC_SIZE, size);
 	/*
 	 * Intentionally corrupt the log record CRC based on the error injection
@@ -1814,11 +1814,11 @@ xlog_sync(
 	 */
 #ifdef DEBUG
 	if (XFS_TEST_ERROR(log->l_mp, XFS_ERRTAG_LOG_BAD_CRC)) {
-		iclog->ic_header.h_crc &= cpu_to_le32(0xAAAAAAAA);
+		iclog->ic_header->h_crc &= cpu_to_le32(0xAAAAAAAA);
 		iclog->ic_fail_crc = true;
 		xfs_warn(log->l_mp,
 	"Intentionally corrupted log record at LSN 0x%llx. Shutdown imminent.",
-			 be64_to_cpu(iclog->ic_header.h_lsn));
+			 be64_to_cpu(iclog->ic_header->h_lsn));
 	}
 #endif
 	xlog_verify_iclog(log, iclog, count);
@@ -1845,7 +1845,7 @@ xlog_dealloc_log(
 	iclog = log->l_iclog;
 	for (i = 0; i < log->l_iclog_bufs; i++) {
 		next_iclog = iclog->ic_next;
-		kvfree(iclog->ic_data);
+		kvfree(iclog->ic_header);
 		kfree(iclog);
 		iclog = next_iclog;
 	}
@@ -1867,7 +1867,7 @@ xlog_state_finish_copy(
 {
 	lockdep_assert_held(&log->l_icloglock);
 
-	be32_add_cpu(&iclog->ic_header.h_num_logops, record_cnt);
+	be32_add_cpu(&iclog->ic_header->h_num_logops, record_cnt);
 	iclog->ic_offset += copy_bytes;
 }
 
@@ -2290,7 +2290,7 @@ xlog_state_activate_iclog(
 	 * We don't need to cover the dummy.
 	 */
 	if (*iclogs_changed == 0 &&
-	    iclog->ic_header.h_num_logops == cpu_to_be32(XLOG_COVER_OPS)) {
+	    iclog->ic_header->h_num_logops == cpu_to_be32(XLOG_COVER_OPS)) {
 		*iclogs_changed = 1;
 	} else {
 		/*
@@ -2302,11 +2302,11 @@ xlog_state_activate_iclog(
 
 	iclog->ic_state	= XLOG_STATE_ACTIVE;
 	iclog->ic_offset = 0;
-	iclog->ic_header.h_num_logops = 0;
-	memset(iclog->ic_header.h_cycle_data, 0,
-		sizeof(iclog->ic_header.h_cycle_data));
-	iclog->ic_header.h_lsn = 0;
-	iclog->ic_header.h_tail_lsn = 0;
+	iclog->ic_header->h_num_logops = 0;
+	memset(iclog->ic_header->h_cycle_data, 0,
+		sizeof(iclog->ic_header->h_cycle_data));
+	iclog->ic_header->h_lsn = 0;
+	iclog->ic_header->h_tail_lsn = 0;
 }
 
 /*
@@ -2398,7 +2398,7 @@ xlog_get_lowest_lsn(
 		    iclog->ic_state == XLOG_STATE_DIRTY)
 			continue;
 
-		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
+		lsn = be64_to_cpu(iclog->ic_header->h_lsn);
 		if ((lsn && !lowest_lsn) || XFS_LSN_CMP(lsn, lowest_lsn) < 0)
 			lowest_lsn = lsn;
 	} while ((iclog = iclog->ic_next) != log->l_iclog);
@@ -2433,7 +2433,7 @@ xlog_state_iodone_process_iclog(
 		 * If this is not the lowest lsn iclog, then we will leave it
 		 * for another completion to process.
 		 */
-		header_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
+		header_lsn = be64_to_cpu(iclog->ic_header->h_lsn);
 		lowest_lsn = xlog_get_lowest_lsn(log);
 		if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0)
 			return false;
@@ -2616,7 +2616,7 @@ xlog_state_get_iclog_space(
 		goto restart;
 	}
 
-	head = &iclog->ic_header;
+	head = iclog->ic_header;
 
 	atomic_inc(&iclog->ic_refcnt);	/* prevents sync */
 	log_offset = iclog->ic_offset;
@@ -2781,7 +2781,7 @@ xlog_state_switch_iclogs(
 	if (!eventual_size)
 		eventual_size = iclog->ic_offset;
 	iclog->ic_state = XLOG_STATE_WANT_SYNC;
-	iclog->ic_header.h_prev_block = cpu_to_be32(log->l_prev_block);
+	iclog->ic_header->h_prev_block = cpu_to_be32(log->l_prev_block);
 	log->l_prev_block = log->l_curr_block;
 	log->l_prev_cycle = log->l_curr_cycle;
 
@@ -2825,7 +2825,7 @@ xlog_force_and_check_iclog(
 	struct xlog_in_core	*iclog,
 	bool			*completed)
 {
-	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
+	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header->h_lsn);
 	int			error;
 
 	*completed = false;
@@ -2837,7 +2837,7 @@ xlog_force_and_check_iclog(
 	 * If the iclog has already been completed and reused the header LSN
 	 * will have been rewritten by completion
 	 */
-	if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn)
+	if (be64_to_cpu(iclog->ic_header->h_lsn) != lsn)
 		*completed = true;
 	return 0;
 }
@@ -2970,7 +2970,7 @@ xlog_force_lsn(
 		goto out_error;
 
 	iclog = log->l_iclog;
-	while (be64_to_cpu(iclog->ic_header.h_lsn) != lsn) {
+	while (be64_to_cpu(iclog->ic_header->h_lsn) != lsn) {
 		trace_xlog_iclog_force_lsn(iclog, _RET_IP_);
 		iclog = iclog->ic_next;
 		if (iclog == log->l_iclog)
@@ -3236,7 +3236,7 @@ xlog_verify_dump_tail(
 {
 	xfs_alert(log->l_mp,
 "ran out of log space tail 0x%llx/0x%llx, head lsn 0x%llx, head 0x%x/0x%x, prev head 0x%x/0x%x",
-			iclog ? be64_to_cpu(iclog->ic_header.h_tail_lsn) : -1,
+			iclog ? be64_to_cpu(iclog->ic_header->h_tail_lsn) : -1,
 			atomic64_read(&log->l_tail_lsn),
 			log->l_ailp->ail_head_lsn,
 			log->l_curr_cycle, log->l_curr_block,
@@ -3255,7 +3255,7 @@ xlog_verify_tail_lsn(
 	struct xlog		*log,
 	struct xlog_in_core	*iclog)
 {
-	xfs_lsn_t	tail_lsn = be64_to_cpu(iclog->ic_header.h_tail_lsn);
+	xfs_lsn_t	tail_lsn = be64_to_cpu(iclog->ic_header->h_tail_lsn);
 	int		blocks;
 
 	if (CYCLE_LSN(tail_lsn) == log->l_prev_cycle) {
@@ -3309,7 +3309,7 @@ xlog_verify_iclog(
 	struct xlog_in_core	*iclog,
 	int			count)
 {
-	struct xlog_rec_header	*rhead = &iclog->ic_header;
+	struct xlog_rec_header	*rhead = iclog->ic_header;
 	xlog_in_core_t		*icptr;
 	void			*base_ptr, *ptr;
 	ptrdiff_t		field_offset;
@@ -3507,7 +3507,7 @@ xlog_iclogs_empty(
 		/* endianness does not matter here, zero is zero in
 		 * any language.
 		 */
-		if (iclog->ic_header.h_num_logops)
+		if (iclog->ic_header->h_num_logops)
 			return 0;
 		iclog = iclog->ic_next;
 	} while (iclog != log->l_iclog);
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index f443757e93c2..778ac47adb8c 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -940,7 +940,7 @@ xlog_cil_set_ctx_write_state(
 	struct xlog_in_core	*iclog)
 {
 	struct xfs_cil		*cil = ctx->cil;
-	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
+	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header->h_lsn);
 
 	ASSERT(!ctx->commit_lsn);
 	if (!ctx->start_lsn) {
@@ -1458,9 +1458,9 @@ xlog_cil_push_work(
 	 */
 	spin_lock(&log->l_icloglock);
 	if (ctx->start_lsn != ctx->commit_lsn) {
-		xfs_lsn_t	plsn;
+		xfs_lsn_t	plsn = be64_to_cpu(
+			ctx->commit_iclog->ic_prev->ic_header->h_lsn);
 
-		plsn = be64_to_cpu(ctx->commit_iclog->ic_prev->ic_header.h_lsn);
 		if (plsn && XFS_LSN_CMP(plsn, ctx->commit_lsn) < 0) {
 			/*
 			 * Waiting on ic_force_wait orders the completion of
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index d2f17691ecca..f1aed6e8f747 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -158,10 +158,8 @@ struct xlog_ticket {
 };
 
 /*
- * - A log record header is 512 bytes.  There is plenty of room to grow the
- *	xlog_rec_header_t into the reserved space.
- * - ic_data follows, so a write to disk can start at the beginning of
- *	the iclog.
+ * In-core log structure.
+ *
  * - ic_forcewait is used to implement synchronous forcing of the iclog to disk.
  * - ic_next is the pointer to the next iclog in the ring.
  * - ic_log is a pointer back to the global log structure.
@@ -198,8 +196,7 @@ typedef struct xlog_in_core {
 
 	/* reference counts need their own cacheline */
 	atomic_t		ic_refcnt ____cacheline_aligned_in_smp;
-	xlog_in_core_2_t	*ic_data;
-#define ic_header	ic_data->hic_header
+	struct xlog_rec_header	*ic_header;
 #ifdef DEBUG
 	bool			ic_fail_crc : 1;
 #endif
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 79b8641880ab..aa3a3870f894 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -4934,7 +4934,7 @@ DECLARE_EVENT_CLASS(xlog_iclog_class,
 		__entry->refcount = atomic_read(&iclog->ic_refcnt);
 		__entry->offset = iclog->ic_offset;
 		__entry->flags = iclog->ic_flags;
-		__entry->lsn = be64_to_cpu(iclog->ic_header.h_lsn);
+		__entry->lsn = be64_to_cpu(iclog->ic_header->h_lsn);
 		__entry->caller_ip = caller_ip;
 	),
 	TP_printk("dev %d:%d state %s refcnt %d offset %u lsn 0x%llx flags %s caller %pS",
-- 
2.47.3

[PATCH 4/9] xfs: cleanup xlog_alloc_log a bit

[Christoph Hellwig]

Remove the separate head variable, move the ic_datap initialization
up a bit where the context is more obvious and remove the duplicate
memset right after a zeroing memory allocation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index d9476124def6..3bd2f8787682 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1367,7 +1367,6 @@ xlog_alloc_log(
 	int			num_bblks)
 {
 	struct xlog		*log;
-	xlog_rec_header_t	*head;
 	xlog_in_core_t		**iclogp;
 	xlog_in_core_t		*iclog, *prev_iclog=NULL;
 	int			i;
@@ -1461,22 +1460,21 @@ xlog_alloc_log(
 				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
 		if (!iclog->ic_header)
 			goto out_free_iclog;
-		head = iclog->ic_header;
-		memset(head, 0, sizeof(xlog_rec_header_t));
-		head->h_magicno = cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
-		head->h_version = cpu_to_be32(
+		iclog->ic_header->h_magicno =
+			cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
+		iclog->ic_header->h_version = cpu_to_be32(
 			xfs_has_logv2(log->l_mp) ? 2 : 1);
-		head->h_size = cpu_to_be32(log->l_iclog_size);
-		/* new fields */
-		head->h_fmt = cpu_to_be32(XLOG_FMT);
-		memcpy(&head->h_fs_uuid, &mp->m_sb.sb_uuid, sizeof(uuid_t));
+		iclog->ic_header->h_size = cpu_to_be32(log->l_iclog_size);
+		iclog->ic_header->h_fmt = cpu_to_be32(XLOG_FMT);
+		memcpy(&iclog->ic_header->h_fs_uuid, &mp->m_sb.sb_uuid,
+			sizeof(iclog->ic_header->h_fs_uuid));
 
+		iclog->ic_datap = (void *)iclog->ic_header + log->l_iclog_hsize;
 		iclog->ic_size = log->l_iclog_size - log->l_iclog_hsize;
 		iclog->ic_state = XLOG_STATE_ACTIVE;
 		iclog->ic_log = log;
 		atomic_set(&iclog->ic_refcnt, 0);
 		INIT_LIST_HEAD(&iclog->ic_callbacks);
-		iclog->ic_datap = (void *)iclog->ic_header + log->l_iclog_hsize;
 
 		init_waitqueue_head(&iclog->ic_force_wait);
 		init_waitqueue_head(&iclog->ic_write_wait);
-- 
2.47.3

[PATCH 5/9] xfs: remove a very outdated comment from xlog_alloc_log

[Christoph Hellwig]

The xlog_iclog definition has been pretty standard for a while, so drop
this now rather misleading comment.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 3bd2f8787682..acddab467b77 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1435,13 +1435,6 @@ xlog_alloc_log(
 	init_waitqueue_head(&log->l_flush_wait);
 
 	iclogp = &log->l_iclog;
-	/*
-	 * The amount of memory to allocate for the iclog structure is
-	 * rather funky due to the way the structure is defined.  It is
-	 * done this way so that we can use different sizes for machines
-	 * with different amounts of memory.  See the definition of
-	 * xlog_in_core_t in xfs_log_priv.h for details.
-	 */
 	ASSERT(log->l_iclog_size >= 4096);
 	for (i = 0; i < log->l_iclog_bufs; i++) {
 		size_t bvec_size = howmany(log->l_iclog_size, PAGE_SIZE) *
-- 
2.47.3

[PATCH 6/9] xfs: remove xlog_in_core_2_t

[Christoph Hellwig]

xlog_in_core_2_t is a really odd type, not only is it grossly
misnamed because it actually is an on-disk structure, but it also
reprents the actual on-disk structure in a rather odd way.

A v1 or small v2 log header look like:

	+-----------------------+
	|      xlog_record      |
	+-----------------------+

while larger v2 log headers look like:

	+-----------------------+
	|      xlog_record      |
	+-----------------------+
	|  xlog_rec_ext_header  |
	+-------------------+---+
	|         .....         |
	+-----------------------+
	|  xlog_rec_ext_header  |
	+-----------------------+

I.e., the ext headers are a variable sized array at the end of the
header.  So instead of declaring a union of xlog_rec_header,
xlog_rec_ext_header and padding to BBSIZE, add the proper padding to
struct struct xlog_rec_header and struct xlog_rec_ext_header, and
add a variable sized array of the latter to the former.  This also
exposes the somewhat unusual scope of the log checksums, which is
made explicitly now by adding proper padding and macro designating
the actual payload length.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_log_format.h | 31 +++++++++++++++----------------
 fs/xfs/libxfs/xfs_ondisk.h     |  6 ++++--
 fs/xfs/xfs_log.c               | 21 ++++++---------------
 fs/xfs/xfs_log_priv.h          |  3 +--
 4 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 91a841ea5bb3..4cb69bd285ca 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -126,6 +126,16 @@ struct xlog_op_header {
 #define XLOG_FMT XLOG_FMT_LINUX_LE
 #endif
 
+struct xlog_rec_ext_header {
+	__be32		xh_cycle;	/* write cycle of log */
+	__be32		xh_cycle_data[XLOG_CYCLE_DATA_SIZE];
+	__u8		xh_reserved[252];
+};
+
+/* actual ext header payload size for checksumming */
+#define XLOG_REC_EXT_SIZE \
+	offsetofend(struct xlog_rec_ext_header, xh_cycle_data)
+
 typedef struct xlog_rec_header {
 	__be32	  h_magicno;	/* log record (LR) identifier		:  4 */
 	__be32	  h_cycle;	/* write cycle of log			:  4 */
@@ -161,30 +171,19 @@ typedef struct xlog_rec_header {
 	 * (little-endian) architectures.
 	 */
 	__u32	  h_pad0;
+
+	__u8	  h_reserved[184];
+	struct xlog_rec_ext_header h_ext[];
 } xlog_rec_header_t;
 
 #ifdef __i386__
 #define XLOG_REC_SIZE		offsetofend(struct xlog_rec_header, h_size)
-#define XLOG_REC_SIZE_OTHER	sizeof(struct xlog_rec_header)
+#define XLOG_REC_SIZE_OTHER	offsetofend(struct xlog_rec_header, h_pad0)
 #else
-#define XLOG_REC_SIZE		sizeof(struct xlog_rec_header)
+#define XLOG_REC_SIZE		offsetofend(struct xlog_rec_header, h_pad0)
 #define XLOG_REC_SIZE_OTHER	offsetofend(struct xlog_rec_header, h_size)
 #endif /* __i386__ */
 
-typedef struct xlog_rec_ext_header {
-	__be32	  xh_cycle;	/* write cycle of log			: 4 */
-	__be32	  xh_cycle_data[XLOG_CYCLE_DATA_SIZE];		/*	: 256 */
-} xlog_rec_ext_header_t;
-
-/*
- * Quite misnamed, because this union lays out the actual on-disk log buffer.
- */
-typedef union xlog_in_core2 {
-	xlog_rec_header_t	hic_header;
-	xlog_rec_ext_header_t	hic_xheader;
-	char			hic_sector[XLOG_HEADER_SIZE];
-} xlog_in_core_2_t;
-
 /* not an on-disk structure, but needed by log recovery in userspace */
 struct xfs_log_iovec {
 	void		*i_addr;	/* beginning address of region */
diff --git a/fs/xfs/libxfs/xfs_ondisk.h b/fs/xfs/libxfs/xfs_ondisk.h
index 7bfa3242e2c5..2e9715cc1641 100644
--- a/fs/xfs/libxfs/xfs_ondisk.h
+++ b/fs/xfs/libxfs/xfs_ondisk.h
@@ -174,9 +174,11 @@ xfs_check_ondisk_structs(void)
 	XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format,	16);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent,		32);
 	XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent,		16);
-	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_header,		328);
-	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_ext_header,	260);
+	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_header,		512);
+	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_ext_header,	512);
 
+	XFS_CHECK_OFFSET(struct xlog_rec_header, h_reserved,		328);
+	XFS_CHECK_OFFSET(struct xlog_rec_ext_header, xh_reserved,	260);
 	XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents,	16);
 	XFS_CHECK_OFFSET(struct xfs_cui_log_format, cui_extents,	16);
 	XFS_CHECK_OFFSET(struct xfs_rui_log_format, rui_extents,	16);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index acddab467b77..1fe3abbd3d36 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1526,12 +1526,8 @@ xlog_pack_data(
 		dp += BBSIZE;
 	}
 
-	if (xfs_has_logv2(log->l_mp)) {
-		xlog_in_core_2_t *xhdr = (xlog_in_core_2_t *)iclog->ic_header;
-
-		for (i = 1; i < log->l_iclog_heads; i++)
-			xhdr[i].hic_xheader.xh_cycle = cycle_lsn;
-	}
+	for (i = 0; i < log->l_iclog_heads - 1; i++)
+		rhead->h_ext[i].xh_cycle = cycle_lsn;
 }
 
 /*
@@ -1556,16 +1552,11 @@ xlog_cksum(
 
 	/* ... then for additional cycle data for v2 logs ... */
 	if (xfs_has_logv2(log->l_mp)) {
-		union xlog_in_core2 *xhdr = (union xlog_in_core2 *)rhead;
-		int		i;
-		int		xheads;
+		int		xheads, i;
 
-		xheads = DIV_ROUND_UP(size, XLOG_HEADER_CYCLE_SIZE);
-
-		for (i = 1; i < xheads; i++) {
-			crc = crc32c(crc, &xhdr[i].hic_xheader,
-				     sizeof(struct xlog_rec_ext_header));
-		}
+		xheads = DIV_ROUND_UP(size, XLOG_HEADER_CYCLE_SIZE) - 1;
+		for (i = 0; i < xheads; i++)
+			crc = crc32c(crc, &rhead->h_ext[i], XLOG_REC_EXT_SIZE);
 	}
 
 	/* ... and finally for the payload */
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index f1aed6e8f747..ac98ac71152d 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -716,11 +716,10 @@ xlog_item_space(
 static inline __be32 *xlog_cycle_data(struct xlog_rec_header *rhead, unsigned i)
 {
 	if (i >= XLOG_CYCLE_DATA_SIZE) {
-		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_ext[j - 1].xh_cycle_data[k];
 	}
 
 	return &rhead->h_cycle_data[i];
-- 
2.47.3

[PATCH 7/9] xfs: remove the xlog_rec_header_t typedef

[Christoph Hellwig]

There are almost no users of the typedef left, kill it and switch the
remaining users to use the underlying struct.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_log_format.h |  4 ++--
 fs/xfs/xfs_log.c               |  6 +++---
 fs/xfs/xfs_log_recover.c       | 28 ++++++++++++++--------------
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 4cb69bd285ca..908e7060428c 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -136,7 +136,7 @@ struct xlog_rec_ext_header {
 #define XLOG_REC_EXT_SIZE \
 	offsetofend(struct xlog_rec_ext_header, xh_cycle_data)
 
-typedef struct xlog_rec_header {
+struct xlog_rec_header {
 	__be32	  h_magicno;	/* log record (LR) identifier		:  4 */
 	__be32	  h_cycle;	/* write cycle of log			:  4 */
 	__be32	  h_version;	/* LR version				:  4 */
@@ -174,7 +174,7 @@ typedef struct xlog_rec_header {
 
 	__u8	  h_reserved[184];
 	struct xlog_rec_ext_header h_ext[];
-} xlog_rec_header_t;
+};
 
 #ifdef __i386__
 #define XLOG_REC_SIZE		offsetofend(struct xlog_rec_header, h_size)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 1fe3abbd3d36..8b3b79699596 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -2578,9 +2578,9 @@ xlog_state_get_iclog_space(
 	struct xlog_ticket	*ticket,
 	int			*logoffsetp)
 {
-	int		  log_offset;
-	xlog_rec_header_t *head;
-	xlog_in_core_t	  *iclog;
+	int			log_offset;
+	struct xlog_rec_header	*head;
+	struct xlog_in_core	*iclog;
 
 restart:
 	spin_lock(&log->l_icloglock);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index ef0f6efc4381..03e42c7dab56 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -190,8 +190,8 @@ xlog_bwrite(
  */
 STATIC void
 xlog_header_check_dump(
-	xfs_mount_t		*mp,
-	xlog_rec_header_t	*head)
+	struct xfs_mount		*mp,
+	struct xlog_rec_header		*head)
 {
 	xfs_debug(mp, "%s:  SB : uuid = %pU, fmt = %d",
 		__func__, &mp->m_sb.sb_uuid, XLOG_FMT);
@@ -207,8 +207,8 @@ xlog_header_check_dump(
  */
 STATIC int
 xlog_header_check_recover(
-	xfs_mount_t		*mp,
-	xlog_rec_header_t	*head)
+	struct xfs_mount	*mp,
+	struct xlog_rec_header	*head)
 {
 	ASSERT(head->h_magicno == cpu_to_be32(XLOG_HEADER_MAGIC_NUM));
 
@@ -238,8 +238,8 @@ xlog_header_check_recover(
  */
 STATIC int
 xlog_header_check_mount(
-	xfs_mount_t		*mp,
-	xlog_rec_header_t	*head)
+	struct xfs_mount	*mp,
+	struct xlog_rec_header	*head)
 {
 	ASSERT(head->h_magicno == cpu_to_be32(XLOG_HEADER_MAGIC_NUM));
 
@@ -400,7 +400,7 @@ xlog_find_verify_log_record(
 	xfs_daddr_t		i;
 	char			*buffer;
 	char			*offset = NULL;
-	xlog_rec_header_t	*head = NULL;
+	struct xlog_rec_header	*head = NULL;
 	int			error = 0;
 	int			smallmem = 0;
 	int			num_blks = *last_blk - start_blk;
@@ -437,7 +437,7 @@ xlog_find_verify_log_record(
 				goto out;
 		}
 
-		head = (xlog_rec_header_t *)offset;
+		head = (struct xlog_rec_header *)offset;
 
 		if (head->h_magicno == cpu_to_be32(XLOG_HEADER_MAGIC_NUM))
 			break;
@@ -1237,7 +1237,7 @@ xlog_find_tail(
 	xfs_daddr_t		*head_blk,
 	xfs_daddr_t		*tail_blk)
 {
-	xlog_rec_header_t	*rhead;
+	struct xlog_rec_header	*rhead;
 	char			*offset = NULL;
 	char			*buffer;
 	int			error;
@@ -1487,7 +1487,7 @@ xlog_add_record(
 	int			tail_cycle,
 	int			tail_block)
 {
-	xlog_rec_header_t	*recp = (xlog_rec_header_t *)buf;
+	struct xlog_rec_header	*recp = (struct xlog_rec_header *)buf;
 
 	memset(buf, 0, BBSIZE);
 	recp->h_magicno = cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
@@ -2997,7 +2997,7 @@ xlog_do_recovery_pass(
 	int			pass,
 	xfs_daddr_t		*first_bad)	/* out: first bad log rec */
 {
-	xlog_rec_header_t	*rhead;
+	struct xlog_rec_header	*rhead;
 	xfs_daddr_t		blk_no, rblk_no;
 	xfs_daddr_t		rhead_blk;
 	char			*offset;
@@ -3034,7 +3034,7 @@ xlog_do_recovery_pass(
 		if (error)
 			goto bread_err1;
 
-		rhead = (xlog_rec_header_t *)offset;
+		rhead = (struct xlog_rec_header *)offset;
 
 		/*
 		 * xfsprogs has a bug where record length is based on lsunit but
@@ -3141,7 +3141,7 @@ xlog_do_recovery_pass(
 				if (error)
 					goto bread_err2;
 			}
-			rhead = (xlog_rec_header_t *)offset;
+			rhead = (struct xlog_rec_header *)offset;
 			error = xlog_valid_rec_header(log, rhead,
 					split_hblks ? blk_no : 0, h_size);
 			if (error)
@@ -3223,7 +3223,7 @@ xlog_do_recovery_pass(
 		if (error)
 			goto bread_err2;
 
-		rhead = (xlog_rec_header_t *)offset;
+		rhead = (struct xlog_rec_header *)offset;
 		error = xlog_valid_rec_header(log, rhead, blk_no, h_size);
 		if (error)
 			goto bread_err2;
-- 
2.47.3

[PATCH 8/9] xfs: remove l_iclog_heads

[Christoph Hellwig]

l_iclog_heads is only used in one place and can be trivially derived
from l_iclog_hsize by a single shift operation.  Remove it, and switch
the initialization of l_iclog_hsize to use struct_size so that it is
directly derived from the on-disk format definition.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c      | 11 ++++++-----
 fs/xfs/xfs_log_priv.h |  1 -
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 8b3b79699596..47a8e74c8c5c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1279,11 +1279,12 @@ xlog_get_iclog_buffer_size(
 	log->l_iclog_size = mp->m_logbsize;
 
 	/*
-	 * # headers = size / 32k - one header holds cycles from 32k of data.
+	 * Combined size of the log record headers.  The first 32k cycles
+	 * are stored directly in the xlog_rec_header, the rest in the
+	 * variable number of xlog_rec_ext_headers at its end.
 	 */
-	log->l_iclog_heads =
-		DIV_ROUND_UP(mp->m_logbsize, XLOG_HEADER_CYCLE_SIZE);
-	log->l_iclog_hsize = log->l_iclog_heads << BBSHIFT;
+	log->l_iclog_hsize = struct_size(log->l_iclog->ic_header, h_ext,
+		DIV_ROUND_UP(mp->m_logbsize, XLOG_HEADER_CYCLE_SIZE) - 1);
 }
 
 void
@@ -1526,7 +1527,7 @@ xlog_pack_data(
 		dp += BBSIZE;
 	}
 
-	for (i = 0; i < log->l_iclog_heads - 1; i++)
+	for (i = 0; i < (log->l_iclog_hsize >> BBSHIFT) - 1; i++)
 		rhead->h_ext[i].xh_cycle = cycle_lsn;
 }
 
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index ac98ac71152d..17733ba7f251 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -406,7 +406,6 @@ struct xlog {
 	struct list_head	*l_buf_cancel_table;
 	struct list_head	r_dfops;	/* recovered log intent items */
 	int			l_iclog_hsize;  /* size of iclog header */
-	int			l_iclog_heads;  /* # of iclog header sectors */
 	uint			l_sectBBsize;   /* sector size in BBs (2^n) */
 	int			l_iclog_size;	/* size of log in bytes */
 	int			l_iclog_bufs;	/* number of iclog buffers */
-- 
2.47.3

[PATCH 9/9] xfs: remove the xlog_in_core_t typedef

[Christoph Hellwig]

Switch the few remaining users to use the underlying struct directly.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_log.c      | 18 +++++++++---------
 fs/xfs/xfs_log_priv.h |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 47a8e74c8c5c..a311385b23d8 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1368,8 +1368,8 @@ xlog_alloc_log(
 	int			num_bblks)
 {
 	struct xlog		*log;
-	xlog_in_core_t		**iclogp;
-	xlog_in_core_t		*iclog, *prev_iclog=NULL;
+	struct xlog_in_core	**iclogp;
+	struct xlog_in_core	*iclog, *prev_iclog = NULL;
 	int			i;
 	int			error = -ENOMEM;
 	uint			log2_size = 0;
@@ -1813,10 +1813,10 @@ xlog_sync(
  */
 STATIC void
 xlog_dealloc_log(
-	struct xlog	*log)
+	struct xlog		*log)
 {
-	xlog_in_core_t	*iclog, *next_iclog;
-	int		i;
+	struct xlog_in_core	*iclog, *next_iclog;
+	int			i;
 
 	/*
 	 * Destroy the CIL after waiting for iclog IO completion because an
@@ -3293,7 +3293,7 @@ xlog_verify_iclog(
 	int			count)
 {
 	struct xlog_rec_header	*rhead = iclog->ic_header;
-	xlog_in_core_t		*icptr;
+	struct xlog_in_core	*icptr;
 	void			*base_ptr, *ptr;
 	ptrdiff_t		field_offset;
 	uint8_t			clientid;
@@ -3481,11 +3481,10 @@ xlog_force_shutdown(
 
 STATIC int
 xlog_iclogs_empty(
-	struct xlog	*log)
+	struct xlog		*log)
 {
-	xlog_in_core_t	*iclog;
+	struct xlog_in_core	*iclog = log->l_iclog;
 
-	iclog = log->l_iclog;
 	do {
 		/* endianness does not matter here, zero is zero in
 		 * any language.
@@ -3494,6 +3493,7 @@ xlog_iclogs_empty(
 			return 0;
 		iclog = iclog->ic_next;
 	} while (iclog != log->l_iclog);
+
 	return 1;
 }
 
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 17733ba7f251..0fe59f0525aa 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -181,7 +181,7 @@ struct xlog_ticket {
  * We'll put all the read-only and l_icloglock fields in the first cacheline,
  * and move everything else out to subsequent cachelines.
  */
-typedef struct xlog_in_core {
+struct xlog_in_core {
 	wait_queue_head_t	ic_force_wait;
 	wait_queue_head_t	ic_write_wait;
 	struct xlog_in_core	*ic_next;
@@ -204,7 +204,7 @@ typedef struct xlog_in_core {
 	struct work_struct	ic_end_io_work;
 	struct bio		ic_bio;
 	struct bio_vec		ic_bvec[];
-} xlog_in_core_t;
+};
 
 /*
  * The CIL context is used to aggregate per-transaction details as well be
@@ -418,7 +418,7 @@ struct xlog {
 						/* waiting for iclog flush */
 	int			l_covered_state;/* state of "covering disk
 						 * log entries" */
-	xlog_in_core_t		*l_iclog;       /* head log queue	*/
+	struct xlog_in_core	*l_iclog;       /* head log queue	*/
 	spinlock_t		l_icloglock;    /* grab to change iclog state */
 	int			l_curr_cycle;   /* Cycle number of log writes */
 	int			l_prev_cycle;   /* Cycle number before last
-- 
2.47.3

Re: [PATCH 1/9] xfs: add a XLOG_CYCLE_DATA_SIZE constant

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:05AM +0900, Christoph Hellwig wrote:
> The XLOG_HEADER_CYCLE_SIZE / BBSIZE expression is used a lot
> in the log code, give it a symbolic name.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Simple enough conversion...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_log_format.h |  5 +++--
>  fs/xfs/xfs_log.c               | 18 +++++++++---------
>  fs/xfs/xfs_log_recover.c       |  6 +++---
>  3 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 6c50cb2ece19..91a841ea5bb3 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -31,6 +31,7 @@ typedef uint32_t xlog_tid_t;
>  #define XLOG_BIG_RECORD_BSIZE	(32*1024)	/* 32k buffers */
>  #define XLOG_MAX_RECORD_BSIZE	(256*1024)
>  #define XLOG_HEADER_CYCLE_SIZE	(32*1024)	/* cycle data in header */
> +#define XLOG_CYCLE_DATA_SIZE	(XLOG_HEADER_CYCLE_SIZE / BBSIZE)
>  #define XLOG_MIN_RECORD_BSHIFT	14		/* 16384 == 1 << 14 */
>  #define XLOG_BIG_RECORD_BSHIFT	15		/* 32k == 1 << 15 */
>  #define XLOG_MAX_RECORD_BSHIFT	18		/* 256k == 1 << 18 */
> @@ -135,7 +136,7 @@ typedef struct xlog_rec_header {
>  	__le32	  h_crc;	/* crc of log record                    :  4 */
>  	__be32	  h_prev_block; /* block number to previous LR		:  4 */
>  	__be32	  h_num_logops;	/* number of log operations in this LR	:  4 */
> -	__be32	  h_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE];
> +	__be32	  h_cycle_data[XLOG_CYCLE_DATA_SIZE];
>  
>  	/* fields added by the Linux port: */
>  	__be32    h_fmt;        /* format of log record                 :  4 */
> @@ -172,7 +173,7 @@ typedef struct xlog_rec_header {
>  
>  typedef struct xlog_rec_ext_header {
>  	__be32	  xh_cycle;	/* write cycle of log			: 4 */
> -	__be32	  xh_cycle_data[XLOG_HEADER_CYCLE_SIZE / BBSIZE]; /*	: 256 */
> +	__be32	  xh_cycle_data[XLOG_CYCLE_DATA_SIZE];		/*	: 256 */
>  } xlog_rec_ext_header_t;
>  
>  /*
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 603e85c1ab4c..e09e5f71ed8c 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1533,7 +1533,7 @@ xlog_pack_data(
>  
>  	dp = iclog->ic_datap;
>  	for (i = 0; i < BTOBB(size); i++) {
> -		if (i >= (XLOG_HEADER_CYCLE_SIZE / BBSIZE))
> +		if (i >= XLOG_CYCLE_DATA_SIZE)
>  			break;
>  		iclog->ic_header.h_cycle_data[i] = *(__be32 *)dp;
>  		*(__be32 *)dp = cycle_lsn;
> @@ -1544,8 +1544,8 @@ xlog_pack_data(
>  		xlog_in_core_2_t *xhdr = iclog->ic_data;
>  
>  		for ( ; i < BTOBB(size); i++) {
> -			j = i / (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> -			k = i % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> +			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;
> @@ -3368,9 +3368,9 @@ xlog_verify_iclog(
>  			clientid = ophead->oh_clientid;
>  		} else {
>  			idx = BTOBBT((void *)&ophead->oh_clientid - iclog->ic_datap);
> -			if (idx >= (XLOG_HEADER_CYCLE_SIZE / BBSIZE)) {
> -				j = idx / (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> -				k = idx % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> +			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 {
> @@ -3392,9 +3392,9 @@ xlog_verify_iclog(
>  			op_len = be32_to_cpu(ophead->oh_len);
>  		} else {
>  			idx = BTOBBT((void *)&ophead->oh_len - iclog->ic_datap);
> -			if (idx >= (XLOG_HEADER_CYCLE_SIZE / BBSIZE)) {
> -				j = idx / (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> -				k = idx % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> +			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]);
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 549d60959aee..bb2b3f976deb 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2866,7 +2866,7 @@ xlog_unpack_data(
>  	int			i, j, k;
>  
>  	for (i = 0; i < BTOBB(be32_to_cpu(rhead->h_len)) &&
> -		  i < (XLOG_HEADER_CYCLE_SIZE / BBSIZE); i++) {
> +		  i < XLOG_CYCLE_DATA_SIZE; i++) {
>  		*(__be32 *)dp = *(__be32 *)&rhead->h_cycle_data[i];
>  		dp += BBSIZE;
>  	}
> @@ -2874,8 +2874,8 @@ xlog_unpack_data(
>  	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_HEADER_CYCLE_SIZE / BBSIZE);
> -			k = i % (XLOG_HEADER_CYCLE_SIZE / BBSIZE);
> +			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
> 
> 

Re: [PATCH 2/9] xfs: add a on-disk log header cycle array accessor

[Darrick J. Wong]

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
> 
> 

Re: [PATCH 3/9] xfs: don't use xlog_in_core_2_t in struct xlog_in_core

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:07AM +0900, Christoph Hellwig wrote:
> Most accessed to the on-disk log record header are for the original
> xlog_rec_header.  Make that the main structure, and case for the
> single remaining place using other union legs.
> 
> This prepares for removing xlog_in_core_2_t entirely.

Er... so xlog_rec_header is the header that gets written out at the
start of any log buffer?  And if a log record has more than
XLOG_CYCLE_DATA_SIZE basic blocks (BBs) in it, then it'll have some
quantity of "extended" headers in the form of a xlog_rec_ext_header
right after the xlog_rec_header, right?  And both the regular and ext
headers both have a __be32 array containing the original first four
bytes of each BB, because each BB has a munged version of the LSN cycle
stamped into the first four bytes, right?

The previous patch refactored how the cycle_data transformation
happened, right?

So this patch just gets rid of the strange ic_header #define, and
updates the code to access ic_data->hic_header directly?  And now that
we have xlog_cycle_data to abstract the xlog_rec_header ->
xlog_in_core_2_t casting, this just works fine here.  Right?

--D

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c      | 74 +++++++++++++++++++++----------------------
>  fs/xfs/xfs_log_cil.c  |  6 ++--
>  fs/xfs/xfs_log_priv.h |  9 ++----
>  fs/xfs/xfs_trace.h    |  2 +-
>  4 files changed, 44 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index a569a4320a3a..d9476124def6 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -534,8 +534,8 @@ xlog_state_release_iclog(
>  	 */
>  	if ((iclog->ic_state == XLOG_STATE_WANT_SYNC ||
>  	     (iclog->ic_flags & XLOG_ICL_NEED_FUA)) &&
> -	    !iclog->ic_header.h_tail_lsn) {
> -		iclog->ic_header.h_tail_lsn =
> +	    !iclog->ic_header->h_tail_lsn) {
> +		iclog->ic_header->h_tail_lsn =
>  				cpu_to_be64(atomic64_read(&log->l_tail_lsn));
>  	}
>  
> @@ -1457,11 +1457,11 @@ xlog_alloc_log(
>  		iclog->ic_prev = prev_iclog;
>  		prev_iclog = iclog;
>  
> -		iclog->ic_data = kvzalloc(log->l_iclog_size,
> +		iclog->ic_header = kvzalloc(log->l_iclog_size,
>  				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> -		if (!iclog->ic_data)
> +		if (!iclog->ic_header)
>  			goto out_free_iclog;
> -		head = &iclog->ic_header;
> +		head = iclog->ic_header;
>  		memset(head, 0, sizeof(xlog_rec_header_t));
>  		head->h_magicno = cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
>  		head->h_version = cpu_to_be32(
> @@ -1476,7 +1476,7 @@ xlog_alloc_log(
>  		iclog->ic_log = log;
>  		atomic_set(&iclog->ic_refcnt, 0);
>  		INIT_LIST_HEAD(&iclog->ic_callbacks);
> -		iclog->ic_datap = (void *)iclog->ic_data + log->l_iclog_hsize;
> +		iclog->ic_datap = (void *)iclog->ic_header + log->l_iclog_hsize;
>  
>  		init_waitqueue_head(&iclog->ic_force_wait);
>  		init_waitqueue_head(&iclog->ic_write_wait);
> @@ -1504,7 +1504,7 @@ xlog_alloc_log(
>  out_free_iclog:
>  	for (iclog = log->l_iclog; iclog; iclog = prev_iclog) {
>  		prev_iclog = iclog->ic_next;
> -		kvfree(iclog->ic_data);
> +		kvfree(iclog->ic_header);
>  		kfree(iclog);
>  		if (prev_iclog == log->l_iclog)
>  			break;
> @@ -1524,7 +1524,7 @@ xlog_pack_data(
>  	struct xlog_in_core	*iclog,
>  	int			roundoff)
>  {
> -	struct xlog_rec_header	*rhead = &iclog->ic_header;
> +	struct xlog_rec_header	*rhead = iclog->ic_header;
>  	__be32			cycle_lsn = CYCLE_LSN_DISK(rhead->h_lsn);
>  	char			*dp = iclog->ic_datap;
>  	int			i;
> @@ -1536,7 +1536,7 @@ xlog_pack_data(
>  	}
>  
>  	if (xfs_has_logv2(log->l_mp)) {
> -		xlog_in_core_2_t *xhdr = iclog->ic_data;
> +		xlog_in_core_2_t *xhdr = (xlog_in_core_2_t *)iclog->ic_header;
>  
>  		for (i = 1; i < log->l_iclog_heads; i++)
>  			xhdr[i].hic_xheader.xh_cycle = cycle_lsn;
> @@ -1658,11 +1658,11 @@ xlog_write_iclog(
>  
>  	iclog->ic_flags &= ~(XLOG_ICL_NEED_FLUSH | XLOG_ICL_NEED_FUA);
>  
> -	if (is_vmalloc_addr(iclog->ic_data)) {
> -		if (!bio_add_vmalloc(&iclog->ic_bio, iclog->ic_data, count))
> +	if (is_vmalloc_addr(iclog->ic_header)) {
> +		if (!bio_add_vmalloc(&iclog->ic_bio, iclog->ic_header, count))
>  			goto shutdown;
>  	} else {
> -		bio_add_virt_nofail(&iclog->ic_bio, iclog->ic_data, count);
> +		bio_add_virt_nofail(&iclog->ic_bio, iclog->ic_header, count);
>  	}
>  
>  	/*
> @@ -1791,19 +1791,19 @@ xlog_sync(
>  	size = iclog->ic_offset;
>  	if (xfs_has_logv2(log->l_mp))
>  		size += roundoff;
> -	iclog->ic_header.h_len = cpu_to_be32(size);
> +	iclog->ic_header->h_len = cpu_to_be32(size);
>  
>  	XFS_STATS_INC(log->l_mp, xs_log_writes);
>  	XFS_STATS_ADD(log->l_mp, xs_log_blocks, BTOBB(count));
>  
> -	bno = BLOCK_LSN(be64_to_cpu(iclog->ic_header.h_lsn));
> +	bno = BLOCK_LSN(be64_to_cpu(iclog->ic_header->h_lsn));
>  
>  	/* Do we need to split this write into 2 parts? */
>  	if (bno + BTOBB(count) > log->l_logBBsize)
> -		xlog_split_iclog(log, &iclog->ic_header, bno, count);
> +		xlog_split_iclog(log, iclog->ic_header, bno, count);
>  
>  	/* calculcate the checksum */
> -	iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header,
> +	iclog->ic_header->h_crc = xlog_cksum(log, iclog->ic_header,
>  			iclog->ic_datap, XLOG_REC_SIZE, size);
>  	/*
>  	 * Intentionally corrupt the log record CRC based on the error injection
> @@ -1814,11 +1814,11 @@ xlog_sync(
>  	 */
>  #ifdef DEBUG
>  	if (XFS_TEST_ERROR(log->l_mp, XFS_ERRTAG_LOG_BAD_CRC)) {
> -		iclog->ic_header.h_crc &= cpu_to_le32(0xAAAAAAAA);
> +		iclog->ic_header->h_crc &= cpu_to_le32(0xAAAAAAAA);
>  		iclog->ic_fail_crc = true;
>  		xfs_warn(log->l_mp,
>  	"Intentionally corrupted log record at LSN 0x%llx. Shutdown imminent.",
> -			 be64_to_cpu(iclog->ic_header.h_lsn));
> +			 be64_to_cpu(iclog->ic_header->h_lsn));
>  	}
>  #endif
>  	xlog_verify_iclog(log, iclog, count);
> @@ -1845,7 +1845,7 @@ xlog_dealloc_log(
>  	iclog = log->l_iclog;
>  	for (i = 0; i < log->l_iclog_bufs; i++) {
>  		next_iclog = iclog->ic_next;
> -		kvfree(iclog->ic_data);
> +		kvfree(iclog->ic_header);
>  		kfree(iclog);
>  		iclog = next_iclog;
>  	}
> @@ -1867,7 +1867,7 @@ xlog_state_finish_copy(
>  {
>  	lockdep_assert_held(&log->l_icloglock);
>  
> -	be32_add_cpu(&iclog->ic_header.h_num_logops, record_cnt);
> +	be32_add_cpu(&iclog->ic_header->h_num_logops, record_cnt);
>  	iclog->ic_offset += copy_bytes;
>  }
>  
> @@ -2290,7 +2290,7 @@ xlog_state_activate_iclog(
>  	 * We don't need to cover the dummy.
>  	 */
>  	if (*iclogs_changed == 0 &&
> -	    iclog->ic_header.h_num_logops == cpu_to_be32(XLOG_COVER_OPS)) {
> +	    iclog->ic_header->h_num_logops == cpu_to_be32(XLOG_COVER_OPS)) {
>  		*iclogs_changed = 1;
>  	} else {
>  		/*
> @@ -2302,11 +2302,11 @@ xlog_state_activate_iclog(
>  
>  	iclog->ic_state	= XLOG_STATE_ACTIVE;
>  	iclog->ic_offset = 0;
> -	iclog->ic_header.h_num_logops = 0;
> -	memset(iclog->ic_header.h_cycle_data, 0,
> -		sizeof(iclog->ic_header.h_cycle_data));
> -	iclog->ic_header.h_lsn = 0;
> -	iclog->ic_header.h_tail_lsn = 0;
> +	iclog->ic_header->h_num_logops = 0;
> +	memset(iclog->ic_header->h_cycle_data, 0,
> +		sizeof(iclog->ic_header->h_cycle_data));
> +	iclog->ic_header->h_lsn = 0;
> +	iclog->ic_header->h_tail_lsn = 0;
>  }
>  
>  /*
> @@ -2398,7 +2398,7 @@ xlog_get_lowest_lsn(
>  		    iclog->ic_state == XLOG_STATE_DIRTY)
>  			continue;
>  
> -		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> +		lsn = be64_to_cpu(iclog->ic_header->h_lsn);
>  		if ((lsn && !lowest_lsn) || XFS_LSN_CMP(lsn, lowest_lsn) < 0)
>  			lowest_lsn = lsn;
>  	} while ((iclog = iclog->ic_next) != log->l_iclog);
> @@ -2433,7 +2433,7 @@ xlog_state_iodone_process_iclog(
>  		 * If this is not the lowest lsn iclog, then we will leave it
>  		 * for another completion to process.
>  		 */
> -		header_lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> +		header_lsn = be64_to_cpu(iclog->ic_header->h_lsn);
>  		lowest_lsn = xlog_get_lowest_lsn(log);
>  		if (lowest_lsn && XFS_LSN_CMP(lowest_lsn, header_lsn) < 0)
>  			return false;
> @@ -2616,7 +2616,7 @@ xlog_state_get_iclog_space(
>  		goto restart;
>  	}
>  
> -	head = &iclog->ic_header;
> +	head = iclog->ic_header;
>  
>  	atomic_inc(&iclog->ic_refcnt);	/* prevents sync */
>  	log_offset = iclog->ic_offset;
> @@ -2781,7 +2781,7 @@ xlog_state_switch_iclogs(
>  	if (!eventual_size)
>  		eventual_size = iclog->ic_offset;
>  	iclog->ic_state = XLOG_STATE_WANT_SYNC;
> -	iclog->ic_header.h_prev_block = cpu_to_be32(log->l_prev_block);
> +	iclog->ic_header->h_prev_block = cpu_to_be32(log->l_prev_block);
>  	log->l_prev_block = log->l_curr_block;
>  	log->l_prev_cycle = log->l_curr_cycle;
>  
> @@ -2825,7 +2825,7 @@ xlog_force_and_check_iclog(
>  	struct xlog_in_core	*iclog,
>  	bool			*completed)
>  {
> -	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> +	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header->h_lsn);
>  	int			error;
>  
>  	*completed = false;
> @@ -2837,7 +2837,7 @@ xlog_force_and_check_iclog(
>  	 * If the iclog has already been completed and reused the header LSN
>  	 * will have been rewritten by completion
>  	 */
> -	if (be64_to_cpu(iclog->ic_header.h_lsn) != lsn)
> +	if (be64_to_cpu(iclog->ic_header->h_lsn) != lsn)
>  		*completed = true;
>  	return 0;
>  }
> @@ -2970,7 +2970,7 @@ xlog_force_lsn(
>  		goto out_error;
>  
>  	iclog = log->l_iclog;
> -	while (be64_to_cpu(iclog->ic_header.h_lsn) != lsn) {
> +	while (be64_to_cpu(iclog->ic_header->h_lsn) != lsn) {
>  		trace_xlog_iclog_force_lsn(iclog, _RET_IP_);
>  		iclog = iclog->ic_next;
>  		if (iclog == log->l_iclog)
> @@ -3236,7 +3236,7 @@ xlog_verify_dump_tail(
>  {
>  	xfs_alert(log->l_mp,
>  "ran out of log space tail 0x%llx/0x%llx, head lsn 0x%llx, head 0x%x/0x%x, prev head 0x%x/0x%x",
> -			iclog ? be64_to_cpu(iclog->ic_header.h_tail_lsn) : -1,
> +			iclog ? be64_to_cpu(iclog->ic_header->h_tail_lsn) : -1,
>  			atomic64_read(&log->l_tail_lsn),
>  			log->l_ailp->ail_head_lsn,
>  			log->l_curr_cycle, log->l_curr_block,
> @@ -3255,7 +3255,7 @@ xlog_verify_tail_lsn(
>  	struct xlog		*log,
>  	struct xlog_in_core	*iclog)
>  {
> -	xfs_lsn_t	tail_lsn = be64_to_cpu(iclog->ic_header.h_tail_lsn);
> +	xfs_lsn_t	tail_lsn = be64_to_cpu(iclog->ic_header->h_tail_lsn);
>  	int		blocks;
>  
>  	if (CYCLE_LSN(tail_lsn) == log->l_prev_cycle) {
> @@ -3309,7 +3309,7 @@ xlog_verify_iclog(
>  	struct xlog_in_core	*iclog,
>  	int			count)
>  {
> -	struct xlog_rec_header	*rhead = &iclog->ic_header;
> +	struct xlog_rec_header	*rhead = iclog->ic_header;
>  	xlog_in_core_t		*icptr;
>  	void			*base_ptr, *ptr;
>  	ptrdiff_t		field_offset;
> @@ -3507,7 +3507,7 @@ xlog_iclogs_empty(
>  		/* endianness does not matter here, zero is zero in
>  		 * any language.
>  		 */
> -		if (iclog->ic_header.h_num_logops)
> +		if (iclog->ic_header->h_num_logops)
>  			return 0;
>  		iclog = iclog->ic_next;
>  	} while (iclog != log->l_iclog);
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index f443757e93c2..778ac47adb8c 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -940,7 +940,7 @@ xlog_cil_set_ctx_write_state(
>  	struct xlog_in_core	*iclog)
>  {
>  	struct xfs_cil		*cil = ctx->cil;
> -	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> +	xfs_lsn_t		lsn = be64_to_cpu(iclog->ic_header->h_lsn);
>  
>  	ASSERT(!ctx->commit_lsn);
>  	if (!ctx->start_lsn) {
> @@ -1458,9 +1458,9 @@ xlog_cil_push_work(
>  	 */
>  	spin_lock(&log->l_icloglock);
>  	if (ctx->start_lsn != ctx->commit_lsn) {
> -		xfs_lsn_t	plsn;
> +		xfs_lsn_t	plsn = be64_to_cpu(
> +			ctx->commit_iclog->ic_prev->ic_header->h_lsn);
>  
> -		plsn = be64_to_cpu(ctx->commit_iclog->ic_prev->ic_header.h_lsn);
>  		if (plsn && XFS_LSN_CMP(plsn, ctx->commit_lsn) < 0) {
>  			/*
>  			 * Waiting on ic_force_wait orders the completion of
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index d2f17691ecca..f1aed6e8f747 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -158,10 +158,8 @@ struct xlog_ticket {
>  };
>  
>  /*
> - * - A log record header is 512 bytes.  There is plenty of room to grow the
> - *	xlog_rec_header_t into the reserved space.
> - * - ic_data follows, so a write to disk can start at the beginning of
> - *	the iclog.
> + * In-core log structure.
> + *
>   * - ic_forcewait is used to implement synchronous forcing of the iclog to disk.
>   * - ic_next is the pointer to the next iclog in the ring.
>   * - ic_log is a pointer back to the global log structure.
> @@ -198,8 +196,7 @@ typedef struct xlog_in_core {
>  
>  	/* reference counts need their own cacheline */
>  	atomic_t		ic_refcnt ____cacheline_aligned_in_smp;
> -	xlog_in_core_2_t	*ic_data;
> -#define ic_header	ic_data->hic_header
> +	struct xlog_rec_header	*ic_header;
>  #ifdef DEBUG
>  	bool			ic_fail_crc : 1;
>  #endif
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 79b8641880ab..aa3a3870f894 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -4934,7 +4934,7 @@ DECLARE_EVENT_CLASS(xlog_iclog_class,
>  		__entry->refcount = atomic_read(&iclog->ic_refcnt);
>  		__entry->offset = iclog->ic_offset;
>  		__entry->flags = iclog->ic_flags;
> -		__entry->lsn = be64_to_cpu(iclog->ic_header.h_lsn);
> +		__entry->lsn = be64_to_cpu(iclog->ic_header->h_lsn);
>  		__entry->caller_ip = caller_ip;
>  	),
>  	TP_printk("dev %d:%d state %s refcnt %d offset %u lsn 0x%llx flags %s caller %pS",
> -- 
> 2.47.3
> 
> 

Re: [PATCH 4/9] xfs: cleanup xlog_alloc_log a bit

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:08AM +0900, Christoph Hellwig wrote:
> Remove the separate head variable, move the ic_datap initialization
> up a bit where the context is more obvious and remove the duplicate
> memset right after a zeroing memory allocation.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

This looks like a pretty easy variable removal,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index d9476124def6..3bd2f8787682 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1367,7 +1367,6 @@ xlog_alloc_log(
>  	int			num_bblks)
>  {
>  	struct xlog		*log;
> -	xlog_rec_header_t	*head;
>  	xlog_in_core_t		**iclogp;
>  	xlog_in_core_t		*iclog, *prev_iclog=NULL;
>  	int			i;
> @@ -1461,22 +1460,21 @@ xlog_alloc_log(
>  				GFP_KERNEL | __GFP_RETRY_MAYFAIL);
>  		if (!iclog->ic_header)
>  			goto out_free_iclog;
> -		head = iclog->ic_header;
> -		memset(head, 0, sizeof(xlog_rec_header_t));
> -		head->h_magicno = cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
> -		head->h_version = cpu_to_be32(
> +		iclog->ic_header->h_magicno =
> +			cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
> +		iclog->ic_header->h_version = cpu_to_be32(
>  			xfs_has_logv2(log->l_mp) ? 2 : 1);
> -		head->h_size = cpu_to_be32(log->l_iclog_size);
> -		/* new fields */
> -		head->h_fmt = cpu_to_be32(XLOG_FMT);
> -		memcpy(&head->h_fs_uuid, &mp->m_sb.sb_uuid, sizeof(uuid_t));
> +		iclog->ic_header->h_size = cpu_to_be32(log->l_iclog_size);
> +		iclog->ic_header->h_fmt = cpu_to_be32(XLOG_FMT);
> +		memcpy(&iclog->ic_header->h_fs_uuid, &mp->m_sb.sb_uuid,
> +			sizeof(iclog->ic_header->h_fs_uuid));
>  
> +		iclog->ic_datap = (void *)iclog->ic_header + log->l_iclog_hsize;
>  		iclog->ic_size = log->l_iclog_size - log->l_iclog_hsize;
>  		iclog->ic_state = XLOG_STATE_ACTIVE;
>  		iclog->ic_log = log;
>  		atomic_set(&iclog->ic_refcnt, 0);
>  		INIT_LIST_HEAD(&iclog->ic_callbacks);
> -		iclog->ic_datap = (void *)iclog->ic_header + log->l_iclog_hsize;
>  
>  		init_waitqueue_head(&iclog->ic_force_wait);
>  		init_waitqueue_head(&iclog->ic_write_wait);
> -- 
> 2.47.3
> 
> 

Re: [PATCH 5/9] xfs: remove a very outdated comment from xlog_alloc_log

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:09AM +0900, Christoph Hellwig wrote:
> The xlog_iclog definition has been pretty standard for a while, so drop
> this now rather misleading comment.

The iclog structure's size scales proportionately to the log buffer
size, right?  AFAICT it's the iclog header itself plus enough bio_vecs
to map every page of the log buffer.  So there's really nothing weird
about that, right?

If 'yes' to both questions, then
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 3bd2f8787682..acddab467b77 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1435,13 +1435,6 @@ xlog_alloc_log(
>  	init_waitqueue_head(&log->l_flush_wait);
>  
>  	iclogp = &log->l_iclog;
> -	/*
> -	 * The amount of memory to allocate for the iclog structure is
> -	 * rather funky due to the way the structure is defined.  It is
> -	 * done this way so that we can use different sizes for machines
> -	 * with different amounts of memory.  See the definition of
> -	 * xlog_in_core_t in xfs_log_priv.h for details.
> -	 */
>  	ASSERT(log->l_iclog_size >= 4096);
>  	for (i = 0; i < log->l_iclog_bufs; i++) {
>  		size_t bvec_size = howmany(log->l_iclog_size, PAGE_SIZE) *
> -- 
> 2.47.3
> 
> 

Re: [PATCH 6/9] xfs: remove xlog_in_core_2_t

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:10AM +0900, Christoph Hellwig wrote:
> xlog_in_core_2_t is a really odd type, not only is it grossly
> misnamed because it actually is an on-disk structure, but it also
> reprents the actual on-disk structure in a rather odd way.
> 
> A v1 or small v2 log header look like:
> 
> 	+-----------------------+
> 	|      xlog_record      |
> 	+-----------------------+
> 
> while larger v2 log headers look like:
> 
> 	+-----------------------+
> 	|      xlog_record      |
> 	+-----------------------+
> 	|  xlog_rec_ext_header  |
> 	+-------------------+---+
> 	|         .....         |
> 	+-----------------------+
> 	|  xlog_rec_ext_header  |
> 	+-----------------------+
> 
> I.e., the ext headers are a variable sized array at the end of the
> header.  So instead of declaring a union of xlog_rec_header,
> xlog_rec_ext_header and padding to BBSIZE, add the proper padding to
> struct struct xlog_rec_header and struct xlog_rec_ext_header, and
> add a variable sized array of the latter to the former.  This also
> exposes the somewhat unusual scope of the log checksums, which is
> made explicitly now by adding proper padding and macro designating
> the actual payload length.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_log_format.h | 31 +++++++++++++++----------------
>  fs/xfs/libxfs/xfs_ondisk.h     |  6 ++++--
>  fs/xfs/xfs_log.c               | 21 ++++++---------------
>  fs/xfs/xfs_log_priv.h          |  3 +--
>  4 files changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 91a841ea5bb3..4cb69bd285ca 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -126,6 +126,16 @@ struct xlog_op_header {
>  #define XLOG_FMT XLOG_FMT_LINUX_LE
>  #endif
>  
> +struct xlog_rec_ext_header {
> +	__be32		xh_cycle;	/* write cycle of log */
> +	__be32		xh_cycle_data[XLOG_CYCLE_DATA_SIZE];
> +	__u8		xh_reserved[252];

Just out of curiosity, why do we reserve so much space at the end of the
extended header?  Wouldn't it have been more efficient to fill all 508
bytes with xh_cycle_data?

Oh well, water under the bridge now.

> +};
> +
> +/* actual ext header payload size for checksumming */
> +#define XLOG_REC_EXT_SIZE \
> +	offsetofend(struct xlog_rec_ext_header, xh_cycle_data)
> +
>  typedef struct xlog_rec_header {
>  	__be32	  h_magicno;	/* log record (LR) identifier		:  4 */
>  	__be32	  h_cycle;	/* write cycle of log			:  4 */
> @@ -161,30 +171,19 @@ typedef struct xlog_rec_header {
>  	 * (little-endian) architectures.
>  	 */
>  	__u32	  h_pad0;
> +
> +	__u8	  h_reserved[184];
> +	struct xlog_rec_ext_header h_ext[];

Ok, so you're explicitly padding struct xlog_rec_header and
xlog_rec_ext_header to be 512 bytes now, and making the xlog_rec_header
have a VLA of xlog_rec_ext_header.

The log buffer crc is computed from the start of xlog_rec_header::h_crc
to the end(ish) of the xlog_rec_header; and the first 256 bytes of each
xlog_rec_ext_header, right?

>  } xlog_rec_header_t;
>  
>  #ifdef __i386__
>  #define XLOG_REC_SIZE		offsetofend(struct xlog_rec_header, h_size)
> -#define XLOG_REC_SIZE_OTHER	sizeof(struct xlog_rec_header)
> +#define XLOG_REC_SIZE_OTHER	offsetofend(struct xlog_rec_header, h_pad0)
>  #else
> -#define XLOG_REC_SIZE		sizeof(struct xlog_rec_header)
> +#define XLOG_REC_SIZE		offsetofend(struct xlog_rec_header, h_pad0)
>  #define XLOG_REC_SIZE_OTHER	offsetofend(struct xlog_rec_header, h_size)
>  #endif /* __i386__ */
>  
> -typedef struct xlog_rec_ext_header {
> -	__be32	  xh_cycle;	/* write cycle of log			: 4 */
> -	__be32	  xh_cycle_data[XLOG_CYCLE_DATA_SIZE];		/*	: 256 */
> -} xlog_rec_ext_header_t;
> -
> -/*
> - * Quite misnamed, because this union lays out the actual on-disk log buffer.
> - */
> -typedef union xlog_in_core2 {
> -	xlog_rec_header_t	hic_header;
> -	xlog_rec_ext_header_t	hic_xheader;
> -	char			hic_sector[XLOG_HEADER_SIZE];

That /was/ effin' weird.

> -} xlog_in_core_2_t;
> -
>  /* not an on-disk structure, but needed by log recovery in userspace */
>  struct xfs_log_iovec {
>  	void		*i_addr;	/* beginning address of region */
> diff --git a/fs/xfs/libxfs/xfs_ondisk.h b/fs/xfs/libxfs/xfs_ondisk.h
> index 7bfa3242e2c5..2e9715cc1641 100644
> --- a/fs/xfs/libxfs/xfs_ondisk.h
> +++ b/fs/xfs/libxfs/xfs_ondisk.h
> @@ -174,9 +174,11 @@ xfs_check_ondisk_structs(void)
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format,	16);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent,		32);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent,		16);
> -	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_header,		328);
> -	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_ext_header,	260);
> +	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_header,		512);
> +	XFS_CHECK_STRUCT_SIZE(struct xlog_rec_ext_header,	512);
>  
> +	XFS_CHECK_OFFSET(struct xlog_rec_header, h_reserved,		328);
> +	XFS_CHECK_OFFSET(struct xlog_rec_ext_header, xh_reserved,	260);
>  	XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents,	16);
>  	XFS_CHECK_OFFSET(struct xfs_cui_log_format, cui_extents,	16);
>  	XFS_CHECK_OFFSET(struct xfs_rui_log_format, rui_extents,	16);
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index acddab467b77..1fe3abbd3d36 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1526,12 +1526,8 @@ xlog_pack_data(
>  		dp += BBSIZE;
>  	}
>  
> -	if (xfs_has_logv2(log->l_mp)) {

Is the xfs_has_logv2 still necessary here?

What happens if log->l_iclog_heads > 1 && !logv2?  Or has the kernel
already checked for that and aborted the mount?

(I /think/ the data format changes look ok, but wowee this patch
generated questions :P)

--D

> -		xlog_in_core_2_t *xhdr = (xlog_in_core_2_t *)iclog->ic_header;
> -
> -		for (i = 1; i < log->l_iclog_heads; i++)
> -			xhdr[i].hic_xheader.xh_cycle = cycle_lsn;
> -	}
> +	for (i = 0; i < log->l_iclog_heads - 1; i++)
> +		rhead->h_ext[i].xh_cycle = cycle_lsn;
>  }
>  
>  /*
> @@ -1556,16 +1552,11 @@ xlog_cksum(
>  
>  	/* ... then for additional cycle data for v2 logs ... */
>  	if (xfs_has_logv2(log->l_mp)) {
> -		union xlog_in_core2 *xhdr = (union xlog_in_core2 *)rhead;
> -		int		i;
> -		int		xheads;
> +		int		xheads, i;
>  
> -		xheads = DIV_ROUND_UP(size, XLOG_HEADER_CYCLE_SIZE);
> -
> -		for (i = 1; i < xheads; i++) {
> -			crc = crc32c(crc, &xhdr[i].hic_xheader,
> -				     sizeof(struct xlog_rec_ext_header));
> -		}
> +		xheads = DIV_ROUND_UP(size, XLOG_HEADER_CYCLE_SIZE) - 1;
> +		for (i = 0; i < xheads; i++)
> +			crc = crc32c(crc, &rhead->h_ext[i], XLOG_REC_EXT_SIZE);
>  	}
>  
>  	/* ... and finally for the payload */
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index f1aed6e8f747..ac98ac71152d 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -716,11 +716,10 @@ xlog_item_space(
>  static inline __be32 *xlog_cycle_data(struct xlog_rec_header *rhead, unsigned i)
>  {
>  	if (i >= XLOG_CYCLE_DATA_SIZE) {
> -		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_ext[j - 1].xh_cycle_data[k];
>  	}
>  
>  	return &rhead->h_cycle_data[i];
> -- 
> 2.47.3
> 
> 

Re: [PATCH 7/9] xfs: remove the xlog_rec_header_t typedef

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:11AM +0900, Christoph Hellwig wrote:
> There are almost no users of the typedef left, kill it and switch the
> remaining users to use the underlying struct.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Woot.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_log_format.h |  4 ++--
>  fs/xfs/xfs_log.c               |  6 +++---
>  fs/xfs/xfs_log_recover.c       | 28 ++++++++++++++--------------
>  3 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 4cb69bd285ca..908e7060428c 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -136,7 +136,7 @@ struct xlog_rec_ext_header {
>  #define XLOG_REC_EXT_SIZE \
>  	offsetofend(struct xlog_rec_ext_header, xh_cycle_data)
>  
> -typedef struct xlog_rec_header {
> +struct xlog_rec_header {
>  	__be32	  h_magicno;	/* log record (LR) identifier		:  4 */
>  	__be32	  h_cycle;	/* write cycle of log			:  4 */
>  	__be32	  h_version;	/* LR version				:  4 */
> @@ -174,7 +174,7 @@ typedef struct xlog_rec_header {
>  
>  	__u8	  h_reserved[184];
>  	struct xlog_rec_ext_header h_ext[];
> -} xlog_rec_header_t;
> +};
>  
>  #ifdef __i386__
>  #define XLOG_REC_SIZE		offsetofend(struct xlog_rec_header, h_size)
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 1fe3abbd3d36..8b3b79699596 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -2578,9 +2578,9 @@ xlog_state_get_iclog_space(
>  	struct xlog_ticket	*ticket,
>  	int			*logoffsetp)
>  {
> -	int		  log_offset;
> -	xlog_rec_header_t *head;
> -	xlog_in_core_t	  *iclog;
> +	int			log_offset;
> +	struct xlog_rec_header	*head;
> +	struct xlog_in_core	*iclog;
>  
>  restart:
>  	spin_lock(&log->l_icloglock);
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index ef0f6efc4381..03e42c7dab56 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -190,8 +190,8 @@ xlog_bwrite(
>   */
>  STATIC void
>  xlog_header_check_dump(
> -	xfs_mount_t		*mp,
> -	xlog_rec_header_t	*head)
> +	struct xfs_mount		*mp,
> +	struct xlog_rec_header		*head)
>  {
>  	xfs_debug(mp, "%s:  SB : uuid = %pU, fmt = %d",
>  		__func__, &mp->m_sb.sb_uuid, XLOG_FMT);
> @@ -207,8 +207,8 @@ xlog_header_check_dump(
>   */
>  STATIC int
>  xlog_header_check_recover(
> -	xfs_mount_t		*mp,
> -	xlog_rec_header_t	*head)
> +	struct xfs_mount	*mp,
> +	struct xlog_rec_header	*head)
>  {
>  	ASSERT(head->h_magicno == cpu_to_be32(XLOG_HEADER_MAGIC_NUM));
>  
> @@ -238,8 +238,8 @@ xlog_header_check_recover(
>   */
>  STATIC int
>  xlog_header_check_mount(
> -	xfs_mount_t		*mp,
> -	xlog_rec_header_t	*head)
> +	struct xfs_mount	*mp,
> +	struct xlog_rec_header	*head)
>  {
>  	ASSERT(head->h_magicno == cpu_to_be32(XLOG_HEADER_MAGIC_NUM));
>  
> @@ -400,7 +400,7 @@ xlog_find_verify_log_record(
>  	xfs_daddr_t		i;
>  	char			*buffer;
>  	char			*offset = NULL;
> -	xlog_rec_header_t	*head = NULL;
> +	struct xlog_rec_header	*head = NULL;
>  	int			error = 0;
>  	int			smallmem = 0;
>  	int			num_blks = *last_blk - start_blk;
> @@ -437,7 +437,7 @@ xlog_find_verify_log_record(
>  				goto out;
>  		}
>  
> -		head = (xlog_rec_header_t *)offset;
> +		head = (struct xlog_rec_header *)offset;
>  
>  		if (head->h_magicno == cpu_to_be32(XLOG_HEADER_MAGIC_NUM))
>  			break;
> @@ -1237,7 +1237,7 @@ xlog_find_tail(
>  	xfs_daddr_t		*head_blk,
>  	xfs_daddr_t		*tail_blk)
>  {
> -	xlog_rec_header_t	*rhead;
> +	struct xlog_rec_header	*rhead;
>  	char			*offset = NULL;
>  	char			*buffer;
>  	int			error;
> @@ -1487,7 +1487,7 @@ xlog_add_record(
>  	int			tail_cycle,
>  	int			tail_block)
>  {
> -	xlog_rec_header_t	*recp = (xlog_rec_header_t *)buf;
> +	struct xlog_rec_header	*recp = (struct xlog_rec_header *)buf;
>  
>  	memset(buf, 0, BBSIZE);
>  	recp->h_magicno = cpu_to_be32(XLOG_HEADER_MAGIC_NUM);
> @@ -2997,7 +2997,7 @@ xlog_do_recovery_pass(
>  	int			pass,
>  	xfs_daddr_t		*first_bad)	/* out: first bad log rec */
>  {
> -	xlog_rec_header_t	*rhead;
> +	struct xlog_rec_header	*rhead;
>  	xfs_daddr_t		blk_no, rblk_no;
>  	xfs_daddr_t		rhead_blk;
>  	char			*offset;
> @@ -3034,7 +3034,7 @@ xlog_do_recovery_pass(
>  		if (error)
>  			goto bread_err1;
>  
> -		rhead = (xlog_rec_header_t *)offset;
> +		rhead = (struct xlog_rec_header *)offset;
>  
>  		/*
>  		 * xfsprogs has a bug where record length is based on lsunit but
> @@ -3141,7 +3141,7 @@ xlog_do_recovery_pass(
>  				if (error)
>  					goto bread_err2;
>  			}
> -			rhead = (xlog_rec_header_t *)offset;
> +			rhead = (struct xlog_rec_header *)offset;
>  			error = xlog_valid_rec_header(log, rhead,
>  					split_hblks ? blk_no : 0, h_size);
>  			if (error)
> @@ -3223,7 +3223,7 @@ xlog_do_recovery_pass(
>  		if (error)
>  			goto bread_err2;
>  
> -		rhead = (xlog_rec_header_t *)offset;
> +		rhead = (struct xlog_rec_header *)offset;
>  		error = xlog_valid_rec_header(log, rhead, blk_no, h_size);
>  		if (error)
>  			goto bread_err2;
> -- 
> 2.47.3
> 
> 

Re: [PATCH 8/9] xfs: remove l_iclog_heads

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:12AM +0900, Christoph Hellwig wrote:
> l_iclog_heads is only used in one place and can be trivially derived
> from l_iclog_hsize by a single shift operation.  Remove it, and switch
> the initialization of l_iclog_hsize to use struct_size so that it is
> directly derived from the on-disk format definition.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_log.c      | 11 ++++++-----
>  fs/xfs/xfs_log_priv.h |  1 -
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 8b3b79699596..47a8e74c8c5c 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1279,11 +1279,12 @@ xlog_get_iclog_buffer_size(
>  	log->l_iclog_size = mp->m_logbsize;
>  
>  	/*
> -	 * # headers = size / 32k - one header holds cycles from 32k of data.
> +	 * Combined size of the log record headers.  The first 32k cycles
> +	 * are stored directly in the xlog_rec_header, the rest in the
> +	 * variable number of xlog_rec_ext_headers at its end.
>  	 */
> -	log->l_iclog_heads =
> -		DIV_ROUND_UP(mp->m_logbsize, XLOG_HEADER_CYCLE_SIZE);
> -	log->l_iclog_hsize = log->l_iclog_heads << BBSHIFT;
> +	log->l_iclog_hsize = struct_size(log->l_iclog->ic_header, h_ext,
> +		DIV_ROUND_UP(mp->m_logbsize, XLOG_HEADER_CYCLE_SIZE) - 1);

Hrm.  offsetof(xlog_rec_header::h_ext) == 512 and
sizeof(xlog_rec_ext_header) == 512, so I think this computation still
yields the same results.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  }
>  
>  void
> @@ -1526,7 +1527,7 @@ xlog_pack_data(
>  		dp += BBSIZE;
>  	}
>  
> -	for (i = 0; i < log->l_iclog_heads - 1; i++)
> +	for (i = 0; i < (log->l_iclog_hsize >> BBSHIFT) - 1; i++)
>  		rhead->h_ext[i].xh_cycle = cycle_lsn;
>  }
>  
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index ac98ac71152d..17733ba7f251 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -406,7 +406,6 @@ struct xlog {
>  	struct list_head	*l_buf_cancel_table;
>  	struct list_head	r_dfops;	/* recovered log intent items */
>  	int			l_iclog_hsize;  /* size of iclog header */
> -	int			l_iclog_heads;  /* # of iclog header sectors */
>  	uint			l_sectBBsize;   /* sector size in BBs (2^n) */
>  	int			l_iclog_size;	/* size of log in bytes */
>  	int			l_iclog_bufs;	/* number of iclog buffers */
> -- 
> 2.47.3
> 
> 

Re: [PATCH 9/9] xfs: remove the xlog_in_core_t typedef

[Darrick J. Wong]

On Mon, Oct 13, 2025 at 11:42:13AM +0900, Christoph Hellwig wrote:
> Switch the few remaining users to use the underlying struct directly.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Gooooody!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log.c      | 18 +++++++++---------
>  fs/xfs/xfs_log_priv.h |  6 +++---
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 47a8e74c8c5c..a311385b23d8 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1368,8 +1368,8 @@ xlog_alloc_log(
>  	int			num_bblks)
>  {
>  	struct xlog		*log;
> -	xlog_in_core_t		**iclogp;
> -	xlog_in_core_t		*iclog, *prev_iclog=NULL;
> +	struct xlog_in_core	**iclogp;
> +	struct xlog_in_core	*iclog, *prev_iclog = NULL;
>  	int			i;
>  	int			error = -ENOMEM;
>  	uint			log2_size = 0;
> @@ -1813,10 +1813,10 @@ xlog_sync(
>   */
>  STATIC void
>  xlog_dealloc_log(
> -	struct xlog	*log)
> +	struct xlog		*log)
>  {
> -	xlog_in_core_t	*iclog, *next_iclog;
> -	int		i;
> +	struct xlog_in_core	*iclog, *next_iclog;
> +	int			i;
>  
>  	/*
>  	 * Destroy the CIL after waiting for iclog IO completion because an
> @@ -3293,7 +3293,7 @@ xlog_verify_iclog(
>  	int			count)
>  {
>  	struct xlog_rec_header	*rhead = iclog->ic_header;
> -	xlog_in_core_t		*icptr;
> +	struct xlog_in_core	*icptr;
>  	void			*base_ptr, *ptr;
>  	ptrdiff_t		field_offset;
>  	uint8_t			clientid;
> @@ -3481,11 +3481,10 @@ xlog_force_shutdown(
>  
>  STATIC int
>  xlog_iclogs_empty(
> -	struct xlog	*log)
> +	struct xlog		*log)
>  {
> -	xlog_in_core_t	*iclog;
> +	struct xlog_in_core	*iclog = log->l_iclog;
>  
> -	iclog = log->l_iclog;
>  	do {
>  		/* endianness does not matter here, zero is zero in
>  		 * any language.
> @@ -3494,6 +3493,7 @@ xlog_iclogs_empty(
>  			return 0;
>  		iclog = iclog->ic_next;
>  	} while (iclog != log->l_iclog);
> +
>  	return 1;
>  }
>  
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 17733ba7f251..0fe59f0525aa 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -181,7 +181,7 @@ struct xlog_ticket {
>   * We'll put all the read-only and l_icloglock fields in the first cacheline,
>   * and move everything else out to subsequent cachelines.
>   */
> -typedef struct xlog_in_core {
> +struct xlog_in_core {
>  	wait_queue_head_t	ic_force_wait;
>  	wait_queue_head_t	ic_write_wait;
>  	struct xlog_in_core	*ic_next;
> @@ -204,7 +204,7 @@ typedef struct xlog_in_core {
>  	struct work_struct	ic_end_io_work;
>  	struct bio		ic_bio;
>  	struct bio_vec		ic_bvec[];
> -} xlog_in_core_t;
> +};
>  
>  /*
>   * The CIL context is used to aggregate per-transaction details as well be
> @@ -418,7 +418,7 @@ struct xlog {
>  						/* waiting for iclog flush */
>  	int			l_covered_state;/* state of "covering disk
>  						 * log entries" */
> -	xlog_in_core_t		*l_iclog;       /* head log queue	*/
> +	struct xlog_in_core	*l_iclog;       /* head log queue	*/
>  	spinlock_t		l_icloglock;    /* grab to change iclog state */
>  	int			l_curr_cycle;   /* Cycle number of log writes */
>  	int			l_prev_cycle;   /* Cycle number before last
> -- 
> 2.47.3
> 
> 

Re: [PATCH 2/9] xfs: add a on-disk log header cycle array accessor

[Christoph Hellwig]

On Tue, Oct 14, 2025 at 02:27:17PM -0700, Darrick J. Wong wrote:
> > @@ -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?

Yes.

> > +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?

It can't happen.  It is bound by l_iclog_hsize or h_len, which
can't go into the subsequent blocks for v1 logs.  For the recovery
case we also check for h_len being smaller than h_size, and h_size
is fixed to XLOG_BIG_RECORD_BSIZE for v1 logs.

Re: [PATCH 3/9] xfs: don't use xlog_in_core_2_t in struct xlog_in_core

[Christoph Hellwig]

On Tue, Oct 14, 2025 at 02:47:42PM -0700, Darrick J. Wong wrote:
> On Mon, Oct 13, 2025 at 11:42:07AM +0900, Christoph Hellwig wrote:
> > Most accessed to the on-disk log record header are for the original
> > xlog_rec_header.  Make that the main structure, and case for the
> > single remaining place using other union legs.
> > 
> > This prepares for removing xlog_in_core_2_t entirely.
> 
> Er... so xlog_rec_header is the header that gets written out at the
> start of any log buffer?

Yes.

> And if a log record has more than
> XLOG_CYCLE_DATA_SIZE basic blocks (BBs) in it, then it'll have some
> quantity of "extended" headers in the form of a xlog_rec_ext_header
> right after the xlog_rec_header, right?

They are not directly behіnd the current definition of the
xlog_rec_header, but rather at each multiple of 512 bytes past the
start of the xlog_rec_header.

> And both the regular and ext
> headers both have a __be32 array containing the original first four
> bytes of each BB, because each BB has a munged version of the LSN cycle
> stamped into the first four bytes, right?

Yes.

> The previous patch refactored how the cycle_data transformation
> happened, right?

Yes.

> So this patch just gets rid of the strange ic_header #define, and
> updates the code to access ic_data->hic_header directly?  And now that
> we have xlog_cycle_data to abstract the xlog_rec_header ->
> xlog_in_core_2_t casting, this just works fine here.  Right?

Yes.

Re: [PATCH 5/9] xfs: remove a very outdated comment from xlog_alloc_log

[Christoph Hellwig]

On Tue, Oct 14, 2025 at 02:52:26PM -0700, Darrick J. Wong wrote:
> On Mon, Oct 13, 2025 at 11:42:09AM +0900, Christoph Hellwig wrote:
> > The xlog_iclog definition has been pretty standard for a while, so drop
> > this now rather misleading comment.
> 
> The iclog structure's size scales proportionately to the log buffer
> size, right?  AFAICT it's the iclog header itself plus enough bio_vecs
> to map every page of the log buffer.  So there's really nothing weird
> about that, right?

Yes.

(it might be worth to look into just using high order folios for
the on-disk log, that way we'd always need exactly two bio_vecs and
make the bio building simpler, but that's a separate discussion)

Re: [PATCH 6/9] xfs: remove xlog_in_core_2_t

[Christoph Hellwig]

On Tue, Oct 14, 2025 at 03:07:57PM -0700, Darrick J. Wong wrote:
> > +	__be32		xh_cycle_data[XLOG_CYCLE_DATA_SIZE];
> > +	__u8		xh_reserved[252];
> 
> Just out of curiosity, why do we reserve so much space at the end of the
> extended header?  Wouldn't it have been more efficient to fill all 508
> bytes with xh_cycle_data?

That's something I asked myself when doing this as well.  Or if we're
so inefficient, why don't we at least put the cycle data into the same
offset for both the initial and extended headers.  Also why do we write
xh_cycle into the extended header and don't every read it?

> > +
> > +	__u8	  h_reserved[184];
> > +	struct xlog_rec_ext_header h_ext[];
> 
> Ok, so you're explicitly padding struct xlog_rec_header and
> xlog_rec_ext_header to be 512 bytes now, and making the xlog_rec_header
> have a VLA of xlog_rec_ext_header.

Yes.

> The log buffer crc is computed from the start of xlog_rec_header::h_crc
> to the end(ish) of the xlog_rec_header; and the first 256 bytes of each
> xlog_rec_ext_header, right?

Yes.

> > +++ b/fs/xfs/xfs_log.c
> > @@ -1526,12 +1526,8 @@ xlog_pack_data(
> >  		dp += BBSIZE;
> >  	}
> >  
> > -	if (xfs_has_logv2(log->l_mp)) {
> 
> Is the xfs_has_logv2 still necessary here?
> 
> What happens if log->l_iclog_heads > 1 && !logv2?  Or has the kernel
> already checked for that and aborted the mount?

l_iclog_heads is set based on m_logbsize, and xfs_finish_flags verifies
that it is never bigger than XLOG_BIG_RECORD_BSIZE for v1 logs.

Re: [PATCH 2/9] xfs: add a on-disk log header cycle array accessor

[Darrick J. Wong]

On Wed, Oct 15, 2025 at 06:32:38AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 14, 2025 at 02:27:17PM -0700, Darrick J. Wong wrote:
> > > @@ -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?
> 
> Yes.
> 
> > > +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?
> 
> It can't happen.  It is bound by l_iclog_hsize or h_len, which
> can't go into the subsequent blocks for v1 logs.  For the recovery
> case we also check for h_len being smaller than h_size, and h_size
> is fixed to XLOG_BIG_RECORD_BSIZE for v1 logs.

Oh ok.  Yeah, that's right, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

Re: [PATCH 3/9] xfs: don't use xlog_in_core_2_t in struct xlog_in_core

[Darrick J. Wong]

On Wed, Oct 15, 2025 at 06:34:22AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 14, 2025 at 02:47:42PM -0700, Darrick J. Wong wrote:
> > On Mon, Oct 13, 2025 at 11:42:07AM +0900, Christoph Hellwig wrote:
> > > Most accessed to the on-disk log record header are for the original
> > > xlog_rec_header.  Make that the main structure, and case for the
> > > single remaining place using other union legs.
> > > 
> > > This prepares for removing xlog_in_core_2_t entirely.
> > 
> > Er... so xlog_rec_header is the header that gets written out at the
> > start of any log buffer?
> 
> Yes.
> 
> > And if a log record has more than
> > XLOG_CYCLE_DATA_SIZE basic blocks (BBs) in it, then it'll have some
> > quantity of "extended" headers in the form of a xlog_rec_ext_header
> > right after the xlog_rec_header, right?
> 
> They are not directly behіnd the current definition of the
> xlog_rec_header, but rather at each multiple of 512 bytes past the
> start of the xlog_rec_header.

<nod> And that only became obvious after I read through that patch that
removes xlog_in_core_2_t.

> > And both the regular and ext
> > headers both have a __be32 array containing the original first four
> > bytes of each BB, because each BB has a munged version of the LSN cycle
> > stamped into the first four bytes, right?
> 
> Yes.
> 
> > The previous patch refactored how the cycle_data transformation
> > happened, right?
> 
> Yes.
> 
> > So this patch just gets rid of the strange ic_header #define, and
> > updates the code to access ic_data->hic_header directly?  And now that
> > we have xlog_cycle_data to abstract the xlog_rec_header ->
> > xlog_in_core_2_t casting, this just works fine here.  Right?
> 
> Yes.

I'm satisfied then.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

Re: [PATCH 6/9] xfs: remove xlog_in_core_2_t

[Darrick J. Wong]

On Wed, Oct 15, 2025 at 06:41:09AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 14, 2025 at 03:07:57PM -0700, Darrick J. Wong wrote:
> > > +	__be32		xh_cycle_data[XLOG_CYCLE_DATA_SIZE];
> > > +	__u8		xh_reserved[252];
> > 
> > Just out of curiosity, why do we reserve so much space at the end of the
> > extended header?  Wouldn't it have been more efficient to fill all 508
> > bytes with xh_cycle_data?
> 
> That's something I asked myself when doing this as well.  Or if we're
> so inefficient, why don't we at least put the cycle data into the same
> offset for both the initial and extended headers.  Also why do we write
> xh_cycle into the extended header and don't every read it?

Oh well, log v3 then. :(

> > > +
> > > +	__u8	  h_reserved[184];
> > > +	struct xlog_rec_ext_header h_ext[];
> > 
> > Ok, so you're explicitly padding struct xlog_rec_header and
> > xlog_rec_ext_header to be 512 bytes now, and making the xlog_rec_header
> > have a VLA of xlog_rec_ext_header.
> 
> Yes.
> 
> > The log buffer crc is computed from the start of xlog_rec_header::h_crc
> > to the end(ish) of the xlog_rec_header; and the first 256 bytes of each
> > xlog_rec_ext_header, right?
> 
> Yes.
> 
> > > +++ b/fs/xfs/xfs_log.c
> > > @@ -1526,12 +1526,8 @@ xlog_pack_data(
> > >  		dp += BBSIZE;
> > >  	}
> > >  
> > > -	if (xfs_has_logv2(log->l_mp)) {
> > 
> > Is the xfs_has_logv2 still necessary here?
> > 
> > What happens if log->l_iclog_heads > 1 && !logv2?  Or has the kernel
> > already checked for that and aborted the mount?
> 
> l_iclog_heads is set based on m_logbsize, and xfs_finish_flags verifies
> that it is never bigger than XLOG_BIG_RECORD_BSIZE for v1 logs.

Sounds good to me!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

[PATCH 2/9] xfs: add a on-disk log header cycle array accessor

[Christoph Hellwig]

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>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
 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;
-		}
-
 		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) {
+		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

Re: [PATCH 2/9] xfs: add a on-disk log header cycle array accessor

[Carlos Maiolino]

On Mon, Oct 27, 2025 at 08:05:49AM +0100, 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>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  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;
> -		}
> -
>  		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) {
> +		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
>