linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* agfl and related cleanups
@ 2020-01-30 13:33 Christoph Hellwig
  2020-01-30 13:33 ` [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl Christoph Hellwig
                   ` (6 more replies)
  0 siblings, 7 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

Hi all,

patch 1 is the interesting one that changes the struct agfl definition
so that we avoid warnings from new gcc versions about taking the address
of a member of a packed structure.  The rest is random cleanups in
related areas.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
@ 2020-01-30 13:33 ` Christoph Hellwig
  2020-02-03 17:46   ` Eric Sandeen
  2020-02-24 22:02   ` Christoph Hellwig
  2020-01-30 13:33 ` [PATCH 2/6] xfs: remove the xfs_agfl_t typedef Christoph Hellwig
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

struct xfs_agfl is a header in front of the AGFL entries that exists
for CRC enabled file systems.  For not CRC enabled file systems the AGFL
is simply a list of agbno.  Make the CRC case similar to that by just
using the list behind the new header.  This indirectly solves a problem
with modern gcc versions that warn about taking addresses of packed
structures (and we have to pack the AGFL given that gcc rounds up
structure sizes).  Also replace the helper macro to get from a buffer
with an inline function in xfs_alloc.h to make the code easier to
read.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c         |  2 +-
 fs/xfs/libxfs/xfs_alloc.c      | 11 ++++++-----
 fs/xfs/libxfs/xfs_alloc.h      |  9 +++++++++
 fs/xfs/libxfs/xfs_format.h     |  6 ------
 fs/xfs/scrub/agheader_repair.c |  2 +-
 5 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 08d6beb54f8c..831bdd035900 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -301,7 +301,7 @@ xfs_agflblock_init(
 		uuid_copy(&agfl->agfl_uuid, &mp->m_sb.sb_meta_uuid);
 	}
 
-	agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, bp);
+	agfl_bno = xfs_buf_to_agfl_bno(bp);
 	for (bucket = 0; bucket < xfs_agfl_size(mp); bucket++)
 		agfl_bno[bucket] = cpu_to_be32(NULLAGBLOCK);
 }
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index d8053bc96c4d..b95a688e9f87 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -589,6 +589,7 @@ xfs_agfl_verify(
 {
 	struct xfs_mount *mp = bp->b_mount;
 	struct xfs_agfl	*agfl = XFS_BUF_TO_AGFL(bp);
+	__be32		*agfl_bno = xfs_buf_to_agfl_bno(bp);
 	int		i;
 
 	/*
@@ -614,8 +615,8 @@ xfs_agfl_verify(
 		return __this_address;
 
 	for (i = 0; i < xfs_agfl_size(mp); i++) {
-		if (be32_to_cpu(agfl->agfl_bno[i]) != NULLAGBLOCK &&
-		    be32_to_cpu(agfl->agfl_bno[i]) >= mp->m_sb.sb_agblocks)
+		if (be32_to_cpu(agfl_bno[i]) != NULLAGBLOCK &&
+		    be32_to_cpu(agfl_bno[i]) >= mp->m_sb.sb_agblocks)
 			return __this_address;
 	}
 
@@ -2684,7 +2685,7 @@ xfs_alloc_get_freelist(
 	/*
 	 * Get the block number and update the data structures.
 	 */
-	agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp);
+	agfl_bno = xfs_buf_to_agfl_bno(agflbp);
 	bno = be32_to_cpu(agfl_bno[be32_to_cpu(agf->agf_flfirst)]);
 	be32_add_cpu(&agf->agf_flfirst, 1);
 	xfs_trans_brelse(tp, agflbp);
@@ -2820,7 +2821,7 @@ xfs_alloc_put_freelist(
 
 	ASSERT(be32_to_cpu(agf->agf_flcount) <= xfs_agfl_size(mp));
 
-	agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp);
+	agfl_bno = xfs_buf_to_agfl_bno(agflbp);
 	blockp = &agfl_bno[be32_to_cpu(agf->agf_fllast)];
 	*blockp = cpu_to_be32(bno);
 	startoff = (char *)blockp - (char *)agflbp->b_addr;
@@ -3408,7 +3409,7 @@ xfs_agfl_walk(
 	unsigned int		i;
 	int			error;
 
-	agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp);
+	agfl_bno = xfs_buf_to_agfl_bno(agflbp);
 	i = be32_to_cpu(agf->agf_flfirst);
 
 	/* Nothing to walk in an empty AGFL. */
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 7380fbe4a3ff..a851bf77f17b 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -236,4 +236,13 @@ typedef int (*xfs_agfl_walk_fn)(struct xfs_mount *mp, xfs_agblock_t bno,
 int xfs_agfl_walk(struct xfs_mount *mp, struct xfs_agf *agf,
 		struct xfs_buf *agflbp, xfs_agfl_walk_fn walk_fn, void *priv);
 
+static inline __be32 *
+xfs_buf_to_agfl_bno(
+	struct xfs_buf		*bp)
+{
+	if (xfs_sb_version_hascrc(&bp->b_mount->m_sb))
+		return bp->b_addr + sizeof(struct xfs_agfl);
+	return bp->b_addr;
+}
+
 #endif	/* __XFS_ALLOC_H__ */
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 77e9fa385980..8c7aea7795da 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -785,18 +785,12 @@ typedef struct xfs_agi {
 #define	XFS_AGFL_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGFL_DADDR(mp))
 #define	XFS_BUF_TO_AGFL(bp)	((xfs_agfl_t *)((bp)->b_addr))
 
-#define XFS_BUF_TO_AGFL_BNO(mp, bp) \
-	(xfs_sb_version_hascrc(&((mp)->m_sb)) ? \
-		&(XFS_BUF_TO_AGFL(bp)->agfl_bno[0]) : \
-		(__be32 *)(bp)->b_addr)
-
 typedef struct xfs_agfl {
 	__be32		agfl_magicnum;
 	__be32		agfl_seqno;
 	uuid_t		agfl_uuid;
 	__be64		agfl_lsn;
 	__be32		agfl_crc;
-	__be32		agfl_bno[];	/* actually xfs_agfl_size(mp) */
 } __attribute__((packed)) xfs_agfl_t;
 
 #define XFS_AGFL_CRC_OFF	offsetof(struct xfs_agfl, agfl_crc)
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index d5e6db9af434..68ee1ce1ae36 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -602,7 +602,7 @@ xrep_agfl_init_header(
 	 * step.
 	 */
 	fl_off = 0;
-	agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agfl_bp);
+	agfl_bno = xfs_buf_to_agfl_bno(agfl_bp);
 	for_each_xfs_bitmap_extent(br, n, agfl_extents) {
 		agbno = XFS_FSB_TO_AGBNO(mp, br->start);
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/6] xfs: remove the xfs_agfl_t typedef
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
  2020-01-30 13:33 ` [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl Christoph Hellwig
@ 2020-01-30 13:33 ` Christoph Hellwig
  2020-02-03 17:46   ` Eric Sandeen
  2020-01-30 13:33 ` [PATCH 3/6] xfs: remove XFS_BUF_TO_AGFL Christoph Hellwig
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

There is just a single user left, so remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_format.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 8c7aea7795da..11a450e00231 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -783,15 +783,15 @@ typedef struct xfs_agi {
  */
 #define XFS_AGFL_DADDR(mp)	((xfs_daddr_t)(3 << (mp)->m_sectbb_log))
 #define	XFS_AGFL_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGFL_DADDR(mp))
-#define	XFS_BUF_TO_AGFL(bp)	((xfs_agfl_t *)((bp)->b_addr))
+#define	XFS_BUF_TO_AGFL(bp)	((struct xfs_agfl *)((bp)->b_addr))
 
-typedef struct xfs_agfl {
+struct xfs_agfl {
 	__be32		agfl_magicnum;
 	__be32		agfl_seqno;
 	uuid_t		agfl_uuid;
 	__be64		agfl_lsn;
 	__be32		agfl_crc;
-} __attribute__((packed)) xfs_agfl_t;
+} __attribute__((packed));
 
 #define XFS_AGFL_CRC_OFF	offsetof(struct xfs_agfl, agfl_crc)
 
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 3/6] xfs: remove XFS_BUF_TO_AGFL
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
  2020-01-30 13:33 ` [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl Christoph Hellwig
  2020-01-30 13:33 ` [PATCH 2/6] xfs: remove the xfs_agfl_t typedef Christoph Hellwig
@ 2020-01-30 13:33 ` Christoph Hellwig
  2020-02-03 18:28   ` Eric Sandeen
  2020-01-30 13:33 ` [PATCH 4/6] xfs: remove XFS_BUF_TO_AGI Christoph Hellwig
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

Just dereference bp->b_addr directly and make the code a little
simpler and more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c         | 2 +-
 fs/xfs/libxfs/xfs_alloc.c      | 7 ++++---
 fs/xfs/libxfs/xfs_format.h     | 1 -
 fs/xfs/scrub/agheader_repair.c | 3 +--
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 831bdd035900..32ceba66456f 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -291,7 +291,7 @@ xfs_agflblock_init(
 	struct xfs_buf		*bp,
 	struct aghdr_init_data	*id)
 {
-	struct xfs_agfl		*agfl = XFS_BUF_TO_AGFL(bp);
+	struct xfs_agfl		*agfl = bp->b_addr;
 	__be32			*agfl_bno;
 	int			bucket;
 
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index b95a688e9f87..2006a49ea95f 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -588,7 +588,7 @@ xfs_agfl_verify(
 	struct xfs_buf	*bp)
 {
 	struct xfs_mount *mp = bp->b_mount;
-	struct xfs_agfl	*agfl = XFS_BUF_TO_AGFL(bp);
+	struct xfs_agfl	*agfl = bp->b_addr;
 	__be32		*agfl_bno = xfs_buf_to_agfl_bno(bp);
 	int		i;
 
@@ -620,7 +620,7 @@ xfs_agfl_verify(
 			return __this_address;
 	}
 
-	if (!xfs_log_check_lsn(mp, be64_to_cpu(XFS_BUF_TO_AGFL(bp)->agfl_lsn)))
+	if (!xfs_log_check_lsn(mp, be64_to_cpu(agfl->agfl_lsn)))
 		return __this_address;
 	return NULL;
 }
@@ -656,6 +656,7 @@ xfs_agfl_write_verify(
 {
 	struct xfs_mount	*mp = bp->b_mount;
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
+	struct xfs_agfl		*agfl = bp->b_addr;
 	xfs_failaddr_t		fa;
 
 	/* no verification of non-crc AGFLs */
@@ -669,7 +670,7 @@ xfs_agfl_write_verify(
 	}
 
 	if (bip)
-		XFS_BUF_TO_AGFL(bp)->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
+		agfl->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
 	xfs_buf_update_cksum(bp, XFS_AGFL_CRC_OFF);
 }
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 11a450e00231..fe685ad91e0f 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -783,7 +783,6 @@ typedef struct xfs_agi {
  */
 #define XFS_AGFL_DADDR(mp)	((xfs_daddr_t)(3 << (mp)->m_sectbb_log))
 #define	XFS_AGFL_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGFL_DADDR(mp))
-#define	XFS_BUF_TO_AGFL(bp)	((struct xfs_agfl *)((bp)->b_addr))
 
 struct xfs_agfl {
 	__be32		agfl_magicnum;
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 68ee1ce1ae36..6da2e87d19a8 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -580,7 +580,7 @@ xrep_agfl_init_header(
 	__be32			*agfl_bno;
 	struct xfs_bitmap_range	*br;
 	struct xfs_bitmap_range	*n;
-	struct xfs_agfl		*agfl;
+	struct xfs_agfl		*agfl = agfl_bp->b_addr;
 	xfs_agblock_t		agbno;
 	unsigned int		fl_off;
 
@@ -590,7 +590,6 @@ xrep_agfl_init_header(
 	 * Start rewriting the header by setting the bno[] array to
 	 * NULLAGBLOCK, then setting AGFL header fields.
 	 */
-	agfl = XFS_BUF_TO_AGFL(agfl_bp);
 	memset(agfl, 0xFF, BBTOB(agfl_bp->b_length));
 	agfl->agfl_magicnum = cpu_to_be32(XFS_AGFL_MAGIC);
 	agfl->agfl_seqno = cpu_to_be32(sc->sa.agno);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/6] xfs: remove XFS_BUF_TO_AGI
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
                   ` (2 preceding siblings ...)
  2020-01-30 13:33 ` [PATCH 3/6] xfs: remove XFS_BUF_TO_AGFL Christoph Hellwig
@ 2020-01-30 13:33 ` Christoph Hellwig
  2020-02-03 18:30   ` Eric Sandeen
  2020-01-30 13:33 ` [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF Christoph Hellwig
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

Just dereference bp->b_addr directly and make the code a little
simpler and more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c           |  6 +++---
 fs/xfs/libxfs/xfs_format.h       |  1 -
 fs/xfs/libxfs/xfs_ialloc.c       | 27 +++++++++++++--------------
 fs/xfs/libxfs/xfs_ialloc_btree.c | 10 +++++-----
 fs/xfs/scrub/agheader.c          |  4 ++--
 fs/xfs/scrub/agheader_repair.c   |  8 ++++----
 fs/xfs/xfs_inode.c               |  6 +++---
 fs/xfs/xfs_log_recover.c         |  6 +++---
 8 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 32ceba66456f..465d0d568411 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -312,7 +312,7 @@ xfs_agiblock_init(
 	struct xfs_buf		*bp,
 	struct aghdr_init_data	*id)
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(bp);
+	struct xfs_agi		*agi = bp->b_addr;
 	int			bucket;
 
 	agi->agi_magicnum = cpu_to_be32(XFS_AGI_MAGIC);
@@ -502,7 +502,7 @@ xfs_ag_extend_space(
 	if (error)
 		return error;
 
-	agi = XFS_BUF_TO_AGI(bp);
+	agi = bp->b_addr;
 	be32_add_cpu(&agi->agi_length, len);
 	ASSERT(id->agno == mp->m_sb.sb_agcount - 1 ||
 	       be32_to_cpu(agi->agi_length) == mp->m_sb.sb_agblocks);
@@ -569,7 +569,7 @@ xfs_ag_get_geometry(
 	memset(ageo, 0, sizeof(*ageo));
 	ageo->ag_number = agno;
 
-	agi = XFS_BUF_TO_AGI(agi_bp);
+	agi = agi_bp->b_addr;
 	ageo->ag_icount = be32_to_cpu(agi->agi_count);
 	ageo->ag_ifree = be32_to_cpu(agi->agi_freecount);
 
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index fe685ad91e0f..5710fed6c75a 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -775,7 +775,6 @@ typedef struct xfs_agi {
 /* disk block (xfs_daddr_t) in the AG */
 #define XFS_AGI_DADDR(mp)	((xfs_daddr_t)(2 << (mp)->m_sectbb_log))
 #define	XFS_AGI_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGI_DADDR(mp))
-#define	XFS_BUF_TO_AGI(bp)	((xfs_agi_t *)((bp)->b_addr))
 
 /*
  * The third a.g. block contains the a.g. freelist, an array
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index bf161e930f1d..b4a404278935 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -177,7 +177,7 @@ xfs_inobt_insert(
 	xfs_btnum_t		btnum)
 {
 	struct xfs_btree_cur	*cur;
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi		*agi = agbp->b_addr;
 	xfs_agnumber_t		agno = be32_to_cpu(agi->agi_seqno);
 	xfs_agino_t		thisino;
 	int			i;
@@ -525,7 +525,7 @@ xfs_inobt_insert_sprec(
 	bool				merge)	/* merge or replace */
 {
 	struct xfs_btree_cur		*cur;
-	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi			*agi = agbp->b_addr;
 	xfs_agnumber_t			agno = be32_to_cpu(agi->agi_seqno);
 	int				error;
 	int				i;
@@ -658,7 +658,7 @@ xfs_ialloc_ag_alloc(
 	 * chunk of inodes.  If the filesystem is striped, this will fill
 	 * an entire stripe unit with inodes.
 	 */
-	agi = XFS_BUF_TO_AGI(agbp);
+	agi = agbp->b_addr;
 	newino = be32_to_cpu(agi->agi_newino);
 	agno = be32_to_cpu(agi->agi_seqno);
 	args.agbno = XFS_AGINO_TO_AGBNO(args.mp, newino) +
@@ -1130,7 +1130,7 @@ xfs_dialloc_ag_inobt(
 	xfs_ino_t		*inop)
 {
 	struct xfs_mount	*mp = tp->t_mountp;
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi		*agi = agbp->b_addr;
 	xfs_agnumber_t		agno = be32_to_cpu(agi->agi_seqno);
 	xfs_agnumber_t		pagno = XFS_INO_TO_AGNO(mp, parent);
 	xfs_agino_t		pagino = XFS_INO_TO_AGINO(mp, parent);
@@ -1583,7 +1583,7 @@ xfs_dialloc_ag(
 	xfs_ino_t		*inop)
 {
 	struct xfs_mount		*mp = tp->t_mountp;
-	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi			*agi = agbp->b_addr;
 	xfs_agnumber_t			agno = be32_to_cpu(agi->agi_seqno);
 	xfs_agnumber_t			pagno = XFS_INO_TO_AGNO(mp, parent);
 	xfs_agino_t			pagino = XFS_INO_TO_AGINO(mp, parent);
@@ -1943,7 +1943,7 @@ xfs_difree_inobt(
 	struct xfs_icluster		*xic,
 	struct xfs_inobt_rec_incore	*orec)
 {
-	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi			*agi = agbp->b_addr;
 	xfs_agnumber_t			agno = be32_to_cpu(agi->agi_seqno);
 	struct xfs_perag		*pag;
 	struct xfs_btree_cur		*cur;
@@ -2079,7 +2079,7 @@ xfs_difree_finobt(
 	xfs_agino_t			agino,
 	struct xfs_inobt_rec_incore	*ibtrec) /* inobt record */
 {
-	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi			*agi = agbp->b_addr;
 	xfs_agnumber_t			agno = be32_to_cpu(agi->agi_seqno);
 	struct xfs_btree_cur		*cur;
 	struct xfs_inobt_rec_incore	rec;
@@ -2489,9 +2489,8 @@ xfs_ialloc_log_agi(
 		sizeof(xfs_agi_t)
 	};
 #ifdef DEBUG
-	xfs_agi_t		*agi;	/* allocation group header */
+	struct xfs_agi		*agi = bp->b_addr;
 
-	agi = XFS_BUF_TO_AGI(bp);
 	ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
 #endif
 
@@ -2523,14 +2522,13 @@ xfs_agi_verify(
 	struct xfs_buf	*bp)
 {
 	struct xfs_mount *mp = bp->b_mount;
-	struct xfs_agi	*agi = XFS_BUF_TO_AGI(bp);
+	struct xfs_agi	*agi = bp->b_addr;
 	int		i;
 
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
 		if (!uuid_equal(&agi->agi_uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
-		if (!xfs_log_check_lsn(mp,
-				be64_to_cpu(XFS_BUF_TO_AGI(bp)->agi_lsn)))
+		if (!xfs_log_check_lsn(mp, be64_to_cpu(agi->agi_lsn)))
 			return __this_address;
 	}
 
@@ -2593,6 +2591,7 @@ xfs_agi_write_verify(
 {
 	struct xfs_mount	*mp = bp->b_mount;
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
+	struct xfs_agi		*agi = bp->b_addr;
 	xfs_failaddr_t		fa;
 
 	fa = xfs_agi_verify(bp);
@@ -2605,7 +2604,7 @@ xfs_agi_write_verify(
 		return;
 
 	if (bip)
-		XFS_BUF_TO_AGI(bp)->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
+		agi->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 	xfs_buf_update_cksum(bp, XFS_AGI_CRC_OFF);
 }
 
@@ -2661,7 +2660,7 @@ xfs_ialloc_read_agi(
 	if (error)
 		return error;
 
-	agi = XFS_BUF_TO_AGI(*bpp);
+	agi = (*bpp)->b_addr;
 	pag = xfs_perag_get(mp, agno);
 	if (!pag->pagi_init) {
 		pag->pagi_freecount = be32_to_cpu(agi->agi_freecount);
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index b82992f795aa..6903820f1c4b 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -45,7 +45,7 @@ xfs_inobt_set_root(
 	int			inc)	/* level change */
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi		*agi = agbp->b_addr;
 
 	agi->agi_root = nptr->s;
 	be32_add_cpu(&agi->agi_level, inc);
@@ -59,7 +59,7 @@ xfs_finobt_set_root(
 	int			inc)	/* level change */
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi		*agi = agbp->b_addr;
 
 	agi->agi_free_root = nptr->s;
 	be32_add_cpu(&agi->agi_free_level, inc);
@@ -212,7 +212,7 @@ xfs_inobt_init_ptr_from_cur(
 	struct xfs_btree_cur	*cur,
 	union xfs_btree_ptr	*ptr)
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(cur->bc_private.a.agbp);
+	struct xfs_agi		*agi = cur->bc_private.a.agbp->b_addr;
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agi->agi_seqno));
 
@@ -224,7 +224,7 @@ xfs_finobt_init_ptr_from_cur(
 	struct xfs_btree_cur	*cur,
 	union xfs_btree_ptr	*ptr)
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(cur->bc_private.a.agbp);
+	struct xfs_agi		*agi = cur->bc_private.a.agbp->b_addr;
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agi->agi_seqno));
 	ptr->s = agi->agi_free_root;
@@ -410,7 +410,7 @@ xfs_inobt_init_cursor(
 	xfs_agnumber_t		agno,		/* allocation group number */
 	xfs_btnum_t		btnum)		/* ialloc or free ino btree */
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agbp);
+	struct xfs_agi		*agi = agbp->b_addr;
 	struct xfs_btree_cur	*cur;
 
 	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index ba0f747c82e8..a117e10feb82 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -765,7 +765,7 @@ static inline void
 xchk_agi_xref_icounts(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(sc->sa.agi_bp);
+	struct xfs_agi		*agi = sc->sa.agi_bp->b_addr;
 	xfs_agino_t		icount;
 	xfs_agino_t		freecount;
 	int			error;
@@ -834,7 +834,7 @@ xchk_agi(
 		goto out;
 	xchk_buffer_recheck(sc, sc->sa.agi_bp);
 
-	agi = XFS_BUF_TO_AGI(sc->sa.agi_bp);
+	agi = sc->sa.agi_bp->b_addr;
 
 	/* Check the AG length */
 	eoag = be32_to_cpu(agi->agi_length);
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 6da2e87d19a8..6f0f5ff2cb3f 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -760,7 +760,7 @@ xrep_agi_init_header(
 	struct xfs_buf		*agi_bp,
 	struct xfs_agi		*old_agi)
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agi_bp);
+	struct xfs_agi		*agi = agi_bp->b_addr;
 	struct xfs_mount	*mp = sc->mp;
 
 	memcpy(old_agi, agi, sizeof(*old_agi));
@@ -806,7 +806,7 @@ xrep_agi_calc_from_btrees(
 	struct xfs_buf		*agi_bp)
 {
 	struct xfs_btree_cur	*cur;
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agi_bp);
+	struct xfs_agi		*agi = agi_bp->b_addr;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agino_t		count;
 	xfs_agino_t		freecount;
@@ -834,7 +834,7 @@ xrep_agi_commit_new(
 	struct xfs_buf		*agi_bp)
 {
 	struct xfs_perag	*pag;
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agi_bp);
+	struct xfs_agi		*agi = agi_bp->b_addr;
 
 	/* Trigger inode count recalculation */
 	xfs_force_summary_recalc(sc->mp);
@@ -891,7 +891,7 @@ xrep_agi(
 	if (error)
 		return error;
 	agi_bp->b_ops = &xfs_agi_buf_ops;
-	agi = XFS_BUF_TO_AGI(agi_bp);
+	agi = agi_bp->b_addr;
 
 	/* Find the AGI btree roots. */
 	error = xrep_agi_find_btrees(sc, fab);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c5077e6326c7..1628e20969aa 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2119,7 +2119,7 @@ xfs_iunlink_update_bucket(
 	unsigned int		bucket_index,
 	xfs_agino_t		new_agino)
 {
-	struct xfs_agi		*agi = XFS_BUF_TO_AGI(agibp);
+	struct xfs_agi		*agi = agibp->b_addr;
 	xfs_agino_t		old_value;
 	int			offset;
 
@@ -2259,7 +2259,7 @@ xfs_iunlink(
 	error = xfs_read_agi(mp, tp, agno, &agibp);
 	if (error)
 		return error;
-	agi = XFS_BUF_TO_AGI(agibp);
+	agi = agibp->b_addr;
 
 	/*
 	 * Get the index into the agi hash table for the list this inode will
@@ -2443,7 +2443,7 @@ xfs_iunlink_remove(
 	error = xfs_read_agi(mp, tp, agno, &agibp);
 	if (error)
 		return error;
-	agi = XFS_BUF_TO_AGI(agibp);
+	agi = agibp->b_addr;
 
 	/*
 	 * Get the index into the agi hash table for the list this inode will
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 25cfc85dbaa7..00d5df5fb26b 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -4947,7 +4947,7 @@ xlog_recover_clear_agi_bucket(
 	if (error)
 		goto out_abort;
 
-	agi = XFS_BUF_TO_AGI(agibp);
+	agi = agibp->b_addr;
 	agi->agi_unlinked[bucket] = cpu_to_be32(NULLAGINO);
 	offset = offsetof(xfs_agi_t, agi_unlinked) +
 		 (sizeof(xfs_agino_t) * bucket);
@@ -5083,7 +5083,7 @@ xlog_recover_process_iunlinks(
 		 * buffer reference though, so that it stays pinned in memory
 		 * while we need the buffer.
 		 */
-		agi = XFS_BUF_TO_AGI(agibp);
+		agi = agibp->b_addr;
 		xfs_buf_unlock(agibp);
 
 		for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++) {
@@ -5840,7 +5840,7 @@ xlog_recover_check_summary(
 			xfs_alert(mp, "%s agi read failed agno %d error %d",
 						__func__, agno, error);
 		} else {
-			struct xfs_agi	*agi = XFS_BUF_TO_AGI(agibp);
+			struct xfs_agi	*agi = agibp->b_addr;
 
 			itotal += be32_to_cpu(agi->agi_count);
 			ifree += be32_to_cpu(agi->agi_freecount);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
                   ` (3 preceding siblings ...)
  2020-01-30 13:33 ` [PATCH 4/6] xfs: remove XFS_BUF_TO_AGI Christoph Hellwig
@ 2020-01-30 13:33 ` Christoph Hellwig
  2020-02-03 18:35   ` Eric Sandeen
  2020-02-03 18:54   ` Eric Sandeen
  2020-01-30 13:33 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
  2020-02-03  9:12 ` agfl and related cleanups Chandan Rajendra
  6 siblings, 2 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

Just dereference bp->b_addr directly and make the code a little
simpler and more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c             |  6 ++--
 fs/xfs/libxfs/xfs_alloc.c          | 52 +++++++++++++-----------------
 fs/xfs/libxfs/xfs_alloc_btree.c    | 10 +++---
 fs/xfs/libxfs/xfs_format.h         |  1 -
 fs/xfs/libxfs/xfs_refcount_btree.c | 12 +++----
 fs/xfs/libxfs/xfs_rmap_btree.c     | 12 +++----
 fs/xfs/scrub/agheader.c            | 14 ++++----
 fs/xfs/scrub/agheader_repair.c     | 14 ++++----
 fs/xfs/scrub/repair.c              |  8 +++--
 fs/xfs/xfs_discard.c               |  7 ++--
 fs/xfs/xfs_log_recover.c           |  4 +--
 11 files changed, 68 insertions(+), 72 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 465d0d568411..447e363d8468 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -243,7 +243,7 @@ xfs_agfblock_init(
 	struct xfs_buf		*bp,
 	struct aghdr_init_data	*id)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(bp);
+	struct xfs_agf		*agf = bp->b_addr;
 	xfs_extlen_t		tmpsize;
 
 	agf->agf_magicnum = cpu_to_be32(XFS_AGF_MAGIC);
@@ -515,7 +515,7 @@ xfs_ag_extend_space(
 	if (error)
 		return error;
 
-	agf = XFS_BUF_TO_AGF(bp);
+	agf = bp->b_addr;
 	be32_add_cpu(&agf->agf_length, len);
 	ASSERT(agf->agf_length == agi->agi_length);
 	xfs_alloc_log_agf(tp, bp, XFS_AGF_LENGTH);
@@ -573,7 +573,7 @@ xfs_ag_get_geometry(
 	ageo->ag_icount = be32_to_cpu(agi->agi_count);
 	ageo->ag_ifree = be32_to_cpu(agi->agi_freecount);
 
-	agf = XFS_BUF_TO_AGF(agf_bp);
+	agf = agf_bp->b_addr;
 	ageo->ag_length = be32_to_cpu(agf->agf_length);
 	freeblks = pag->pagf_freeblks +
 		   pag->pagf_flcount +
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 2006a49ea95f..8a47fb296082 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -715,7 +715,7 @@ xfs_alloc_update_counters(
 	struct xfs_buf		*agbp,
 	long			len)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 
 	pag->pagf_freeblks += len;
 	be32_add_cpu(&agf->agf_freeblks, len);
@@ -924,13 +924,13 @@ xfs_alloc_cur_finish(
 	struct xfs_alloc_arg	*args,
 	struct xfs_alloc_cur	*acur)
 {
+	struct xfs_agf __maybe_unused *agf = args->agbp->b_addr;
 	int			error;
 
 	ASSERT(acur->cnt && acur->bnolt);
 	ASSERT(acur->bno >= acur->rec_bno);
 	ASSERT(acur->bno + acur->len <= acur->rec_bno + acur->rec_len);
-	ASSERT(acur->rec_bno + acur->rec_len <=
-	       be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+	ASSERT(acur->rec_bno + acur->rec_len <= be32_to_cpu(agf->agf_length));
 
 	error = xfs_alloc_fixup_trees(acur->cnt, acur->bnolt, acur->rec_bno,
 				      acur->rec_len, acur->bno, acur->len, 0);
@@ -1028,6 +1028,7 @@ xfs_alloc_ag_vextent_small(
 	xfs_extlen_t		*flenp,	/* result length */
 	int			*stat)	/* status: 0-freelist, 1-normal/none */
 {
+	struct xfs_agf		*agf = args->agbp->b_addr;
 	int			error = 0;
 	xfs_agblock_t		fbno = NULLAGBLOCK;
 	xfs_extlen_t		flen = 0;
@@ -1056,8 +1057,7 @@ xfs_alloc_ag_vextent_small(
 
 	if (args->minlen != 1 || args->alignment != 1 ||
 	    args->resv == XFS_AG_RESV_AGFL ||
-	    (be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_flcount) <=
-	     args->minleft))
+	    be32_to_cpu(agf->agf_flcount) <= args->minleft)
 		goto out;
 
 	error = xfs_alloc_get_freelist(args->tp, args->agbp, &fbno, 0);
@@ -1081,9 +1081,7 @@ xfs_alloc_ag_vextent_small(
 	}
 	*fbnop = args->agbno = fbno;
 	*flenp = args->len = 1;
-	if (XFS_IS_CORRUPT(args->mp,
-			   fbno >= be32_to_cpu(
-				   XFS_BUF_TO_AGF(args->agbp)->agf_length))) {
+	if (XFS_IS_CORRUPT(args->mp, fbno >= be32_to_cpu(agf->agf_length))) {
 		error = -EFSCORRUPTED;
 		goto error;
 	}
@@ -1205,6 +1203,7 @@ STATIC int			/* error */
 xfs_alloc_ag_vextent_exact(
 	xfs_alloc_arg_t	*args)	/* allocation argument structure */
 {
+	struct xfs_agf __maybe_unused *agf = args->agbp->b_addr;
 	xfs_btree_cur_t	*bno_cur;/* by block-number btree cursor */
 	xfs_btree_cur_t	*cnt_cur;/* by count btree cursor */
 	int		error;
@@ -1283,8 +1282,7 @@ xfs_alloc_ag_vextent_exact(
 	 */
 	cnt_cur = xfs_allocbt_init_cursor(args->mp, args->tp, args->agbp,
 		args->agno, XFS_BTNUM_CNT);
-	ASSERT(args->agbno + args->len <=
-		be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+	ASSERT(args->agbno + args->len <= be32_to_cpu(agf->agf_length));
 	error = xfs_alloc_fixup_trees(cnt_cur, bno_cur, fbno, flen, args->agbno,
 				      args->len, XFSA_FIXUP_BNO_OK);
 	if (error) {
@@ -1663,6 +1661,7 @@ STATIC int				/* error */
 xfs_alloc_ag_vextent_size(
 	xfs_alloc_arg_t	*args)		/* allocation argument structure */
 {
+	struct xfs_agf	*agf = args->agbp->b_addr;
 	xfs_btree_cur_t	*bno_cur;	/* cursor for bno btree */
 	xfs_btree_cur_t	*cnt_cur;	/* cursor for cnt btree */
 	int		error;		/* error result */
@@ -1853,8 +1852,7 @@ xfs_alloc_ag_vextent_size(
 	args->agbno = rbno;
 	if (XFS_IS_CORRUPT(args->mp,
 			   args->agbno + args->len >
-			   be32_to_cpu(
-				   XFS_BUF_TO_AGF(args->agbp)->agf_length))) {
+			   be32_to_cpu(agf->agf_length))) {
 		error = -EFSCORRUPTED;
 		goto error0;
 	}
@@ -2426,7 +2424,7 @@ xfs_agfl_reset(
 	struct xfs_perag	*pag)
 {
 	struct xfs_mount	*mp = tp->t_mountp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 
 	ASSERT(pag->pagf_agflreset);
 	trace_xfs_agfl_reset(mp, agf, 0, _RET_IP_);
@@ -2657,7 +2655,7 @@ xfs_alloc_get_freelist(
 	xfs_agblock_t	*bnop,	/* block address retrieved from freelist */
 	int		btreeblk) /* destination is a AGF btree */
 {
-	xfs_agf_t	*agf;	/* a.g. freespace structure */
+	struct xfs_agf	*agf = agbp->b_addr;
 	xfs_buf_t	*agflbp;/* buffer for a.g. freelist structure */
 	xfs_agblock_t	bno;	/* block number returned */
 	__be32		*agfl_bno;
@@ -2669,7 +2667,6 @@ xfs_alloc_get_freelist(
 	/*
 	 * Freelist is empty, give up.
 	 */
-	agf = XFS_BUF_TO_AGF(agbp);
 	if (!agf->agf_flcount) {
 		*bnop = NULLAGBLOCK;
 		return 0;
@@ -2747,7 +2744,7 @@ xfs_alloc_log_agf(
 		sizeof(xfs_agf_t)
 	};
 
-	trace_xfs_agf(tp->t_mountp, XFS_BUF_TO_AGF(bp), fields, _RET_IP_);
+	trace_xfs_agf(tp->t_mountp, bp->b_addr, fields, _RET_IP_);
 
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_AGF_BUF);
 
@@ -2785,18 +2782,15 @@ xfs_alloc_put_freelist(
 	xfs_agblock_t		bno,	/* block being freed */
 	int			btreeblk) /* block came from a AGF btree */
 {
-	xfs_agf_t		*agf;	/* a.g. freespace structure */
+	struct xfs_mount	*mp = tp->t_mountp;
+	struct xfs_agf		*agf = agbp->b_addr;
 	__be32			*blockp;/* pointer to array entry */
 	int			error;
 	int			logflags;
-	xfs_mount_t		*mp;	/* mount structure */
 	xfs_perag_t		*pag;	/* per allocation group data */
 	__be32			*agfl_bno;
 	int			startoff;
 
-	agf = XFS_BUF_TO_AGF(agbp);
-	mp = tp->t_mountp;
-
 	if (!agflbp && (error = xfs_alloc_read_agfl(mp, tp,
 			be32_to_cpu(agf->agf_seqno), &agflbp)))
 		return error;
@@ -2840,13 +2834,12 @@ xfs_agf_verify(
 	struct xfs_buf		*bp)
 {
 	struct xfs_mount	*mp = bp->b_mount;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(bp);
+	struct xfs_agf		*agf = bp->b_addr;
 
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
 		if (!uuid_equal(&agf->agf_uuid, &mp->m_sb.sb_meta_uuid))
 			return __this_address;
-		if (!xfs_log_check_lsn(mp,
-				be64_to_cpu(XFS_BUF_TO_AGF(bp)->agf_lsn)))
+		if (!xfs_log_check_lsn(mp, be64_to_cpu(agf->agf_lsn)))
 			return __this_address;
 	}
 
@@ -2916,6 +2909,7 @@ xfs_agf_write_verify(
 {
 	struct xfs_mount	*mp = bp->b_mount;
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
+	struct xfs_agf		*agf = bp->b_addr;
 	xfs_failaddr_t		fa;
 
 	fa = xfs_agf_verify(bp);
@@ -2928,7 +2922,7 @@ xfs_agf_write_verify(
 		return;
 
 	if (bip)
-		XFS_BUF_TO_AGF(bp)->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
+		agf->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
 	xfs_buf_update_cksum(bp, XFS_AGF_CRC_OFF);
 }
@@ -2996,7 +2990,7 @@ xfs_alloc_read_agf(
 		return error;
 	ASSERT(!(*bpp)->b_error);
 
-	agf = XFS_BUF_TO_AGF(*bpp);
+	agf = (*bpp)->b_addr;
 	pag = xfs_perag_get(mp, agno);
 	if (!pag->pagf_init) {
 		pag->pagf_freeblks = be32_to_cpu(agf->agf_freeblks);
@@ -3277,6 +3271,7 @@ __xfs_free_extent(
 	struct xfs_buf			*agbp;
 	xfs_agnumber_t			agno = XFS_FSB_TO_AGNO(mp, bno);
 	xfs_agblock_t			agbno = XFS_FSB_TO_AGBNO(mp, bno);
+	struct xfs_agf			*agf;
 	int				error;
 	unsigned int			busy_flags = 0;
 
@@ -3290,6 +3285,7 @@ __xfs_free_extent(
 	error = xfs_free_extent_fix_freelist(tp, agno, &agbp);
 	if (error)
 		return error;
+	agf = agbp->b_addr;
 
 	if (XFS_IS_CORRUPT(mp, agbno >= mp->m_sb.sb_agblocks)) {
 		error = -EFSCORRUPTED;
@@ -3297,9 +3293,7 @@ __xfs_free_extent(
 	}
 
 	/* validate the extent size is legal now we have the agf locked */
-	if (XFS_IS_CORRUPT(mp,
-			   agbno + len >
-			   be32_to_cpu(XFS_BUF_TO_AGF(agbp)->agf_length))) {
+	if (XFS_IS_CORRUPT(mp, agbno + len > be32_to_cpu(agf->agf_length))) {
 		error = -EFSCORRUPTED;
 		goto err;
 	}
diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 279694d73e4e..b1b3dc1b0b89 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -36,7 +36,7 @@ xfs_allocbt_set_root(
 	int			inc)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_agnumber_t		seqno = be32_to_cpu(agf->agf_seqno);
 	int			btnum = cur->bc_btnum;
 	struct xfs_perag	*pag = xfs_perag_get(cur->bc_mp, seqno);
@@ -87,7 +87,7 @@ xfs_allocbt_free_block(
 	struct xfs_buf		*bp)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_agblock_t		bno;
 	int			error;
 
@@ -113,7 +113,7 @@ xfs_allocbt_update_lastrec(
 	int			ptr,
 	int			reason)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
+	struct xfs_agf		*agf = cur->bc_private.a.agbp->b_addr;
 	xfs_agnumber_t		seqno = be32_to_cpu(agf->agf_seqno);
 	struct xfs_perag	*pag;
 	__be32			len;
@@ -226,7 +226,7 @@ xfs_allocbt_init_ptr_from_cur(
 	struct xfs_btree_cur	*cur,
 	union xfs_btree_ptr	*ptr)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
+	struct xfs_agf		*agf = cur->bc_private.a.agbp->b_addr;
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agf->agf_seqno));
 
@@ -482,7 +482,7 @@ xfs_allocbt_init_cursor(
 	xfs_agnumber_t		agno,		/* allocation group number */
 	xfs_btnum_t		btnum)		/* btree identifier */
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	struct xfs_btree_cur	*cur;
 
 	ASSERT(btnum == XFS_BTNUM_BNO || btnum == XFS_BTNUM_CNT);
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 5710fed6c75a..03531f0f537a 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -707,7 +707,6 @@ typedef struct xfs_agf {
 /* disk block (xfs_daddr_t) in the AG */
 #define XFS_AGF_DADDR(mp)	((xfs_daddr_t)(1 << (mp)->m_sectbb_log))
 #define	XFS_AGF_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGF_DADDR(mp))
-#define	XFS_BUF_TO_AGF(bp)	((xfs_agf_t *)((bp)->b_addr))
 
 /*
  * Size of the unlinked inode hash table in the agi.
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 38529dbacd55..a76997740e45 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -35,7 +35,7 @@ xfs_refcountbt_set_root(
 	int			inc)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_agnumber_t		seqno = be32_to_cpu(agf->agf_seqno);
 	struct xfs_perag	*pag = xfs_perag_get(cur->bc_mp, seqno);
 
@@ -58,7 +58,7 @@ xfs_refcountbt_alloc_block(
 	int			*stat)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	struct xfs_alloc_arg	args;		/* block allocation args */
 	int			error;		/* error return value */
 
@@ -102,7 +102,7 @@ xfs_refcountbt_free_block(
 {
 	struct xfs_mount	*mp = cur->bc_mp;
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
 	int			error;
 
@@ -169,7 +169,7 @@ xfs_refcountbt_init_ptr_from_cur(
 	struct xfs_btree_cur	*cur,
 	union xfs_btree_ptr	*ptr)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
+	struct xfs_agf		*agf = cur->bc_private.a.agbp->b_addr;
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agf->agf_seqno));
 
@@ -320,7 +320,7 @@ xfs_refcountbt_init_cursor(
 	struct xfs_buf		*agbp,
 	xfs_agnumber_t		agno)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	struct xfs_btree_cur	*cur;
 
 	ASSERT(agno != NULLAGNUMBER);
@@ -420,7 +420,7 @@ xfs_refcountbt_calc_reserves(
 	if (error)
 		return error;
 
-	agf = XFS_BUF_TO_AGF(agbp);
+	agf = agbp->b_addr;
 	agblocks = be32_to_cpu(agf->agf_length);
 	tree_len = be32_to_cpu(agf->agf_refcount_blocks);
 	xfs_trans_brelse(tp, agbp);
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index fc78efa52c94..725cb892f157 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -61,7 +61,7 @@ xfs_rmapbt_set_root(
 	int			inc)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_agnumber_t		seqno = be32_to_cpu(agf->agf_seqno);
 	int			btnum = cur->bc_btnum;
 	struct xfs_perag	*pag = xfs_perag_get(cur->bc_mp, seqno);
@@ -84,7 +84,7 @@ xfs_rmapbt_alloc_block(
 	int			*stat)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	int			error;
 	xfs_agblock_t		bno;
 
@@ -121,7 +121,7 @@ xfs_rmapbt_free_block(
 	struct xfs_buf		*bp)
 {
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	xfs_agblock_t		bno;
 	int			error;
 
@@ -215,7 +215,7 @@ xfs_rmapbt_init_ptr_from_cur(
 	struct xfs_btree_cur	*cur,
 	union xfs_btree_ptr	*ptr)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
+	struct xfs_agf		*agf = cur->bc_private.a.agbp->b_addr;
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agf->agf_seqno));
 
@@ -458,7 +458,7 @@ xfs_rmapbt_init_cursor(
 	struct xfs_buf		*agbp,
 	xfs_agnumber_t		agno)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
+	struct xfs_agf		*agf = agbp->b_addr;
 	struct xfs_btree_cur	*cur;
 
 	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
@@ -569,7 +569,7 @@ xfs_rmapbt_calc_reserves(
 	if (error)
 		return error;
 
-	agf = XFS_BUF_TO_AGF(agbp);
+	agf = agbp->b_addr;
 	agblocks = be32_to_cpu(agf->agf_length);
 	tree_len = be32_to_cpu(agf->agf_rmap_blocks);
 	xfs_trans_brelse(tp, agbp);
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index a117e10feb82..163478855e7b 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -358,7 +358,7 @@ static inline void
 xchk_agf_xref_freeblks(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
+	struct xfs_agf		*agf = sc->sa.agf_bp->b_addr;
 	xfs_extlen_t		blocks = 0;
 	int			error;
 
@@ -378,7 +378,7 @@ static inline void
 xchk_agf_xref_cntbt(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
+	struct xfs_agf		*agf = sc->sa.agf_bp->b_addr;
 	xfs_agblock_t		agbno;
 	xfs_extlen_t		blocks;
 	int			have;
@@ -410,7 +410,7 @@ STATIC void
 xchk_agf_xref_btreeblks(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
+	struct xfs_agf		*agf = sc->sa.agf_bp->b_addr;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agblock_t		blocks;
 	xfs_agblock_t		btreeblks;
@@ -456,7 +456,7 @@ static inline void
 xchk_agf_xref_refcblks(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
+	struct xfs_agf		*agf = sc->sa.agf_bp->b_addr;
 	xfs_agblock_t		blocks;
 	int			error;
 
@@ -525,7 +525,7 @@ xchk_agf(
 		goto out;
 	xchk_buffer_recheck(sc, sc->sa.agf_bp);
 
-	agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
+	agf = sc->sa.agf_bp->b_addr;
 
 	/* Check the AG length */
 	eoag = be32_to_cpu(agf->agf_length);
@@ -711,7 +711,7 @@ xchk_agfl(
 		goto out;
 
 	/* Allocate buffer to ensure uniqueness of AGFL entries. */
-	agf = XFS_BUF_TO_AGF(sc->sa.agf_bp);
+	agf = sc->sa.agf_bp->b_addr;
 	agflcount = be32_to_cpu(agf->agf_flcount);
 	if (agflcount > xfs_agfl_size(sc->mp)) {
 		xchk_block_set_corrupt(sc, sc->sa.agf_bp);
@@ -728,7 +728,7 @@ xchk_agfl(
 	}
 
 	/* Check the blocks in the AGFL. */
-	error = xfs_agfl_walk(sc->mp, XFS_BUF_TO_AGF(sc->sa.agf_bp),
+	error = xfs_agfl_walk(sc->mp, sc->sa.agf_bp->b_addr,
 			sc->sa.agfl_bp, xchk_agfl_block, &sai);
 	if (error == -ECANCELED) {
 		error = 0;
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index 6f0f5ff2cb3f..c801f5892210 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -140,7 +140,7 @@ xrep_agf_find_btrees(
 	struct xrep_find_ag_btree	*fab,
 	struct xfs_buf			*agfl_bp)
 {
-	struct xfs_agf			*old_agf = XFS_BUF_TO_AGF(agf_bp);
+	struct xfs_agf			*old_agf = agf_bp->b_addr;
 	int				error;
 
 	/* Go find the root data. */
@@ -181,7 +181,7 @@ xrep_agf_init_header(
 	struct xfs_agf		*old_agf)
 {
 	struct xfs_mount	*mp = sc->mp;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agf_bp);
+	struct xfs_agf		*agf = agf_bp->b_addr;
 
 	memcpy(old_agf, agf, sizeof(*old_agf));
 	memset(agf, 0, BBTOB(agf_bp->b_length));
@@ -238,7 +238,7 @@ xrep_agf_calc_from_btrees(
 {
 	struct xrep_agf_allocbt	raa = { .sc = sc };
 	struct xfs_btree_cur	*cur = NULL;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agf_bp);
+	struct xfs_agf		*agf = agf_bp->b_addr;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agblock_t		btreeblks;
 	xfs_agblock_t		blocks;
@@ -302,7 +302,7 @@ xrep_agf_commit_new(
 	struct xfs_buf		*agf_bp)
 {
 	struct xfs_perag	*pag;
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agf_bp);
+	struct xfs_agf		*agf = agf_bp->b_addr;
 
 	/* Trigger fdblocks recalculation */
 	xfs_force_summary_recalc(sc->mp);
@@ -376,7 +376,7 @@ xrep_agf(
 	if (error)
 		return error;
 	agf_bp->b_ops = &xfs_agf_buf_ops;
-	agf = XFS_BUF_TO_AGF(agf_bp);
+	agf = agf_bp->b_addr;
 
 	/*
 	 * Load the AGFL so that we can screen out OWN_AG blocks that are on
@@ -395,7 +395,7 @@ xrep_agf(
 	 * Spot-check the AGFL blocks; if they're obviously corrupt then
 	 * there's nothing we can do but bail out.
 	 */
-	error = xfs_agfl_walk(sc->mp, XFS_BUF_TO_AGF(agf_bp), agfl_bp,
+	error = xfs_agfl_walk(sc->mp, agf_bp->b_addr, agfl_bp,
 			xrep_agf_check_agfl_block, sc);
 	if (error)
 		return error;
@@ -550,7 +550,7 @@ xrep_agfl_update_agf(
 	struct xfs_buf		*agf_bp,
 	xfs_agblock_t		flcount)
 {
-	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agf_bp);
+	struct xfs_agf		*agf = agf_bp->b_addr;
 
 	ASSERT(flcount <= xfs_agfl_size(sc->mp));
 
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index e489d7a8446a..0d5509bf8581 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -208,8 +208,10 @@ xrep_calc_ag_resblks(
 	/* Now grab the block counters from the AGF. */
 	error = xfs_alloc_read_agf(mp, NULL, sm->sm_agno, 0, &bp);
 	if (!error) {
-		aglen = be32_to_cpu(XFS_BUF_TO_AGF(bp)->agf_length);
-		freelen = be32_to_cpu(XFS_BUF_TO_AGF(bp)->agf_freeblks);
+		struct xfs_agf	*agf = bp->b_addr;
+
+		aglen = be32_to_cpu(agf->agf_length);
+		freelen = be32_to_cpu(agf->agf_freeblks);
 		usedlen = aglen - freelen;
 		xfs_buf_relse(bp);
 	}
@@ -879,7 +881,7 @@ xrep_find_ag_btree_roots(
 
 	ri.sc = sc;
 	ri.btree_info = btree_info;
-	ri.agf = XFS_BUF_TO_AGF(agf_bp);
+	ri.agf = agf_bp->b_addr;
 	ri.agfl_bp = agfl_bp;
 	for (fab = btree_info; fab->buf_ops; fab++) {
 		ASSERT(agfl_bp || fab->rmap_owner != XFS_RMAP_OWN_AG);
diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index 0b8350e84d28..f979d0d7e6cd 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -31,6 +31,7 @@ xfs_trim_extents(
 	struct block_device	*bdev = mp->m_ddev_targp->bt_bdev;
 	struct xfs_btree_cur	*cur;
 	struct xfs_buf		*agbp;
+	struct xfs_agf		*agf;
 	struct xfs_perag	*pag;
 	int			error;
 	int			i;
@@ -47,14 +48,14 @@ xfs_trim_extents(
 	error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
 	if (error)
 		goto out_put_perag;
+	agf = agbp->b_addr;
 
 	cur = xfs_allocbt_init_cursor(mp, NULL, agbp, agno, XFS_BTNUM_CNT);
 
 	/*
 	 * Look up the longest btree in the AGF and start with it.
 	 */
-	error = xfs_alloc_lookup_ge(cur, 0,
-			    be32_to_cpu(XFS_BUF_TO_AGF(agbp)->agf_longest), &i);
+	error = xfs_alloc_lookup_ge(cur, 0, be32_to_cpu(agf->agf_longest), &i);
 	if (error)
 		goto out_del_cursor;
 
@@ -75,7 +76,7 @@ xfs_trim_extents(
 			error = -EFSCORRUPTED;
 			goto out_del_cursor;
 		}
-		ASSERT(flen <= be32_to_cpu(XFS_BUF_TO_AGF(agbp)->agf_longest));
+		ASSERT(flen <= be32_to_cpu(agf->agf_longest));
 
 		/*
 		 * use daddr format for all range/len calculations as that is
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 00d5df5fb26b..b6cf99f7153f 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -5809,7 +5809,6 @@ xlog_recover_check_summary(
 	struct xlog	*log)
 {
 	xfs_mount_t	*mp;
-	xfs_agf_t	*agfp;
 	xfs_buf_t	*agfbp;
 	xfs_buf_t	*agibp;
 	xfs_agnumber_t	agno;
@@ -5829,7 +5828,8 @@ xlog_recover_check_summary(
 			xfs_alert(mp, "%s agf read failed agno %d error %d",
 						__func__, agno, error);
 		} else {
-			agfp = XFS_BUF_TO_AGF(agfbp);
+			struct xfs_agf	*agfp = agfbp->b_addr;
+
 			freeblks += be32_to_cpu(agfp->agf_freeblks) +
 				    be32_to_cpu(agfp->agf_flcount);
 			xfs_buf_relse(agfbp);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
                   ` (4 preceding siblings ...)
  2020-01-30 13:33 ` [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF Christoph Hellwig
@ 2020-01-30 13:33 ` Christoph Hellwig
  2020-02-03 18:38   ` Eric Sandeen
  2020-02-03  9:12 ` agfl and related cleanups Chandan Rajendra
  6 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-01-30 13:33 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

Just dereference bp->b_addr directly and make the code a little
simpler and more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c         |  2 +-
 fs/xfs/libxfs/xfs_format.h     |  1 -
 fs/xfs/libxfs/xfs_sb.c         | 17 +++++++++--------
 fs/xfs/scrub/agheader.c        |  2 +-
 fs/xfs/scrub/agheader_repair.c |  2 +-
 fs/xfs/xfs_log_recover.c       |  2 +-
 fs/xfs/xfs_mount.c             |  2 +-
 fs/xfs/xfs_trans.c             |  2 +-
 8 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 447e363d8468..f9b8c177ebc3 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -231,7 +231,7 @@ xfs_sbblock_init(
 	struct xfs_buf		*bp,
 	struct aghdr_init_data	*id)
 {
-	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb		*dsb = bp->b_addr;
 
 	xfs_sb_to_disk(dsb, &mp->m_sb);
 	dsb->sb_inprogress = 1;
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 03531f0f537a..81a1b7084008 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -560,7 +560,6 @@ xfs_is_quota_inode(struct xfs_sb *sbp, xfs_ino_t ino)
 
 #define XFS_SB_DADDR		((xfs_daddr_t)0) /* daddr in filesystem/ag */
 #define	XFS_SB_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_SB_DADDR)
-#define XFS_BUF_TO_SBP(bp)	((xfs_dsb_t *)((bp)->b_addr))
 
 #define	XFS_HDR_BLOCK(mp,d)	((xfs_agblock_t)XFS_BB_TO_FSBT(mp,d))
 #define	XFS_DADDR_TO_FSB(mp,d)	XFS_AGB_TO_FSB(mp, \
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 2f60fc3c99a0..772649f4eed6 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -220,7 +220,7 @@ xfs_validate_sb_common(
 	struct xfs_buf		*bp,
 	struct xfs_sb		*sbp)
 {
-	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb		*dsb = bp->b_addr;
 	uint32_t		agcount = 0;
 	uint32_t		rem;
 
@@ -681,7 +681,7 @@ xfs_sb_read_verify(
 {
 	struct xfs_sb		sb;
 	struct xfs_mount	*mp = bp->b_mount;
-	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb		*dsb = bp->b_addr;
 	int			error;
 
 	/*
@@ -707,7 +707,7 @@ xfs_sb_read_verify(
 	 * Check all the superblock fields.  Don't byteswap the xquota flags
 	 * because _verify_common checks the on-disk values.
 	 */
-	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
+	__xfs_sb_from_disk(&sb, bp->b_addr, false);
 	error = xfs_validate_sb_common(mp, bp, &sb);
 	if (error)
 		goto out_error;
@@ -730,7 +730,7 @@ static void
 xfs_sb_quiet_read_verify(
 	struct xfs_buf	*bp)
 {
-	struct xfs_dsb	*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb	*dsb = bp->b_addr;
 
 	if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
 		/* XFS filesystem, verify noisily! */
@@ -748,13 +748,14 @@ xfs_sb_write_verify(
 	struct xfs_sb		sb;
 	struct xfs_mount	*mp = bp->b_mount;
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
+	struct xfs_dsb		*dsb = bp->b_addr;
 	int			error;
 
 	/*
 	 * Check all the superblock fields.  Don't byteswap the xquota flags
 	 * because _verify_common checks the on-disk values.
 	 */
-	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
+	__xfs_sb_from_disk(&sb, dsb, false);
 	error = xfs_validate_sb_common(mp, bp, &sb);
 	if (error)
 		goto out_error;
@@ -766,7 +767,7 @@ xfs_sb_write_verify(
 		return;
 
 	if (bip)
-		XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
+		dsb->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
 	xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF);
 	return;
@@ -927,7 +928,7 @@ xfs_log_sb(
 	mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
 	mp->m_sb.sb_fdblocks = percpu_counter_sum(&mp->m_fdblocks);
 
-	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_SB_BUF);
 	xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsb) - 1);
 }
@@ -1007,7 +1008,7 @@ xfs_update_secondary_sbs(
 		bp->b_ops = &xfs_sb_buf_ops;
 		xfs_buf_oneshot(bp);
 		xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
-		xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+		xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
 		xfs_buf_delwri_queue(bp, &buffer_list);
 		xfs_buf_relse(bp);
 
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 163478855e7b..e9bcf1faa183 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -92,7 +92,7 @@ xchk_superblock(
 	if (!xchk_process_error(sc, agno, XFS_SB_BLOCK(mp), &error))
 		return error;
 
-	sb = XFS_BUF_TO_SBP(bp);
+	sb = bp->b_addr;
 
 	/*
 	 * Verify the geometries match.  Fields that are permanently
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index c801f5892210..31dbb5d556fd 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -49,7 +49,7 @@ xrep_superblock(
 
 	/* Copy AG 0's superblock to this one. */
 	xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
-	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
 
 	/* Write this to disk. */
 	xfs_trans_buf_set_type(sc->tp, bp, XFS_BLFT_SB_BUF);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index b6cf99f7153f..6abc0863c9c3 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -5636,7 +5636,7 @@ xlog_do_recover(
 
 	/* Convert superblock from on-disk format */
 	sbp = &mp->m_sb;
-	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
+	xfs_sb_from_disk(sbp, bp->b_addr);
 	xfs_buf_relse(bp);
 
 	/* re-initialise in-core superblock and geometry structures */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 56efe140c923..c5513e5a226a 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -310,7 +310,7 @@ xfs_readsb(
 	/*
 	 * Initialize the mount structure from the superblock.
 	 */
-	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
+	xfs_sb_from_disk(sbp, bp->b_addr);
 
 	/*
 	 * If we haven't validated the superblock, do so now before we try
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 3b208f9a865c..73c534093f09 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -450,7 +450,7 @@ xfs_trans_apply_sb_deltas(
 	int		whole = 0;
 
 	bp = xfs_trans_getsb(tp, tp->t_mountp);
-	sbp = XFS_BUF_TO_SBP(bp);
+	sbp = bp->b_addr;
 
 	/*
 	 * Check that superblock mods match the mods made to AGF counters.
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: agfl and related cleanups
  2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
                   ` (5 preceding siblings ...)
  2020-01-30 13:33 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
@ 2020-02-03  9:12 ` Chandan Rajendra
  6 siblings, 0 replies; 31+ messages in thread
From: Chandan Rajendra @ 2020-02-03  9:12 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, Eric Sandeen

On Thursday, January 30, 2020 7:03 PM Christoph Hellwig wrote: 

> Hi all,
> 
> patch 1 is the interesting one that changes the struct agfl definition
> so that we avoid warnings from new gcc versions about taking the address
> of a member of a packed structure.  The rest is random cleanups in
> related areas.

The changes look good  to me,

Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>

-- 
chandan




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-01-30 13:33 ` [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl Christoph Hellwig
@ 2020-02-03 17:46   ` Eric Sandeen
  2020-02-03 17:47     ` Christoph Hellwig
  2020-02-24 22:02   ` Christoph Hellwig
  1 sibling, 1 reply; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 17:46 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> struct xfs_agfl is a header in front of the AGFL entries that exists
> for CRC enabled file systems.  For not CRC enabled file systems the AGFL
> is simply a list of agbno.  Make the CRC case similar to that by just
> using the list behind the new header.  This indirectly solves a problem
> with modern gcc versions that warn about taking addresses of packed
> structures (and we have to pack the AGFL given that gcc rounds up
> structure sizes).  Also replace the helper macro to get from a buffer
> with an inline function in xfs_alloc.h to make the code easier to
> read.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

I like it.

Giving it an RVB but are we 100% sure that there won't ever be any padding
after the xfs_agfl structure before the bno?  I don't understand gcc
alignment magic.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>




^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/6] xfs: remove the xfs_agfl_t typedef
  2020-01-30 13:33 ` [PATCH 2/6] xfs: remove the xfs_agfl_t typedef Christoph Hellwig
@ 2020-02-03 17:46   ` Eric Sandeen
  0 siblings, 0 replies; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 17:46 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> There is just a single user left, so remove it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-03 17:46   ` Eric Sandeen
@ 2020-02-03 17:47     ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-03 17:47 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 03, 2020 at 11:46:11AM -0600, Eric Sandeen wrote:
> On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> > struct xfs_agfl is a header in front of the AGFL entries that exists
> > for CRC enabled file systems.  For not CRC enabled file systems the AGFL
> > is simply a list of agbno.  Make the CRC case similar to that by just
> > using the list behind the new header.  This indirectly solves a problem
> > with modern gcc versions that warn about taking addresses of packed
> > structures (and we have to pack the AGFL given that gcc rounds up
> > structure sizes).  Also replace the helper macro to get from a buffer
> > with an inline function in xfs_alloc.h to make the code easier to
> > read.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> I like it.
> 
> Giving it an RVB but are we 100% sure that there won't ever be any padding
> after the xfs_agfl structure before the bno?  I don't understand gcc
> alignment magic.

That is at least the definition of __attribute__((packed))..

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/6] xfs: remove XFS_BUF_TO_AGFL
  2020-01-30 13:33 ` [PATCH 3/6] xfs: remove XFS_BUF_TO_AGFL Christoph Hellwig
@ 2020-02-03 18:28   ` Eric Sandeen
  0 siblings, 0 replies; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 18:28 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.

Yeah, there was really no need for that.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/6] xfs: remove XFS_BUF_TO_AGI
  2020-01-30 13:33 ` [PATCH 4/6] xfs: remove XFS_BUF_TO_AGI Christoph Hellwig
@ 2020-02-03 18:30   ` Eric Sandeen
  0 siblings, 0 replies; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 18:30 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.

This is so much clearer <heart emoji> ;)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF
  2020-01-30 13:33 ` [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF Christoph Hellwig
@ 2020-02-03 18:35   ` Eric Sandeen
  2020-02-03 18:54   ` Eric Sandeen
  1 sibling, 0 replies; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 18:35 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

...

> @@ -2747,7 +2744,7 @@ xfs_alloc_log_agf(
>  		sizeof(xfs_agf_t)
>  	};
>  
> -	trace_xfs_agf(tp->t_mountp, XFS_BUF_TO_AGF(bp), fields, _RET_IP_);
> +	trace_xfs_agf(tp->t_mountp, bp->b_addr, fields, _RET_IP_);

Why is this not passing in agf?


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP
  2020-01-30 13:33 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
@ 2020-02-03 18:38   ` Eric Sandeen
  2020-02-04  6:15     ` Christoph Hellwig
  0 siblings, 1 reply; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 18:38 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

...

> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 2f60fc3c99a0..772649f4eed6 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -220,7 +220,7 @@ xfs_validate_sb_common(
>  	struct xfs_buf		*bp,
>  	struct xfs_sb		*sbp)
>  {
> -	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  	uint32_t		agcount = 0;
>  	uint32_t		rem;
>  
> @@ -681,7 +681,7 @@ xfs_sb_read_verify(
>  {
>  	struct xfs_sb		sb;
>  	struct xfs_mount	*mp = bp->b_mount;
> -	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  	int			error;
>  
>  	/*
> @@ -707,7 +707,7 @@ xfs_sb_read_verify(
>  	 * Check all the superblock fields.  Don't byteswap the xquota flags
>  	 * because _verify_common checks the on-disk values.
>  	 */
> -	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
> +	__xfs_sb_from_disk(&sb, bp->b_addr, false);

why not dsb here

>  	error = xfs_validate_sb_common(mp, bp, &sb);
>  	if (error)
>  		goto out_error;
> @@ -730,7 +730,7 @@ static void
>  xfs_sb_quiet_read_verify(
>  	struct xfs_buf	*bp)
>  {
> -	struct xfs_dsb	*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb	*dsb = bp->b_addr;
>  
>  	if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
>  		/* XFS filesystem, verify noisily! */
> @@ -748,13 +748,14 @@ xfs_sb_write_verify(
>  	struct xfs_sb		sb;
>  	struct xfs_mount	*mp = bp->b_mount;
>  	struct xfs_buf_log_item	*bip = bp->b_log_item;
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  	int			error;
>  
>  	/*
>  	 * Check all the superblock fields.  Don't byteswap the xquota flags
>  	 * because _verify_common checks the on-disk values.
>  	 */
> -	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
> +	__xfs_sb_from_disk(&sb, dsb, false);

(as you did here)

>  	error = xfs_validate_sb_common(mp, bp, &sb);
>  	if (error)
>  		goto out_error;
> @@ -766,7 +767,7 @@ xfs_sb_write_verify(
>  		return;
>  
>  	if (bip)
> -		XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
> +		dsb->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
>  
>  	xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF);
>  	return;
> @@ -927,7 +928,7 @@ xfs_log_sb(
>  	mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
>  	mp->m_sb.sb_fdblocks = percpu_counter_sum(&mp->m_fdblocks);
>  
> -	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
> +	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);

hm no "dsb" in this case ...

In any case seems like if you already have a local xfs_dsb, use that vs. bp->b_addr?

-Eric


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF
  2020-01-30 13:33 ` [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF Christoph Hellwig
  2020-02-03 18:35   ` Eric Sandeen
@ 2020-02-03 18:54   ` Eric Sandeen
  1 sibling, 0 replies; 31+ messages in thread
From: Eric Sandeen @ 2020-02-03 18:54 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 1/30/20 7:33 AM, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Sorry ignore my question, lost track of the function we were in.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP
  2020-02-03 18:38   ` Eric Sandeen
@ 2020-02-04  6:15     ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-04  6:15 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 03, 2020 at 12:38:34PM -0600, Eric Sandeen wrote:
> > @@ -707,7 +707,7 @@ xfs_sb_read_verify(
> >  	 * Check all the superblock fields.  Don't byteswap the xquota flags
> >  	 * because _verify_common checks the on-disk values.
> >  	 */
> > -	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
> > +	__xfs_sb_from_disk(&sb, bp->b_addr, false);
> 
> why not dsb here

Yes, this should just pass dsb.

> In any case seems like if you already have a local xfs_dsb, use that vs.
> bp->b_addr?

That was the planned, but I obviously missed one spot.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-01-30 13:33 ` [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl Christoph Hellwig
  2020-02-03 17:46   ` Eric Sandeen
@ 2020-02-24 22:02   ` Christoph Hellwig
  2020-02-24 22:19     ` Darrick J. Wong
  2020-02-24 22:27     ` Eric Sandeen
  1 sibling, 2 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-24 22:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, Eric Sandeen

On Thu, Jan 30, 2020 at 02:33:38PM +0100, Christoph Hellwig wrote:
> struct xfs_agfl is a header in front of the AGFL entries that exists
> for CRC enabled file systems.  For not CRC enabled file systems the AGFL
> is simply a list of agbno.  Make the CRC case similar to that by just
> using the list behind the new header.  This indirectly solves a problem
> with modern gcc versions that warn about taking addresses of packed
> structures (and we have to pack the AGFL given that gcc rounds up
> structure sizes).  Also replace the helper macro to get from a buffer
> with an inline function in xfs_alloc.h to make the code easier to
> read.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Any chance we can pick this up for 5.6 to unbreak arm OABI?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:02   ` Christoph Hellwig
@ 2020-02-24 22:19     ` Darrick J. Wong
  2020-02-24 22:21       ` Christoph Hellwig
  2020-02-24 22:27     ` Eric Sandeen
  1 sibling, 1 reply; 31+ messages in thread
From: Darrick J. Wong @ 2020-02-24 22:19 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 24, 2020 at 02:02:56PM -0800, Christoph Hellwig wrote:
> On Thu, Jan 30, 2020 at 02:33:38PM +0100, Christoph Hellwig wrote:
> > struct xfs_agfl is a header in front of the AGFL entries that exists
> > for CRC enabled file systems.  For not CRC enabled file systems the AGFL
> > is simply a list of agbno.  Make the CRC case similar to that by just
> > using the list behind the new header.  This indirectly solves a problem
> > with modern gcc versions that warn about taking addresses of packed
> > structures (and we have to pack the AGFL given that gcc rounds up
> > structure sizes).  Also replace the helper macro to get from a buffer
> > with an inline function in xfs_alloc.h to make the code easier to
> > read.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Any chance we can pick this up for 5.6 to unbreak arm OABI?

Yeah, I can do that.  Is there a Fixes: tag that goes with this?

Also, will you have a chance to respin the last patch for 5.7?

--D

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:19     ` Darrick J. Wong
@ 2020-02-24 22:21       ` Christoph Hellwig
  2020-02-24 22:27         ` Darrick J. Wong
  0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-24 22:21 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Christoph Hellwig, Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 24, 2020 at 02:19:31PM -0800, Darrick J. Wong wrote:
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Any chance we can pick this up for 5.6 to unbreak arm OABI?
> 
> Yeah, I can do that.  Is there a Fixes: tag that goes with this?

I'm not sure what to add.  I think the problem itself has actually
always been around since adding v5 fs support.  But the build break
was only caused by the addition of the BUILD_BUG_ON.

> Also, will you have a chance to respin the last patch for 5.7?

Last patch in this series?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:21       ` Christoph Hellwig
@ 2020-02-24 22:27         ` Darrick J. Wong
  2020-02-24 22:46           ` Christoph Hellwig
  0 siblings, 1 reply; 31+ messages in thread
From: Darrick J. Wong @ 2020-02-24 22:27 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 24, 2020 at 02:21:18PM -0800, Christoph Hellwig wrote:
> On Mon, Feb 24, 2020 at 02:19:31PM -0800, Darrick J. Wong wrote:
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Any chance we can pick this up for 5.6 to unbreak arm OABI?
> > 
> > Yeah, I can do that.  Is there a Fixes: tag that goes with this?
> 
> I'm not sure what to add.  I think the problem itself has actually
> always been around since adding v5 fs support.  But the build break
> was only caused by the addition of the BUILD_BUG_ON.

Hmm.  That's tricky, since in theory this should go all the way back to
the introduction of the v5 format in 3.x, but that's going to require
explicit backporting to get past all the reorganization and other things
that have happened.  We might just have to hand-backport it to the
stable kernels seeing how the macro name change will probably cause all
sorts of problems with AI backports. :/

> > Also, will you have a chance to respin the last patch for 5.7?
> 
> Last patch in this series?

Yes.  From the discussion of patch 6/6,

"+   __xfs_sb_from_disk(&sb, bp->b_addr, false);

"why not dsb here

"Yes, this should just pass dsb."

--D


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:02   ` Christoph Hellwig
  2020-02-24 22:19     ` Darrick J. Wong
@ 2020-02-24 22:27     ` Eric Sandeen
  2020-02-24 22:30       ` Christoph Hellwig
  1 sibling, 1 reply; 31+ messages in thread
From: Eric Sandeen @ 2020-02-24 22:27 UTC (permalink / raw)
  To: Christoph Hellwig, Christoph Hellwig; +Cc: linux-xfs, Eric Sandeen

On 2/24/20 2:02 PM, Christoph Hellwig wrote:
> On Thu, Jan 30, 2020 at 02:33:38PM +0100, Christoph Hellwig wrote:
>> struct xfs_agfl is a header in front of the AGFL entries that exists
>> for CRC enabled file systems.  For not CRC enabled file systems the AGFL
>> is simply a list of agbno.  Make the CRC case similar to that by just
>> using the list behind the new header.  This indirectly solves a problem
>> with modern gcc versions that warn about taking addresses of packed
>> structures (and we have to pack the AGFL given that gcc rounds up
>> structure sizes).  Also replace the helper macro to get from a buffer
>> with an inline function in xfs_alloc.h to make the code easier to
>> read.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Any chance we can pick this up for 5.6 to unbreak arm OABI?
> 

What did I miss, where's the report of actual breakage vs. 
(I thought) harmless GCC complaints?

-Eric

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:27     ` Eric Sandeen
@ 2020-02-24 22:30       ` Christoph Hellwig
  2020-02-24 22:35         ` Eric Sandeen
  0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-24 22:30 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Christoph Hellwig, Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 24, 2020 at 02:27:49PM -0800, Eric Sandeen wrote:
> On 2/24/20 2:02 PM, Christoph Hellwig wrote:
> > On Thu, Jan 30, 2020 at 02:33:38PM +0100, Christoph Hellwig wrote:
> >> struct xfs_agfl is a header in front of the AGFL entries that exists
> >> for CRC enabled file systems.  For not CRC enabled file systems the AGFL
> >> is simply a list of agbno.  Make the CRC case similar to that by just
> >> using the list behind the new header.  This indirectly solves a problem
> >> with modern gcc versions that warn about taking addresses of packed
> >> structures (and we have to pack the AGFL given that gcc rounds up
> >> structure sizes).  Also replace the helper macro to get from a buffer
> >> with an inline function in xfs_alloc.h to make the code easier to
> >> read.
> >>
> >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Any chance we can pick this up for 5.6 to unbreak arm OABI?
> > 
> 
> What did I miss, where's the report of actual breakage vs. 
> (I thought) harmless GCC complaints?

The "harmless" gcc complaint is that the kernel build errors out as
soon as XFS is enabled on arm OABI.  Which is a good thing, as the
file system would not be interoperable with other architectures if it
didn't.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:30       ` Christoph Hellwig
@ 2020-02-24 22:35         ` Eric Sandeen
  2020-02-24 22:46           ` Christoph Hellwig
  0 siblings, 1 reply; 31+ messages in thread
From: Eric Sandeen @ 2020-02-24 22:35 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Christoph Hellwig, linux-xfs, Eric Sandeen

On 2/24/20 2:30 PM, Christoph Hellwig wrote:
> On Mon, Feb 24, 2020 at 02:27:49PM -0800, Eric Sandeen wrote:
>> On 2/24/20 2:02 PM, Christoph Hellwig wrote:
>>> On Thu, Jan 30, 2020 at 02:33:38PM +0100, Christoph Hellwig wrote:
>>>> struct xfs_agfl is a header in front of the AGFL entries that exists
>>>> for CRC enabled file systems.  For not CRC enabled file systems the AGFL
>>>> is simply a list of agbno.  Make the CRC case similar to that by just
>>>> using the list behind the new header.  This indirectly solves a problem
>>>> with modern gcc versions that warn about taking addresses of packed
>>>> structures (and we have to pack the AGFL given that gcc rounds up
>>>> structure sizes).  Also replace the helper macro to get from a buffer
>>>> with an inline function in xfs_alloc.h to make the code easier to
>>>> read.
>>>>
>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>
>>> Any chance we can pick this up for 5.6 to unbreak arm OABI?
>>>
>>
>> What did I miss, where's the report of actual breakage vs. 
>> (I thought) harmless GCC complaints?
> 
> The "harmless" gcc complaint is that the kernel build errors out as
> soon as XFS is enabled on arm OABI.  Which is a good thing, as the
> file system would not be interoperable with other architectures if it
> didn't.

Not just on latest GCC?

Ok, I just hadn't seen that reported (but, I miss lots).

And the commit log doesn't mention any actual breakage; it sounds
like patch to work around a new gcc warning, not an actual fix
for anything real.

-Eric

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:27         ` Darrick J. Wong
@ 2020-02-24 22:46           ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-24 22:46 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Christoph Hellwig, Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 24, 2020 at 02:27:37PM -0800, Darrick J. Wong wrote:
> On Mon, Feb 24, 2020 at 02:21:18PM -0800, Christoph Hellwig wrote:
> > On Mon, Feb 24, 2020 at 02:19:31PM -0800, Darrick J. Wong wrote:
> > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > 
> > > > Any chance we can pick this up for 5.6 to unbreak arm OABI?
> > > 
> > > Yeah, I can do that.  Is there a Fixes: tag that goes with this?
> > 
> > I'm not sure what to add.  I think the problem itself has actually
> > always been around since adding v5 fs support.  But the build break
> > was only caused by the addition of the BUILD_BUG_ON.
> 
> Hmm.  That's tricky, since in theory this should go all the way back to
> the introduction of the v5 format in 3.x, but that's going to require
> explicit backporting to get past all the reorganization and other things
> that have happened.  We might just have to hand-backport it to the
> stable kernels seeing how the macro name change will probably cause all
> sorts of problems with AI backports. :/

So which fixes tag do you want?  Or feel free to just add the one you
feel fits best.

> > > Also, will you have a chance to respin the last patch for 5.7?
> > 
> > Last patch in this series?
> 
> Yes.  From the discussion of patch 6/6,
> 
> "+   __xfs_sb_from_disk(&sb, bp->b_addr, false);
> 
> "why not dsb here
> 
> "Yes, this should just pass dsb."

Oh.  I've actually had the respun branch on my box since a day after
that comment.  But I think it doesn't make sense until the fix in
patch one is in the baseline tree, given how many outstanding patch
series we have.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:35         ` Eric Sandeen
@ 2020-02-24 22:46           ` Christoph Hellwig
  2020-02-24 22:50             ` Eric Sandeen
  0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-24 22:46 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Christoph Hellwig, Christoph Hellwig, linux-xfs, Eric Sandeen

On Mon, Feb 24, 2020 at 02:35:08PM -0800, Eric Sandeen wrote:
> > The "harmless" gcc complaint is that the kernel build errors out as
> > soon as XFS is enabled on arm OABI.  Which is a good thing, as the
> > file system would not be interoperable with other architectures if it
> > didn't.
> 
> Not just on latest GCC?

AFAIK all versions of gcc, as that is the intent of BUILD_BUG_ON.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:46           ` Christoph Hellwig
@ 2020-02-24 22:50             ` Eric Sandeen
  2020-02-25 17:24               ` Christoph Hellwig
  0 siblings, 1 reply; 31+ messages in thread
From: Eric Sandeen @ 2020-02-24 22:50 UTC (permalink / raw)
  To: Christoph Hellwig, Eric Sandeen; +Cc: Christoph Hellwig, linux-xfs

On 2/24/20 2:46 PM, Christoph Hellwig wrote:
> On Mon, Feb 24, 2020 at 02:35:08PM -0800, Eric Sandeen wrote:
>>> The "harmless" gcc complaint is that the kernel build errors out as
>>> soon as XFS is enabled on arm OABI.  Which is a good thing, as the
>>> file system would not be interoperable with other architectures if it
>>> didn't.
>>
>> Not just on latest GCC?
> 
> AFAIK all versions of gcc, as that is the intent of BUILD_BUG_ON.

Right, makes sense.

So let's please commit a changelog that makes it clear that this
fixes an actual alignment problem and build breakage, ok?

Thanks,
-Eric


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl
  2020-02-24 22:50             ` Eric Sandeen
@ 2020-02-25 17:24               ` Christoph Hellwig
  0 siblings, 0 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-02-25 17:24 UTC (permalink / raw)
  To: Eric Sandeen
  Cc: Christoph Hellwig, Eric Sandeen, Christoph Hellwig, linux-xfs

So actually I think I was confused.  The BUILD_BUG_ON is what we had
before adding the __packed.  What we fix here іs just a really silly
warning from new gcc, which in fact only happens in xfsprogs as the
kernel decided that the warning is so damn stupid that we won't enable
it.  So no need for urgent treatment or a new commit message, sorry for
the noise.

I'll just resend the whole series with the review comments addressed.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP
  2020-03-06 14:52 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
  2020-03-10 11:34   ` Brian Foster
@ 2020-03-10 16:00   ` Eric Sandeen
  1 sibling, 0 replies; 31+ messages in thread
From: Eric Sandeen @ 2020-03-10 16:00 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: Eric Sandeen

On 3/6/20 8:52 AM, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP
  2020-03-06 14:52 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
@ 2020-03-10 11:34   ` Brian Foster
  2020-03-10 16:00   ` Eric Sandeen
  1 sibling, 0 replies; 31+ messages in thread
From: Brian Foster @ 2020-03-10 11:34 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs, Eric Sandeen

On Fri, Mar 06, 2020 at 07:52:20AM -0700, Christoph Hellwig wrote:
> Just dereference bp->b_addr directly and make the code a little
> simpler and more clear.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_ag.c         |  2 +-
>  fs/xfs/libxfs/xfs_format.h     |  1 -
>  fs/xfs/libxfs/xfs_sb.c         | 17 +++++++++--------
>  fs/xfs/scrub/agheader.c        |  2 +-
>  fs/xfs/scrub/agheader_repair.c |  2 +-
>  fs/xfs/xfs_log_recover.c       |  2 +-
>  fs/xfs/xfs_mount.c             |  2 +-
>  fs/xfs/xfs_trans.c             |  2 +-
>  8 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> index 447e363d8468..f9b8c177ebc3 100644
> --- a/fs/xfs/libxfs/xfs_ag.c
> +++ b/fs/xfs/libxfs/xfs_ag.c
> @@ -231,7 +231,7 @@ xfs_sbblock_init(
>  	struct xfs_buf		*bp,
>  	struct aghdr_init_data	*id)
>  {
> -	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  
>  	xfs_sb_to_disk(dsb, &mp->m_sb);
>  	dsb->sb_inprogress = 1;
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 03531f0f537a..81a1b7084008 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -560,7 +560,6 @@ xfs_is_quota_inode(struct xfs_sb *sbp, xfs_ino_t ino)
>  
>  #define XFS_SB_DADDR		((xfs_daddr_t)0) /* daddr in filesystem/ag */
>  #define	XFS_SB_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_SB_DADDR)
> -#define XFS_BUF_TO_SBP(bp)	((xfs_dsb_t *)((bp)->b_addr))
>  
>  #define	XFS_HDR_BLOCK(mp,d)	((xfs_agblock_t)XFS_BB_TO_FSBT(mp,d))
>  #define	XFS_DADDR_TO_FSB(mp,d)	XFS_AGB_TO_FSB(mp, \
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 2f60fc3c99a0..00266de58954 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -220,7 +220,7 @@ xfs_validate_sb_common(
>  	struct xfs_buf		*bp,
>  	struct xfs_sb		*sbp)
>  {
> -	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  	uint32_t		agcount = 0;
>  	uint32_t		rem;
>  
> @@ -681,7 +681,7 @@ xfs_sb_read_verify(
>  {
>  	struct xfs_sb		sb;
>  	struct xfs_mount	*mp = bp->b_mount;
> -	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  	int			error;
>  
>  	/*
> @@ -707,7 +707,7 @@ xfs_sb_read_verify(
>  	 * Check all the superblock fields.  Don't byteswap the xquota flags
>  	 * because _verify_common checks the on-disk values.
>  	 */
> -	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
> +	__xfs_sb_from_disk(&sb, dsb, false);
>  	error = xfs_validate_sb_common(mp, bp, &sb);
>  	if (error)
>  		goto out_error;
> @@ -730,7 +730,7 @@ static void
>  xfs_sb_quiet_read_verify(
>  	struct xfs_buf	*bp)
>  {
> -	struct xfs_dsb	*dsb = XFS_BUF_TO_SBP(bp);
> +	struct xfs_dsb	*dsb = bp->b_addr;
>  
>  	if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
>  		/* XFS filesystem, verify noisily! */
> @@ -748,13 +748,14 @@ xfs_sb_write_verify(
>  	struct xfs_sb		sb;
>  	struct xfs_mount	*mp = bp->b_mount;
>  	struct xfs_buf_log_item	*bip = bp->b_log_item;
> +	struct xfs_dsb		*dsb = bp->b_addr;
>  	int			error;
>  
>  	/*
>  	 * Check all the superblock fields.  Don't byteswap the xquota flags
>  	 * because _verify_common checks the on-disk values.
>  	 */
> -	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
> +	__xfs_sb_from_disk(&sb, dsb, false);
>  	error = xfs_validate_sb_common(mp, bp, &sb);
>  	if (error)
>  		goto out_error;
> @@ -766,7 +767,7 @@ xfs_sb_write_verify(
>  		return;
>  
>  	if (bip)
> -		XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
> +		dsb->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
>  
>  	xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF);
>  	return;
> @@ -927,7 +928,7 @@ xfs_log_sb(
>  	mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
>  	mp->m_sb.sb_fdblocks = percpu_counter_sum(&mp->m_fdblocks);
>  
> -	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
> +	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
>  	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_SB_BUF);
>  	xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsb) - 1);
>  }
> @@ -1007,7 +1008,7 @@ xfs_update_secondary_sbs(
>  		bp->b_ops = &xfs_sb_buf_ops;
>  		xfs_buf_oneshot(bp);
>  		xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
> -		xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
> +		xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
>  		xfs_buf_delwri_queue(bp, &buffer_list);
>  		xfs_buf_relse(bp);
>  
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index 163478855e7b..e9bcf1faa183 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
> @@ -92,7 +92,7 @@ xchk_superblock(
>  	if (!xchk_process_error(sc, agno, XFS_SB_BLOCK(mp), &error))
>  		return error;
>  
> -	sb = XFS_BUF_TO_SBP(bp);
> +	sb = bp->b_addr;
>  
>  	/*
>  	 * Verify the geometries match.  Fields that are permanently
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index c801f5892210..31dbb5d556fd 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -49,7 +49,7 @@ xrep_superblock(
>  
>  	/* Copy AG 0's superblock to this one. */
>  	xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
> -	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
> +	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
>  
>  	/* Write this to disk. */
>  	xfs_trans_buf_set_type(sc->tp, bp, XFS_BLFT_SB_BUF);
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index b6cf99f7153f..6abc0863c9c3 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -5636,7 +5636,7 @@ xlog_do_recover(
>  
>  	/* Convert superblock from on-disk format */
>  	sbp = &mp->m_sb;
> -	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
> +	xfs_sb_from_disk(sbp, bp->b_addr);
>  	xfs_buf_relse(bp);
>  
>  	/* re-initialise in-core superblock and geometry structures */
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 56efe140c923..c5513e5a226a 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -310,7 +310,7 @@ xfs_readsb(
>  	/*
>  	 * Initialize the mount structure from the superblock.
>  	 */
> -	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
> +	xfs_sb_from_disk(sbp, bp->b_addr);
>  
>  	/*
>  	 * If we haven't validated the superblock, do so now before we try
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 3b208f9a865c..73c534093f09 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -450,7 +450,7 @@ xfs_trans_apply_sb_deltas(
>  	int		whole = 0;
>  
>  	bp = xfs_trans_getsb(tp, tp->t_mountp);
> -	sbp = XFS_BUF_TO_SBP(bp);
> +	sbp = bp->b_addr;
>  
>  	/*
>  	 * Check that superblock mods match the mods made to AGF counters.
> -- 
> 2.24.1
> 


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP
  2020-03-06 14:52 agfl and related cleanups v2 Christoph Hellwig
@ 2020-03-06 14:52 ` Christoph Hellwig
  2020-03-10 11:34   ` Brian Foster
  2020-03-10 16:00   ` Eric Sandeen
  0 siblings, 2 replies; 31+ messages in thread
From: Christoph Hellwig @ 2020-03-06 14:52 UTC (permalink / raw)
  To: linux-xfs; +Cc: Eric Sandeen

Just dereference bp->b_addr directly and make the code a little
simpler and more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_ag.c         |  2 +-
 fs/xfs/libxfs/xfs_format.h     |  1 -
 fs/xfs/libxfs/xfs_sb.c         | 17 +++++++++--------
 fs/xfs/scrub/agheader.c        |  2 +-
 fs/xfs/scrub/agheader_repair.c |  2 +-
 fs/xfs/xfs_log_recover.c       |  2 +-
 fs/xfs/xfs_mount.c             |  2 +-
 fs/xfs/xfs_trans.c             |  2 +-
 8 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 447e363d8468..f9b8c177ebc3 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -231,7 +231,7 @@ xfs_sbblock_init(
 	struct xfs_buf		*bp,
 	struct aghdr_init_data	*id)
 {
-	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb		*dsb = bp->b_addr;
 
 	xfs_sb_to_disk(dsb, &mp->m_sb);
 	dsb->sb_inprogress = 1;
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 03531f0f537a..81a1b7084008 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -560,7 +560,6 @@ xfs_is_quota_inode(struct xfs_sb *sbp, xfs_ino_t ino)
 
 #define XFS_SB_DADDR		((xfs_daddr_t)0) /* daddr in filesystem/ag */
 #define	XFS_SB_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_SB_DADDR)
-#define XFS_BUF_TO_SBP(bp)	((xfs_dsb_t *)((bp)->b_addr))
 
 #define	XFS_HDR_BLOCK(mp,d)	((xfs_agblock_t)XFS_BB_TO_FSBT(mp,d))
 #define	XFS_DADDR_TO_FSB(mp,d)	XFS_AGB_TO_FSB(mp, \
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 2f60fc3c99a0..00266de58954 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -220,7 +220,7 @@ xfs_validate_sb_common(
 	struct xfs_buf		*bp,
 	struct xfs_sb		*sbp)
 {
-	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb		*dsb = bp->b_addr;
 	uint32_t		agcount = 0;
 	uint32_t		rem;
 
@@ -681,7 +681,7 @@ xfs_sb_read_verify(
 {
 	struct xfs_sb		sb;
 	struct xfs_mount	*mp = bp->b_mount;
-	struct xfs_dsb		*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb		*dsb = bp->b_addr;
 	int			error;
 
 	/*
@@ -707,7 +707,7 @@ xfs_sb_read_verify(
 	 * Check all the superblock fields.  Don't byteswap the xquota flags
 	 * because _verify_common checks the on-disk values.
 	 */
-	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
+	__xfs_sb_from_disk(&sb, dsb, false);
 	error = xfs_validate_sb_common(mp, bp, &sb);
 	if (error)
 		goto out_error;
@@ -730,7 +730,7 @@ static void
 xfs_sb_quiet_read_verify(
 	struct xfs_buf	*bp)
 {
-	struct xfs_dsb	*dsb = XFS_BUF_TO_SBP(bp);
+	struct xfs_dsb	*dsb = bp->b_addr;
 
 	if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
 		/* XFS filesystem, verify noisily! */
@@ -748,13 +748,14 @@ xfs_sb_write_verify(
 	struct xfs_sb		sb;
 	struct xfs_mount	*mp = bp->b_mount;
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
+	struct xfs_dsb		*dsb = bp->b_addr;
 	int			error;
 
 	/*
 	 * Check all the superblock fields.  Don't byteswap the xquota flags
 	 * because _verify_common checks the on-disk values.
 	 */
-	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
+	__xfs_sb_from_disk(&sb, dsb, false);
 	error = xfs_validate_sb_common(mp, bp, &sb);
 	if (error)
 		goto out_error;
@@ -766,7 +767,7 @@ xfs_sb_write_verify(
 		return;
 
 	if (bip)
-		XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
+		dsb->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
 	xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF);
 	return;
@@ -927,7 +928,7 @@ xfs_log_sb(
 	mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
 	mp->m_sb.sb_fdblocks = percpu_counter_sum(&mp->m_fdblocks);
 
-	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_SB_BUF);
 	xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsb) - 1);
 }
@@ -1007,7 +1008,7 @@ xfs_update_secondary_sbs(
 		bp->b_ops = &xfs_sb_buf_ops;
 		xfs_buf_oneshot(bp);
 		xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
-		xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+		xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
 		xfs_buf_delwri_queue(bp, &buffer_list);
 		xfs_buf_relse(bp);
 
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 163478855e7b..e9bcf1faa183 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -92,7 +92,7 @@ xchk_superblock(
 	if (!xchk_process_error(sc, agno, XFS_SB_BLOCK(mp), &error))
 		return error;
 
-	sb = XFS_BUF_TO_SBP(bp);
+	sb = bp->b_addr;
 
 	/*
 	 * Verify the geometries match.  Fields that are permanently
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index c801f5892210..31dbb5d556fd 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -49,7 +49,7 @@ xrep_superblock(
 
 	/* Copy AG 0's superblock to this one. */
 	xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
-	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
 
 	/* Write this to disk. */
 	xfs_trans_buf_set_type(sc->tp, bp, XFS_BLFT_SB_BUF);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index b6cf99f7153f..6abc0863c9c3 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -5636,7 +5636,7 @@ xlog_do_recover(
 
 	/* Convert superblock from on-disk format */
 	sbp = &mp->m_sb;
-	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
+	xfs_sb_from_disk(sbp, bp->b_addr);
 	xfs_buf_relse(bp);
 
 	/* re-initialise in-core superblock and geometry structures */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 56efe140c923..c5513e5a226a 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -310,7 +310,7 @@ xfs_readsb(
 	/*
 	 * Initialize the mount structure from the superblock.
 	 */
-	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
+	xfs_sb_from_disk(sbp, bp->b_addr);
 
 	/*
 	 * If we haven't validated the superblock, do so now before we try
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 3b208f9a865c..73c534093f09 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -450,7 +450,7 @@ xfs_trans_apply_sb_deltas(
 	int		whole = 0;
 
 	bp = xfs_trans_getsb(tp, tp->t_mountp);
-	sbp = XFS_BUF_TO_SBP(bp);
+	sbp = bp->b_addr;
 
 	/*
 	 * Check that superblock mods match the mods made to AGF counters.
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2020-03-10 16:00 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-30 13:33 agfl and related cleanups Christoph Hellwig
2020-01-30 13:33 ` [PATCH 1/6] xfs: remove the agfl_bno member from struct xfs_agfl Christoph Hellwig
2020-02-03 17:46   ` Eric Sandeen
2020-02-03 17:47     ` Christoph Hellwig
2020-02-24 22:02   ` Christoph Hellwig
2020-02-24 22:19     ` Darrick J. Wong
2020-02-24 22:21       ` Christoph Hellwig
2020-02-24 22:27         ` Darrick J. Wong
2020-02-24 22:46           ` Christoph Hellwig
2020-02-24 22:27     ` Eric Sandeen
2020-02-24 22:30       ` Christoph Hellwig
2020-02-24 22:35         ` Eric Sandeen
2020-02-24 22:46           ` Christoph Hellwig
2020-02-24 22:50             ` Eric Sandeen
2020-02-25 17:24               ` Christoph Hellwig
2020-01-30 13:33 ` [PATCH 2/6] xfs: remove the xfs_agfl_t typedef Christoph Hellwig
2020-02-03 17:46   ` Eric Sandeen
2020-01-30 13:33 ` [PATCH 3/6] xfs: remove XFS_BUF_TO_AGFL Christoph Hellwig
2020-02-03 18:28   ` Eric Sandeen
2020-01-30 13:33 ` [PATCH 4/6] xfs: remove XFS_BUF_TO_AGI Christoph Hellwig
2020-02-03 18:30   ` Eric Sandeen
2020-01-30 13:33 ` [PATCH 5/6] xfs: remove XFS_BUF_TO_AGF Christoph Hellwig
2020-02-03 18:35   ` Eric Sandeen
2020-02-03 18:54   ` Eric Sandeen
2020-01-30 13:33 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
2020-02-03 18:38   ` Eric Sandeen
2020-02-04  6:15     ` Christoph Hellwig
2020-02-03  9:12 ` agfl and related cleanups Chandan Rajendra
2020-03-06 14:52 agfl and related cleanups v2 Christoph Hellwig
2020-03-06 14:52 ` [PATCH 6/6] xfs: remove XFS_BUF_TO_SBP Christoph Hellwig
2020-03-10 11:34   ` Brian Foster
2020-03-10 16:00   ` Eric Sandeen

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