linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* fix missing RT detection in libxfs
@ 2026-09-11 14:45 Christoph Hellwig
  2026-09-11 14:45 ` [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-09-11 14:45 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

Hi all,

the first patch in this series fixes the detection of a missing rtdev
specification in libxfs, and the other two clean up lose ends in this
area that I stumbled upon while debugging the original issue.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided
  2026-09-11 14:45 fix missing RT detection in libxfs Christoph Hellwig
@ 2026-09-11 14:45 ` Christoph Hellwig
  2026-09-11 14:57   ` Darrick J. Wong
  2026-09-11 14:45 ` [PATCH 2/3] libxfs: better describe why m_ddev_targp can be set in libxfs_buftarg_init Christoph Hellwig
  2026-09-11 14:45 ` [PATCH 3/3] libxfs: remove buftarg member aliases Christoph Hellwig
  2 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2026-09-11 14:45 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

The user-space only parts of commit 9840f7e09e2f ("xfs: allow internal
RT devices for zoned mode") accidentally set m_rtdev_targp to
m_ddev_targp when not name is set for the RT device, and thus disable
the check for a non-NULL m_rtdev_targp in rtmount_init.

This lead to tools working without specifying a RT device when they
should abort.  For example this can lead to repair trying to read
and rewrite the rtsb when called without -rc, which will then fail
in weird ways.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 libxfs/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libxfs/init.c b/libxfs/init.c
index 5d8b4a153e28..2a46ddf086ed 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -573,7 +573,7 @@ libxfs_buftarg_init(
 	else
 		mp->m_logdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->log,
 				lfail);
-	if (!xi->rt.dev || xi->rt.dev == xi->data.dev)
+	if (xi->rt.dev == xi->data.dev)
 		mp->m_rtdev_targp = mp->m_ddev_targp;
 	else
 		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/3] libxfs: better describe why m_ddev_targp can be set in libxfs_buftarg_init
  2026-09-11 14:45 fix missing RT detection in libxfs Christoph Hellwig
  2026-09-11 14:45 ` [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided Christoph Hellwig
@ 2026-09-11 14:45 ` Christoph Hellwig
  2026-09-11 14:53   ` Darrick J. Wong
  2026-09-11 14:45 ` [PATCH 3/3] libxfs: remove buftarg member aliases Christoph Hellwig
  2 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2026-09-11 14:45 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

I had to wade through this to understand what is going on here, so dump
the thoughts in a comment to make it easier for the next person to
understand the logic.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 libxfs/init.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libxfs/init.c b/libxfs/init.c
index 2a46ddf086ed..a47e610934e9 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -534,7 +534,12 @@ libxfs_buftarg_init(
 	}
 
 	if (mp->m_ddev_targp) {
-		/* should already have all buftargs initialised */
+		/*
+		 * This can happen if the utility called libxfs_buftarg_init
+		 * manually before libxfs_mount, which calls us again.
+		 *
+		 * In this case all buftargs should be initialized already.
+		 */
 		if (mp->m_ddev_targp->bt_bdev != xi->data.dev ||
 		    mp->m_ddev_targp->bt_mount != mp) {
 			fprintf(stderr,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 3/3] libxfs: remove buftarg member aliases
  2026-09-11 14:45 fix missing RT detection in libxfs Christoph Hellwig
  2026-09-11 14:45 ` [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided Christoph Hellwig
  2026-09-11 14:45 ` [PATCH 2/3] libxfs: better describe why m_ddev_targp can be set in libxfs_buftarg_init Christoph Hellwig
@ 2026-09-11 14:45 ` Christoph Hellwig
  2026-09-11 14:53   ` Darrick J. Wong
  2 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2026-09-11 14:45 UTC (permalink / raw)
  To: Andrey Albershteyn; +Cc: linux-xfs

Remove the #defines giving historic IRIX names to the ddev/logdev/rtdev
buftargs in struct xfs_mount and use the current kernel names everywhere.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/xfs_mount.h  |  3 ---
 libxfs/init.c        | 12 ++++++------
 libxfs/trans.c       |  4 ++--
 libxfs/util.c        |  2 +-
 mkfs/xfs_mkfs.c      |  4 ++--
 repair/attr_repair.c | 11 ++++++-----
 repair/da_util.c     |  2 +-
 repair/dino_chunks.c |  8 ++++----
 repair/dinode.c      |  8 ++++----
 repair/phase3.c      |  2 +-
 repair/phase5.c      |  6 +++---
 repair/prefetch.c    |  4 ++--
 repair/quotacheck.c  |  2 +-
 repair/rt.c          |  2 +-
 repair/scan.c        | 16 +++++++++-------
 15 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/include/xfs_mount.h b/include/xfs_mount.h
index 5a714333c16e..0dea578ee3a5 100644
--- a/include/xfs_mount.h
+++ b/include/xfs_mount.h
@@ -102,9 +102,6 @@ typedef struct xfs_mount {
 	struct xfs_buftarg	*m_ddev_targp;
 	struct xfs_buftarg	*m_logdev_targp;
 	struct xfs_buftarg	*m_rtdev_targp;
-#define m_dev		m_ddev_targp
-#define m_logdev	m_logdev_targp
-#define m_rtdev		m_rtdev_targp
 	uint8_t			m_dircook_elog;	/* log d-cookie entry bits */
 	uint8_t			m_blkbit_log;	/* blocklog + NBBY */
 	uint8_t			m_blkbb_log;	/* blocklog - BBSHIFT */
diff --git a/libxfs/init.c b/libxfs/init.c
index a47e610934e9..97c4755dd131 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -331,7 +331,7 @@ rtmount_init(
 			(unsigned long long) mp->m_sb.sb_rblocks);
 		return -1;
 	}
-	error = libxfs_buf_read(mp->m_rtdev, d - XFS_FSB_TO_BB(mp, 1),
+	error = libxfs_buf_read(mp->m_rtdev_targp, d - XFS_FSB_TO_BB(mp, 1),
 			XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
 	if (error) {
 		fprintf(stderr, _("%s: realtime size check failed\n"),
@@ -679,7 +679,7 @@ check_many_rtgroups(
 	xfs_daddr_t		d;
 	int			error;
 
-	if (!mp->m_rtdev->bt_bdev) {
+	if (!mp->m_rtdev_targp->bt_bdev) {
 		fprintf(stderr, _("%s: no rt device, ignoring rgcount %u\n"),
 				progname, sbp->sb_rgcount);
 		if (!xfs_is_debugger(mp))
@@ -690,8 +690,8 @@ check_many_rtgroups(
 	}
 
 	d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks);
-	error = libxfs_buf_read(mp->m_rtdev, d - XFS_FSB_TO_BB(mp, 1), 1, 0,
-			&bp, NULL);
+	error = libxfs_buf_read(mp->m_rtdev_targp, d - XFS_FSB_TO_BB(mp, 1), 1,
+			0, &bp, NULL);
 	if (!error) {
 		libxfs_buf_relse(bp);
 		return true;
@@ -806,7 +806,7 @@ libxfs_mount(
 		return mp;
 
 	/* device size checks must pass unless we're a debugger. */
-	error = libxfs_buf_read(mp->m_dev, d - XFS_FSS_TO_BB(mp, 1),
+	error = libxfs_buf_read(mp->m_ddev_targp, d - XFS_FSS_TO_BB(mp, 1),
 			XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
 	if (error) {
 		fprintf(stderr, _("%s: data size check failed\n"), progname);
@@ -848,7 +848,7 @@ libxfs_mount(
 	 * read the first one and let the user know to check the geometry.
 	 */
 	if (sbp->sb_agcount > 1000000) {
-		error = libxfs_buf_read(mp->m_dev,
+		error = libxfs_buf_read(mp->m_ddev_targp,
 				XFS_AG_DADDR(mp, sbp->sb_agcount - 1, 0), 1,
 				0, &bp, NULL);
 		if (error) {
diff --git a/libxfs/trans.c b/libxfs/trans.c
index c89b035ffeaf..88022c2fc9c2 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -500,7 +500,7 @@ libxfs_trans_getsb(
 	if (tp == NULL)
 		return libxfs_getsb(mp);
 
-	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
+	bp = xfs_trans_buf_item_match(tp, mp->m_ddev_targp, &map, 1);
 	if (bp != NULL) {
 		ASSERT(bp->b_transp == tp);
 		bip = bp->b_log_item;
@@ -529,7 +529,7 @@ libxfs_trans_getrtsb(
 	int			len = XFS_FSS_TO_BB(mp, 1);
 	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
 
-	bp = xfs_trans_buf_item_match(tp, mp->m_rtdev, &map, 1);
+	bp = xfs_trans_buf_item_match(tp, mp->m_rtdev_targp, &map, 1);
 	if (bp != NULL) {
 		ASSERT(bp->b_transp == tp);
 		bip = bp->b_log_item;
diff --git a/libxfs/util.c b/libxfs/util.c
index 6cbbe9056eba..fae8fce484b7 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -527,7 +527,7 @@ libxfs_file_write(
 		    map.br_state == XFS_EXT_UNWRITTEN)
 			return -EINVAL;
 
-		error = libxfs_buf_get(mp->m_dev,
+		error = libxfs_buf_get(mp->m_ddev_targp,
 				XFS_FSB_TO_DADDR(mp, map.br_startblock),
 				XFS_FSB_TO_BB(mp, map.br_blockcount),
 				&bp);
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 4f11c9de339c..714cd3cda8ca 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -5666,7 +5666,7 @@ rewrite_secondary_superblocks(
 	int			error;
 
 	/* rewrite the last superblock */
-	error = -libxfs_buf_read(mp->m_dev,
+	error = -libxfs_buf_read(mp->m_ddev_targp,
 			XFS_AGB_TO_DADDR(mp, mp->m_sb.sb_agcount - 1,
 				XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1), 0, &buf, &xfs_sb_buf_ops);
@@ -5686,7 +5686,7 @@ rewrite_secondary_superblocks(
 	if (mp->m_sb.sb_agcount <= 2)
 		return;
 
-	error = -libxfs_buf_read(mp->m_dev,
+	error = -libxfs_buf_read(mp->m_ddev_targp,
 			XFS_AGB_TO_DADDR(mp, (mp->m_sb.sb_agcount - 1) / 2,
 				XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1), 0, &buf, &xfs_sb_buf_ops);
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index fe4089026cae..982dc838ee55 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -443,9 +443,10 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
 			clearit = 1;
 			break;
 		}
-		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
-				XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE,
-				&bp, &xfs_attr3_rmt_buf_ops);
+		error = -libxfs_buf_read(mp->m_ddev_targp,
+				XFS_FSB_TO_DADDR(mp, bno), XFS_FSB_TO_BB(mp, 1),
+				LIBXFS_READBUF_SALVAGE, &bp,
+				&xfs_attr3_rmt_buf_ops);
 		if (error) {
 			do_warn(
 	_("can't read remote block for attributes of inode %" PRIu64 "\n"), ino);
@@ -879,7 +880,7 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 			goto error_out;
 		}
 
-		error = -libxfs_buf_read(mp->m_dev,
+		error = -libxfs_buf_read(mp->m_ddev_targp,
 				XFS_FSB_TO_DADDR(mp, dev_bno),
 				XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE,
 				&bp, &xfs_attr3_leaf_buf_ops);
@@ -1217,7 +1218,7 @@ process_longform_attr(
 		return 1;
 	}
 
-	error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
+	error = -libxfs_buf_read(mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, bno),
 			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
 			&xfs_da3_node_buf_ops);
 	if (error) {
diff --git a/repair/da_util.c b/repair/da_util.c
index 7f94f4012062..c0625b312fc6 100644
--- a/repair/da_util.c
+++ b/repair/da_util.c
@@ -64,7 +64,7 @@ da_read_buf(
 		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, bmp[i].startblock);
 		map[i].bm_len = XFS_FSB_TO_BB(mp, bmp[i].blockcount);
 	}
-	libxfs_buf_read_map(mp->m_dev, map, nex, LIBXFS_READBUF_SALVAGE,
+	libxfs_buf_read_map(mp->m_ddev_targp, map, nex, LIBXFS_READBUF_SALVAGE,
 			&bp, ops);
 	if (map != map_array)
 		free(map);
diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 932eaf63f474..6b261a1e99bf 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -43,9 +43,9 @@ check_aginode_block(
 	 * tree and we wouldn't be here and we stale the buffers out
 	 * so no one else will overlap them.
 	 */
-	error = -libxfs_buf_read(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, agbno),
-			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
-			NULL);
+	error = -libxfs_buf_read(mp->m_ddev_targp,
+			XFS_AGB_TO_DADDR(mp, agno, agbno), XFS_FSB_TO_BB(mp, 1),
+			LIBXFS_READBUF_SALVAGE, &bp, NULL);
 	if (error) {
 		do_warn(_("cannot read agbno (%u/%u), disk block %" PRId64 "\n"),
 			agno, agbno, XFS_AGB_TO_DADDR(mp, agno, agbno));
@@ -699,7 +699,7 @@ process_inode_chunk(
 		pftrace("about to read off %llu in AG %d",
 			XFS_AGB_TO_DADDR(mp, agno, agbno), agno);
 
-		error = -libxfs_buf_read(mp->m_dev,
+		error = -libxfs_buf_read(mp->m_ddev_targp,
 				XFS_AGB_TO_DADDR(mp, agno, agbno),
 				XFS_FSB_TO_BB(mp,
 					M_IGEO(mp)->blocks_per_cluster),
diff --git a/repair/dinode.c b/repair/dinode.c
index 48939f8bd159..243fcf7a19f4 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -956,8 +956,8 @@ get_agino_buf(
 		cluster_agino, cluster_daddr, cluster_blks);
 #endif
 
-	error = -libxfs_buf_read(mp->m_dev, cluster_daddr, cluster_blks, 0,
-			&bp, &xfs_inode_buf_ops);
+	error = -libxfs_buf_read(mp->m_ddev_targp, cluster_daddr, cluster_blks,
+			0, &bp, &xfs_inode_buf_ops);
 	if (error) {
 		do_warn(_("cannot read inode (%u/%u), disk block %" PRIu64 "\n"),
 			agno, cluster_agino, cluster_daddr);
@@ -1733,7 +1733,7 @@ process_quota_inode(
 		fsbno = blkmap_get(blkmap, qbno);
 		dqid = (xfs_dqid_t)qbno * dqperchunk;
 
-		error = -libxfs_buf_read(mp->m_dev,
+		error = -libxfs_buf_read(mp->m_ddev_targp,
 				XFS_FSB_TO_DADDR(mp, fsbno), dqchunklen,
 				LIBXFS_READBUF_SALVAGE, &bp,
 				&xfs_dquot_buf_ops);
@@ -1845,7 +1845,7 @@ _("cannot read inode %" PRIu64 ", file block %d, NULL disk block\n"),
 
 		byte_cnt = XFS_FSB_TO_B(mp, blk_cnt);
 
-		error = -libxfs_buf_read(mp->m_dev,
+		error = -libxfs_buf_read(mp->m_ddev_targp,
 				XFS_FSB_TO_DADDR(mp, fsbno), BTOBB(byte_cnt),
 				LIBXFS_READBUF_SALVAGE, &bp,
 				&xfs_symlink_buf_ops);
diff --git a/repair/phase3.c b/repair/phase3.c
index 6ec616d9b31d..64a2c961b080 100644
--- a/repair/phase3.c
+++ b/repair/phase3.c
@@ -30,7 +30,7 @@ process_agi_unlinked(
 	int			agi_dirty = 0;
 	int			error;
 
-	error = -libxfs_buf_read(mp->m_dev,
+	error = -libxfs_buf_read(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
 			mp->m_sb.sb_sectsize / BBSIZE, LIBXFS_READBUF_SALVAGE,
 			&bp, &xfs_agi_buf_ops);
diff --git a/repair/phase5.c b/repair/phase5.c
index e44c26885717..019772346eb1 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -135,7 +135,7 @@ build_agi(
 	int			i;
 	int			error;
 
-	error = -libxfs_buf_get(mp->m_dev,
+	error = -libxfs_buf_get(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
 			mp->m_sb.sb_sectsize / BBSIZE, &agi_buf);
 	if (error)
@@ -228,7 +228,7 @@ build_agf_agfl(
 	__be32			*freelist;
 	int			error;
 
-	error = -libxfs_buf_get(mp->m_dev,
+	error = -libxfs_buf_get(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
 			mp->m_sb.sb_sectsize / BBSIZE, &agf_buf);
 	if (error)
@@ -314,7 +314,7 @@ build_agf_agfl(
 		platform_uuid_copy(&agf->agf_uuid, &mp->m_sb.sb_meta_uuid);
 
 	/* initialise the AGFL, then fill it if there are blocks left over. */
-	error = -libxfs_buf_get(mp->m_dev,
+	error = -libxfs_buf_get(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
 			mp->m_sb.sb_sectsize / BBSIZE, &agfl_buf);
 	if (error)
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 8cd3416fa568..3d26636e5e56 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -121,7 +121,7 @@ pf_queue_io(
 	 * the lock holder is either reading it from disk himself or
 	 * completely overwriting it this behaviour is perfectly fine.
 	 */
-	error = -libxfs_buf_get_map(mp->m_dev, map, nmaps,
+	error = -libxfs_buf_get_map(mp->m_ddev_targp, map, nmaps,
 			LIBXFS_GETBUF_TRYLOCK, &bp);
 	if (error)
 		return;
@@ -275,7 +275,7 @@ pf_scan_lbtree(
 	int			rc;
 	int			error;
 
-	error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, dbno),
+	error = -libxfs_buf_read(mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, dbno),
 			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
 			&xfs_bmbt_buf_ops);
 	if (error)
diff --git a/repair/quotacheck.c b/repair/quotacheck.c
index fc7e3864654c..e13092dfcae8 100644
--- a/repair/quotacheck.c
+++ b/repair/quotacheck.c
@@ -369,7 +369,7 @@ qc_walk_dquot_extent(
 		unsigned int	dqnr;
 		uint64_t	dqid;
 
-		error = -libxfs_buf_read(mp->m_dev,
+		error = -libxfs_buf_read(mp->m_ddev_targp,
 				XFS_FSB_TO_DADDR(mp, map->br_startblock + bno),
 				dqchunklen, 0, &bp, &xfs_dquot_buf_ops);
 		if (error) {
diff --git a/repair/rt.c b/repair/rt.c
index 3e51c9b5eb4b..b5b9d4fdc536 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -255,7 +255,7 @@ check_rtfile_contents(
 			break;
 		}
 
-		error = -libxfs_buf_read_uncached(mp->m_dev,
+		error = -libxfs_buf_read_uncached(mp->m_ddev_targp,
 				XFS_FSB_TO_DADDR(mp, map.br_startblock),
 				XFS_FSB_TO_BB(mp, 1), 0, &bp,
 				xfs_rtblock_ops(mp, type));
diff --git a/repair/scan.c b/repair/scan.c
index 7d22ff378484..865983d6c9e0 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -102,8 +102,9 @@ scan_sbtree(
 	struct xfs_buf	*bp;
 	int		error;
 
-	error = salvage_buffer(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, root),
-			XFS_FSB_TO_BB(mp, 1), &bp, ops);
+	error = salvage_buffer(mp->m_ddev_targp,
+			XFS_AGB_TO_DADDR(mp, agno, root),XFS_FSB_TO_BB(mp, 1),
+			&bp, ops);
 	if (error) {
 		do_error(_("can't read btree block %d/%d\n"), agno, root);
 		return;
@@ -161,7 +162,7 @@ scan_lbtree(
 	int		dirty = 0;
 	bool		badcrc = false;
 
-	err = salvage_buffer(mp->m_dev, XFS_FSB_TO_DADDR(mp, root),
+	err = salvage_buffer(mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, root),
 			XFS_FSB_TO_BB(mp, 1), &bp, ops);
 	if (err) {
 		do_error(_("can't read btree block %d/%d\n"),
@@ -3030,7 +3031,7 @@ scan_freelist(
 	if (be32_to_cpu(agf->agf_flcount) == 0)
 		return;
 
-	error = salvage_buffer(mp->m_dev,
+	error = salvage_buffer(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
 			XFS_FSS_TO_BB(mp, 1), &agflbuf, &xfs_agfl_buf_ops);
 	if (error) {
@@ -3312,7 +3313,8 @@ scan_ag(
 		return;
 	}
 
-	error = salvage_buffer(mp->m_dev, XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
+	error = salvage_buffer(mp->m_ddev_targp,
+			XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1), &sbbuf, &xfs_sb_buf_ops);
 	if (error) {
 		objname = _("root superblock");
@@ -3322,7 +3324,7 @@ scan_ag(
 		do_warn(_("superblock has bad CRC for ag %d\n"), agno);
 	libxfs_sb_from_disk(sb, sbbuf->b_addr);
 
-	error = salvage_buffer(mp->m_dev,
+	error = salvage_buffer(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
 			XFS_FSS_TO_BB(mp, 1), &agfbuf, &xfs_agf_buf_ops);
 	if (error) {
@@ -3333,7 +3335,7 @@ scan_ag(
 		do_warn(_("agf has bad CRC for ag %d\n"), agno);
 	agf = agfbuf->b_addr;
 
-	error = salvage_buffer(mp->m_dev,
+	error = salvage_buffer(mp->m_ddev_targp,
 			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
 			XFS_FSS_TO_BB(mp, 1), &agibuf, &xfs_agi_buf_ops);
 	if (error) {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] libxfs: remove buftarg member aliases
  2026-09-11 14:45 ` [PATCH 3/3] libxfs: remove buftarg member aliases Christoph Hellwig
@ 2026-09-11 14:53   ` Darrick J. Wong
  0 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2026-09-11 14:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs

On Fri, Sep 11, 2026 at 04:45:44PM +0200, Christoph Hellwig wrote:
> Remove the #defines giving historic IRIX names to the ddev/logdev/rtdev
> buftargs in struct xfs_mount and use the current kernel names everywhere.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

<shudder> macros goway :)

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

--D

> ---
>  include/xfs_mount.h  |  3 ---
>  libxfs/init.c        | 12 ++++++------
>  libxfs/trans.c       |  4 ++--
>  libxfs/util.c        |  2 +-
>  mkfs/xfs_mkfs.c      |  4 ++--
>  repair/attr_repair.c | 11 ++++++-----
>  repair/da_util.c     |  2 +-
>  repair/dino_chunks.c |  8 ++++----
>  repair/dinode.c      |  8 ++++----
>  repair/phase3.c      |  2 +-
>  repair/phase5.c      |  6 +++---
>  repair/prefetch.c    |  4 ++--
>  repair/quotacheck.c  |  2 +-
>  repair/rt.c          |  2 +-
>  repair/scan.c        | 16 +++++++++-------
>  15 files changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/include/xfs_mount.h b/include/xfs_mount.h
> index 5a714333c16e..0dea578ee3a5 100644
> --- a/include/xfs_mount.h
> +++ b/include/xfs_mount.h
> @@ -102,9 +102,6 @@ typedef struct xfs_mount {
>  	struct xfs_buftarg	*m_ddev_targp;
>  	struct xfs_buftarg	*m_logdev_targp;
>  	struct xfs_buftarg	*m_rtdev_targp;
> -#define m_dev		m_ddev_targp
> -#define m_logdev	m_logdev_targp
> -#define m_rtdev		m_rtdev_targp
>  	uint8_t			m_dircook_elog;	/* log d-cookie entry bits */
>  	uint8_t			m_blkbit_log;	/* blocklog + NBBY */
>  	uint8_t			m_blkbb_log;	/* blocklog - BBSHIFT */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index a47e610934e9..97c4755dd131 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -331,7 +331,7 @@ rtmount_init(
>  			(unsigned long long) mp->m_sb.sb_rblocks);
>  		return -1;
>  	}
> -	error = libxfs_buf_read(mp->m_rtdev, d - XFS_FSB_TO_BB(mp, 1),
> +	error = libxfs_buf_read(mp->m_rtdev_targp, d - XFS_FSB_TO_BB(mp, 1),
>  			XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
>  	if (error) {
>  		fprintf(stderr, _("%s: realtime size check failed\n"),
> @@ -679,7 +679,7 @@ check_many_rtgroups(
>  	xfs_daddr_t		d;
>  	int			error;
>  
> -	if (!mp->m_rtdev->bt_bdev) {
> +	if (!mp->m_rtdev_targp->bt_bdev) {
>  		fprintf(stderr, _("%s: no rt device, ignoring rgcount %u\n"),
>  				progname, sbp->sb_rgcount);
>  		if (!xfs_is_debugger(mp))
> @@ -690,8 +690,8 @@ check_many_rtgroups(
>  	}
>  
>  	d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_rblocks);
> -	error = libxfs_buf_read(mp->m_rtdev, d - XFS_FSB_TO_BB(mp, 1), 1, 0,
> -			&bp, NULL);
> +	error = libxfs_buf_read(mp->m_rtdev_targp, d - XFS_FSB_TO_BB(mp, 1), 1,
> +			0, &bp, NULL);
>  	if (!error) {
>  		libxfs_buf_relse(bp);
>  		return true;
> @@ -806,7 +806,7 @@ libxfs_mount(
>  		return mp;
>  
>  	/* device size checks must pass unless we're a debugger. */
> -	error = libxfs_buf_read(mp->m_dev, d - XFS_FSS_TO_BB(mp, 1),
> +	error = libxfs_buf_read(mp->m_ddev_targp, d - XFS_FSS_TO_BB(mp, 1),
>  			XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
>  	if (error) {
>  		fprintf(stderr, _("%s: data size check failed\n"), progname);
> @@ -848,7 +848,7 @@ libxfs_mount(
>  	 * read the first one and let the user know to check the geometry.
>  	 */
>  	if (sbp->sb_agcount > 1000000) {
> -		error = libxfs_buf_read(mp->m_dev,
> +		error = libxfs_buf_read(mp->m_ddev_targp,
>  				XFS_AG_DADDR(mp, sbp->sb_agcount - 1, 0), 1,
>  				0, &bp, NULL);
>  		if (error) {
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index c89b035ffeaf..88022c2fc9c2 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -500,7 +500,7 @@ libxfs_trans_getsb(
>  	if (tp == NULL)
>  		return libxfs_getsb(mp);
>  
> -	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
> +	bp = xfs_trans_buf_item_match(tp, mp->m_ddev_targp, &map, 1);
>  	if (bp != NULL) {
>  		ASSERT(bp->b_transp == tp);
>  		bip = bp->b_log_item;
> @@ -529,7 +529,7 @@ libxfs_trans_getrtsb(
>  	int			len = XFS_FSS_TO_BB(mp, 1);
>  	DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
>  
> -	bp = xfs_trans_buf_item_match(tp, mp->m_rtdev, &map, 1);
> +	bp = xfs_trans_buf_item_match(tp, mp->m_rtdev_targp, &map, 1);
>  	if (bp != NULL) {
>  		ASSERT(bp->b_transp == tp);
>  		bip = bp->b_log_item;
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 6cbbe9056eba..fae8fce484b7 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -527,7 +527,7 @@ libxfs_file_write(
>  		    map.br_state == XFS_EXT_UNWRITTEN)
>  			return -EINVAL;
>  
> -		error = libxfs_buf_get(mp->m_dev,
> +		error = libxfs_buf_get(mp->m_ddev_targp,
>  				XFS_FSB_TO_DADDR(mp, map.br_startblock),
>  				XFS_FSB_TO_BB(mp, map.br_blockcount),
>  				&bp);
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4f11c9de339c..714cd3cda8ca 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -5666,7 +5666,7 @@ rewrite_secondary_superblocks(
>  	int			error;
>  
>  	/* rewrite the last superblock */
> -	error = -libxfs_buf_read(mp->m_dev,
> +	error = -libxfs_buf_read(mp->m_ddev_targp,
>  			XFS_AGB_TO_DADDR(mp, mp->m_sb.sb_agcount - 1,
>  				XFS_SB_DADDR),
>  			XFS_FSS_TO_BB(mp, 1), 0, &buf, &xfs_sb_buf_ops);
> @@ -5686,7 +5686,7 @@ rewrite_secondary_superblocks(
>  	if (mp->m_sb.sb_agcount <= 2)
>  		return;
>  
> -	error = -libxfs_buf_read(mp->m_dev,
> +	error = -libxfs_buf_read(mp->m_ddev_targp,
>  			XFS_AGB_TO_DADDR(mp, (mp->m_sb.sb_agcount - 1) / 2,
>  				XFS_SB_DADDR),
>  			XFS_FSS_TO_BB(mp, 1), 0, &buf, &xfs_sb_buf_ops);
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index fe4089026cae..982dc838ee55 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -443,9 +443,10 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
>  			clearit = 1;
>  			break;
>  		}
> -		error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
> -				XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE,
> -				&bp, &xfs_attr3_rmt_buf_ops);
> +		error = -libxfs_buf_read(mp->m_ddev_targp,
> +				XFS_FSB_TO_DADDR(mp, bno), XFS_FSB_TO_BB(mp, 1),
> +				LIBXFS_READBUF_SALVAGE, &bp,
> +				&xfs_attr3_rmt_buf_ops);
>  		if (error) {
>  			do_warn(
>  	_("can't read remote block for attributes of inode %" PRIu64 "\n"), ino);
> @@ -879,7 +880,7 @@ process_leaf_attr_level(xfs_mount_t	*mp,
>  			goto error_out;
>  		}
>  
> -		error = -libxfs_buf_read(mp->m_dev,
> +		error = -libxfs_buf_read(mp->m_ddev_targp,
>  				XFS_FSB_TO_DADDR(mp, dev_bno),
>  				XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE,
>  				&bp, &xfs_attr3_leaf_buf_ops);
> @@ -1217,7 +1218,7 @@ process_longform_attr(
>  		return 1;
>  	}
>  
> -	error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, bno),
> +	error = -libxfs_buf_read(mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, bno),
>  			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
>  			&xfs_da3_node_buf_ops);
>  	if (error) {
> diff --git a/repair/da_util.c b/repair/da_util.c
> index 7f94f4012062..c0625b312fc6 100644
> --- a/repair/da_util.c
> +++ b/repair/da_util.c
> @@ -64,7 +64,7 @@ da_read_buf(
>  		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, bmp[i].startblock);
>  		map[i].bm_len = XFS_FSB_TO_BB(mp, bmp[i].blockcount);
>  	}
> -	libxfs_buf_read_map(mp->m_dev, map, nex, LIBXFS_READBUF_SALVAGE,
> +	libxfs_buf_read_map(mp->m_ddev_targp, map, nex, LIBXFS_READBUF_SALVAGE,
>  			&bp, ops);
>  	if (map != map_array)
>  		free(map);
> diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
> index 932eaf63f474..6b261a1e99bf 100644
> --- a/repair/dino_chunks.c
> +++ b/repair/dino_chunks.c
> @@ -43,9 +43,9 @@ check_aginode_block(
>  	 * tree and we wouldn't be here and we stale the buffers out
>  	 * so no one else will overlap them.
>  	 */
> -	error = -libxfs_buf_read(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, agbno),
> -			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
> -			NULL);
> +	error = -libxfs_buf_read(mp->m_ddev_targp,
> +			XFS_AGB_TO_DADDR(mp, agno, agbno), XFS_FSB_TO_BB(mp, 1),
> +			LIBXFS_READBUF_SALVAGE, &bp, NULL);
>  	if (error) {
>  		do_warn(_("cannot read agbno (%u/%u), disk block %" PRId64 "\n"),
>  			agno, agbno, XFS_AGB_TO_DADDR(mp, agno, agbno));
> @@ -699,7 +699,7 @@ process_inode_chunk(
>  		pftrace("about to read off %llu in AG %d",
>  			XFS_AGB_TO_DADDR(mp, agno, agbno), agno);
>  
> -		error = -libxfs_buf_read(mp->m_dev,
> +		error = -libxfs_buf_read(mp->m_ddev_targp,
>  				XFS_AGB_TO_DADDR(mp, agno, agbno),
>  				XFS_FSB_TO_BB(mp,
>  					M_IGEO(mp)->blocks_per_cluster),
> diff --git a/repair/dinode.c b/repair/dinode.c
> index 48939f8bd159..243fcf7a19f4 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -956,8 +956,8 @@ get_agino_buf(
>  		cluster_agino, cluster_daddr, cluster_blks);
>  #endif
>  
> -	error = -libxfs_buf_read(mp->m_dev, cluster_daddr, cluster_blks, 0,
> -			&bp, &xfs_inode_buf_ops);
> +	error = -libxfs_buf_read(mp->m_ddev_targp, cluster_daddr, cluster_blks,
> +			0, &bp, &xfs_inode_buf_ops);
>  	if (error) {
>  		do_warn(_("cannot read inode (%u/%u), disk block %" PRIu64 "\n"),
>  			agno, cluster_agino, cluster_daddr);
> @@ -1733,7 +1733,7 @@ process_quota_inode(
>  		fsbno = blkmap_get(blkmap, qbno);
>  		dqid = (xfs_dqid_t)qbno * dqperchunk;
>  
> -		error = -libxfs_buf_read(mp->m_dev,
> +		error = -libxfs_buf_read(mp->m_ddev_targp,
>  				XFS_FSB_TO_DADDR(mp, fsbno), dqchunklen,
>  				LIBXFS_READBUF_SALVAGE, &bp,
>  				&xfs_dquot_buf_ops);
> @@ -1845,7 +1845,7 @@ _("cannot read inode %" PRIu64 ", file block %d, NULL disk block\n"),
>  
>  		byte_cnt = XFS_FSB_TO_B(mp, blk_cnt);
>  
> -		error = -libxfs_buf_read(mp->m_dev,
> +		error = -libxfs_buf_read(mp->m_ddev_targp,
>  				XFS_FSB_TO_DADDR(mp, fsbno), BTOBB(byte_cnt),
>  				LIBXFS_READBUF_SALVAGE, &bp,
>  				&xfs_symlink_buf_ops);
> diff --git a/repair/phase3.c b/repair/phase3.c
> index 6ec616d9b31d..64a2c961b080 100644
> --- a/repair/phase3.c
> +++ b/repair/phase3.c
> @@ -30,7 +30,7 @@ process_agi_unlinked(
>  	int			agi_dirty = 0;
>  	int			error;
>  
> -	error = -libxfs_buf_read(mp->m_dev,
> +	error = -libxfs_buf_read(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
>  			mp->m_sb.sb_sectsize / BBSIZE, LIBXFS_READBUF_SALVAGE,
>  			&bp, &xfs_agi_buf_ops);
> diff --git a/repair/phase5.c b/repair/phase5.c
> index e44c26885717..019772346eb1 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -135,7 +135,7 @@ build_agi(
>  	int			i;
>  	int			error;
>  
> -	error = -libxfs_buf_get(mp->m_dev,
> +	error = -libxfs_buf_get(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
>  			mp->m_sb.sb_sectsize / BBSIZE, &agi_buf);
>  	if (error)
> @@ -228,7 +228,7 @@ build_agf_agfl(
>  	__be32			*freelist;
>  	int			error;
>  
> -	error = -libxfs_buf_get(mp->m_dev,
> +	error = -libxfs_buf_get(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
>  			mp->m_sb.sb_sectsize / BBSIZE, &agf_buf);
>  	if (error)
> @@ -314,7 +314,7 @@ build_agf_agfl(
>  		platform_uuid_copy(&agf->agf_uuid, &mp->m_sb.sb_meta_uuid);
>  
>  	/* initialise the AGFL, then fill it if there are blocks left over. */
> -	error = -libxfs_buf_get(mp->m_dev,
> +	error = -libxfs_buf_get(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
>  			mp->m_sb.sb_sectsize / BBSIZE, &agfl_buf);
>  	if (error)
> diff --git a/repair/prefetch.c b/repair/prefetch.c
> index 8cd3416fa568..3d26636e5e56 100644
> --- a/repair/prefetch.c
> +++ b/repair/prefetch.c
> @@ -121,7 +121,7 @@ pf_queue_io(
>  	 * the lock holder is either reading it from disk himself or
>  	 * completely overwriting it this behaviour is perfectly fine.
>  	 */
> -	error = -libxfs_buf_get_map(mp->m_dev, map, nmaps,
> +	error = -libxfs_buf_get_map(mp->m_ddev_targp, map, nmaps,
>  			LIBXFS_GETBUF_TRYLOCK, &bp);
>  	if (error)
>  		return;
> @@ -275,7 +275,7 @@ pf_scan_lbtree(
>  	int			rc;
>  	int			error;
>  
> -	error = -libxfs_buf_read(mp->m_dev, XFS_FSB_TO_DADDR(mp, dbno),
> +	error = -libxfs_buf_read(mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, dbno),
>  			XFS_FSB_TO_BB(mp, 1), LIBXFS_READBUF_SALVAGE, &bp,
>  			&xfs_bmbt_buf_ops);
>  	if (error)
> diff --git a/repair/quotacheck.c b/repair/quotacheck.c
> index fc7e3864654c..e13092dfcae8 100644
> --- a/repair/quotacheck.c
> +++ b/repair/quotacheck.c
> @@ -369,7 +369,7 @@ qc_walk_dquot_extent(
>  		unsigned int	dqnr;
>  		uint64_t	dqid;
>  
> -		error = -libxfs_buf_read(mp->m_dev,
> +		error = -libxfs_buf_read(mp->m_ddev_targp,
>  				XFS_FSB_TO_DADDR(mp, map->br_startblock + bno),
>  				dqchunklen, 0, &bp, &xfs_dquot_buf_ops);
>  		if (error) {
> diff --git a/repair/rt.c b/repair/rt.c
> index 3e51c9b5eb4b..b5b9d4fdc536 100644
> --- a/repair/rt.c
> +++ b/repair/rt.c
> @@ -255,7 +255,7 @@ check_rtfile_contents(
>  			break;
>  		}
>  
> -		error = -libxfs_buf_read_uncached(mp->m_dev,
> +		error = -libxfs_buf_read_uncached(mp->m_ddev_targp,
>  				XFS_FSB_TO_DADDR(mp, map.br_startblock),
>  				XFS_FSB_TO_BB(mp, 1), 0, &bp,
>  				xfs_rtblock_ops(mp, type));
> diff --git a/repair/scan.c b/repair/scan.c
> index 7d22ff378484..865983d6c9e0 100644
> --- a/repair/scan.c
> +++ b/repair/scan.c
> @@ -102,8 +102,9 @@ scan_sbtree(
>  	struct xfs_buf	*bp;
>  	int		error;
>  
> -	error = salvage_buffer(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno, root),
> -			XFS_FSB_TO_BB(mp, 1), &bp, ops);
> +	error = salvage_buffer(mp->m_ddev_targp,
> +			XFS_AGB_TO_DADDR(mp, agno, root),XFS_FSB_TO_BB(mp, 1),
> +			&bp, ops);
>  	if (error) {
>  		do_error(_("can't read btree block %d/%d\n"), agno, root);
>  		return;
> @@ -161,7 +162,7 @@ scan_lbtree(
>  	int		dirty = 0;
>  	bool		badcrc = false;
>  
> -	err = salvage_buffer(mp->m_dev, XFS_FSB_TO_DADDR(mp, root),
> +	err = salvage_buffer(mp->m_ddev_targp, XFS_FSB_TO_DADDR(mp, root),
>  			XFS_FSB_TO_BB(mp, 1), &bp, ops);
>  	if (err) {
>  		do_error(_("can't read btree block %d/%d\n"),
> @@ -3030,7 +3031,7 @@ scan_freelist(
>  	if (be32_to_cpu(agf->agf_flcount) == 0)
>  		return;
>  
> -	error = salvage_buffer(mp->m_dev,
> +	error = salvage_buffer(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGFL_DADDR(mp)),
>  			XFS_FSS_TO_BB(mp, 1), &agflbuf, &xfs_agfl_buf_ops);
>  	if (error) {
> @@ -3312,7 +3313,8 @@ scan_ag(
>  		return;
>  	}
>  
> -	error = salvage_buffer(mp->m_dev, XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
> +	error = salvage_buffer(mp->m_ddev_targp,
> +			XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
>  			XFS_FSS_TO_BB(mp, 1), &sbbuf, &xfs_sb_buf_ops);
>  	if (error) {
>  		objname = _("root superblock");
> @@ -3322,7 +3324,7 @@ scan_ag(
>  		do_warn(_("superblock has bad CRC for ag %d\n"), agno);
>  	libxfs_sb_from_disk(sb, sbbuf->b_addr);
>  
> -	error = salvage_buffer(mp->m_dev,
> +	error = salvage_buffer(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGF_DADDR(mp)),
>  			XFS_FSS_TO_BB(mp, 1), &agfbuf, &xfs_agf_buf_ops);
>  	if (error) {
> @@ -3333,7 +3335,7 @@ scan_ag(
>  		do_warn(_("agf has bad CRC for ag %d\n"), agno);
>  	agf = agfbuf->b_addr;
>  
> -	error = salvage_buffer(mp->m_dev,
> +	error = salvage_buffer(mp->m_ddev_targp,
>  			XFS_AG_DADDR(mp, agno, XFS_AGI_DADDR(mp)),
>  			XFS_FSS_TO_BB(mp, 1), &agibuf, &xfs_agi_buf_ops);
>  	if (error) {
> -- 
> 2.53.0
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] libxfs: better describe why m_ddev_targp can be set in libxfs_buftarg_init
  2026-09-11 14:45 ` [PATCH 2/3] libxfs: better describe why m_ddev_targp can be set in libxfs_buftarg_init Christoph Hellwig
@ 2026-09-11 14:53   ` Darrick J. Wong
  0 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2026-09-11 14:53 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs

On Fri, Sep 11, 2026 at 04:45:43PM +0200, Christoph Hellwig wrote:
> I had to wade through this to understand what is going on here, so dump
> the thoughts in a comment to make it easier for the next person to
> understand the logic.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  libxfs/init.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 2a46ddf086ed..a47e610934e9 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -534,7 +534,12 @@ libxfs_buftarg_init(
>  	}
>  
>  	if (mp->m_ddev_targp) {
> -		/* should already have all buftargs initialised */
> +		/*
> +		 * This can happen if the utility called libxfs_buftarg_init
> +		 * manually before libxfs_mount, which calls us again.

That's much clearer about how we get into this state. :)

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

--D

> +		 *
> +		 * In this case all buftargs should be initialized already.
> +		 */
>  		if (mp->m_ddev_targp->bt_bdev != xi->data.dev ||
>  		    mp->m_ddev_targp->bt_mount != mp) {
>  			fprintf(stderr,
> -- 
> 2.53.0
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided
  2026-09-11 14:45 ` [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided Christoph Hellwig
@ 2026-09-11 14:57   ` Darrick J. Wong
  2026-09-11 15:11     ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2026-09-11 14:57 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs

On Fri, Sep 11, 2026 at 04:45:42PM +0200, Christoph Hellwig wrote:
> The user-space only parts of commit 9840f7e09e2f ("xfs: allow internal
> RT devices for zoned mode") accidentally set m_rtdev_targp to
> m_ddev_targp when not name is set for the RT device, and thus disable
> the check for a non-NULL m_rtdev_targp in rtmount_init.
> 
> This lead to tools working without specifying a RT device when they
> should abort.  For example this can lead to repair trying to read
> and rewrite the rtsb when called without -rc, which will then fail
> in weird ways.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  libxfs/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 5d8b4a153e28..2a46ddf086ed 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -573,7 +573,7 @@ libxfs_buftarg_init(
>  	else
>  		mp->m_logdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->log,
>  				lfail);
> -	if (!xi->rt.dev || xi->rt.dev == xi->data.dev)
> +	if (xi->rt.dev == xi->data.dev)
>  		mp->m_rtdev_targp = mp->m_ddev_targp;
>  	else
>  		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,

Does this mean that m_rtdev_targp is never NULL, even when we don't
specify or have a rt device attached?  I would have expected this to be:

	if (!xi->rt.dev)
		mp->m_rtdev_targp = NULL;
	else if (xi->rt.dev == xi->data.dev)
		mp->m_rtdev_targp = mp->m_ddev_targp;
	else
		mp->m_rtdev_targp = libxfs_buftarg_alloc(...);

--D

> -- 
> 2.53.0
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided
  2026-09-11 14:57   ` Darrick J. Wong
@ 2026-09-11 15:11     ` Christoph Hellwig
  2026-09-11 15:30       ` Darrick J. Wong
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2026-09-11 15:11 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, Andrey Albershteyn, linux-xfs

On Fri, Sep 11, 2026 at 07:57:26AM -0700, Darrick J. Wong wrote:
> > -	if (!xi->rt.dev || xi->rt.dev == xi->data.dev)
> > +	if (xi->rt.dev == xi->data.dev)
> >  		mp->m_rtdev_targp = mp->m_ddev_targp;
> >  	else
> >  		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,
> 
> Does this mean that m_rtdev_targp is never NULL, even when we don't
> specify or have a rt device attached?

I think that is the case right now.

> I would have expected this to be:
> 
> 	if (!xi->rt.dev)
> 		mp->m_rtdev_targp = NULL;
> 	else if (xi->rt.dev == xi->data.dev)
> 		mp->m_rtdev_targp = mp->m_ddev_targp;
> 	else
> 		mp->m_rtdev_targp = libxfs_buftarg_alloc(...);

And that is what we get with this patch.  The zeroing is done by
zeroing the entire mount structure in the callers of libxfs_mount
(don't ask me why..).


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided
  2026-09-11 15:11     ` Christoph Hellwig
@ 2026-09-11 15:30       ` Darrick J. Wong
  2026-09-11 15:38         ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2026-09-11 15:30 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Andrey Albershteyn, linux-xfs

On Fri, Sep 11, 2026 at 05:11:20PM +0200, Christoph Hellwig wrote:
> On Fri, Sep 11, 2026 at 07:57:26AM -0700, Darrick J. Wong wrote:
> > > -	if (!xi->rt.dev || xi->rt.dev == xi->data.dev)
> > > +	if (xi->rt.dev == xi->data.dev)
> > >  		mp->m_rtdev_targp = mp->m_ddev_targp;
> > >  	else
> > >  		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,
> > 
> > Does this mean that m_rtdev_targp is never NULL, even when we don't
> > specify or have a rt device attached?
> 
> I think that is the case right now.
> 
> > I would have expected this to be:
> > 
> > 	if (!xi->rt.dev)
> > 		mp->m_rtdev_targp = NULL;
> > 	else if (xi->rt.dev == xi->data.dev)
> > 		mp->m_rtdev_targp = mp->m_ddev_targp;
> > 	else
> > 		mp->m_rtdev_targp = libxfs_buftarg_alloc(...);
> 
> And that is what we get with this patch.  The zeroing is done by
> zeroing the entire mount structure in the callers of libxfs_mount
> (don't ask me why..).

Sorry, but I don't see how we get to m_rtdev_targp==NULL with this
patch?  The end of libxfs_buftarg_init becomes:


	mp->m_ddev_targp = libxfs_buftarg_alloc(mp, xi, &xi->data, dfail);
	if (!xi->log.dev || xi->log.dev == xi->data.dev)
		mp->m_logdev_targp = mp->m_ddev_targp;
	else
		mp->m_logdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->log,
				lfail);
	if (xi->rt.dev == xi->data.dev)
		mp->m_rtdev_targp = mp->m_ddev_targp;
	else
		mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, xi, &xi->rt,
				rfail);

In the case where the rt and data devices are not the same, we create a
new buftarg object, even if xi->rt.dev == 0.  That only happens if we
didn't set xi->rt.name to a path, which means that libxfs_device_open
ignores it:

# truncate -s 3g /tmp/a
# mkfs.xfs -f /tmp/a
# gdb --args ./build-x86_64/db/xfs_db /tmp/a
(gdb) p *mp->m_rtdev_targp
$5 = {
  bt_mount = 0x555555659660 <xmount>,
  lock = {
    __data = {
      __lock = 0,
      __count = 0,
      __owner = 0,
      __nusers = 0,
      __kind = 0,
      __spins = 0,
      __elision = 0,
      __list = {
        __prev = 0x0,
        __next = 0x0
      }
    },
    __size = '\000' <repeats 39 times>,
    __align = 0
  },
  writes_left = 0,
  bt_bdev = 0,
  bt_bdev_fd = -1,
  bt_xfile = 0x0,
  flags = 0,
  bcache = 0x555555684ab0
}

Here we didn't supply an -R option to xfs_db, but libxfs assigns
m_rtdev_targp to a buftarg that can't do anything useful, instead of
NULL which (AFAICT) the code expects:

$ git grep m_rtdev_targp
mkfs/xfs_mkfs.c:5635:   if (mp->m_rtdev_targp->bt_bdev &&
mkfs/xfs_mkfs.c:5636:       mp->m_rtdev_targp != mp->m_ddev_targp &&
<snip>
libxfs/init.c:981:      if (mp->m_rtdev_targp && mp->m_rtdev_targp != mp->m_ddev_targp) {
libxfs/init.c:982:              err2 = libxfs_flush_buftarg(mp->m_rtdev_targp,

OTOH there are other places that use other checks that clearly assume
that m_rtdev_targp is always present:

libxfs/init.c:311:      if (mp->m_rtdev_targp->bt_bdev == 0 && !xfs_is_debugger(mp)) {
libxfs/init.c:1034:     if (mp->m_rtdev_targp != mp->m_ddev_targp)
libxfs/init.c:1035:             libxfs_buftarg_free(mp->m_rtdev_targp);

Ok maybe this patch is fine as-is?  But xfsprogs needs a treewide change
to make the "do we have a rt device attached?" checks match the kernel
code?

<even more confused>

--D

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided
  2026-09-11 15:30       ` Darrick J. Wong
@ 2026-09-11 15:38         ` Christoph Hellwig
  0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2026-09-11 15:38 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, Andrey Albershteyn, linux-xfs

On Fri, Sep 11, 2026 at 08:30:26AM -0700, Darrick J. Wong wrote:
> > And that is what we get with this patch.  The zeroing is done by
> > zeroing the entire mount structure in the callers of libxfs_mount
> > (don't ask me why..).
> 
> Sorry, but I don't see how we get to m_rtdev_targp==NULL with this
> patch?  The end of libxfs_buftarg_init becomes:

...

> In the case where the rt and data devices are not the same, we create a
> new buftarg object, even if xi->rt.dev == 0.  That only happens if we
> didn't set xi->rt.name to a path, which means that libxfs_device_open
> ignores it:

Yeah, you're right.  We'll have a separate m_rtdev_targp, but it
will have a NULL dev.  rtmount_init rejects this except for xfs_db,
but xfs_db can see a "weird" targp.

> OTOH there are other places that use other checks that clearly assume
> that m_rtdev_targp is always present:
> 
> libxfs/init.c:311:      if (mp->m_rtdev_targp->bt_bdev == 0 && !xfs_is_debugger(mp)) {
> libxfs/init.c:1034:     if (mp->m_rtdev_targp != mp->m_ddev_targp)
> libxfs/init.c:1035:             libxfs_buftarg_free(mp->m_rtdev_targp);
> 
> Ok maybe this patch is fine as-is?  But xfsprogs needs a treewide change
> to make the "do we have a rt device attached?" checks match the kernel
> code?

It has passed pretty extensive testing so it must be perfect (TM).
But I agree that another cleanup pass to better match the kernel
sounds like a good idea.  I'll add it to my bucket list.


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-11 15:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11 14:45 fix missing RT detection in libxfs Christoph Hellwig
2026-09-11 14:45 ` [PATCH 1/3] libxfs: don't set mp->m_rtdev_targp when no RT devices is provided Christoph Hellwig
2026-09-11 14:57   ` Darrick J. Wong
2026-09-11 15:11     ` Christoph Hellwig
2026-09-11 15:30       ` Darrick J. Wong
2026-09-11 15:38         ` Christoph Hellwig
2026-09-11 14:45 ` [PATCH 2/3] libxfs: better describe why m_ddev_targp can be set in libxfs_buftarg_init Christoph Hellwig
2026-09-11 14:53   ` Darrick J. Wong
2026-09-11 14:45 ` [PATCH 3/3] libxfs: remove buftarg member aliases Christoph Hellwig
2026-09-11 14:53   ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).