[PATCH 0/5] xfs-5.0: miscellaneous cleanups

[PATCH 0/5] xfs-5.0: miscellaneous cleanups

[Darrick J. Wong]

Hi all,

Here are some cleanups I made against the rmap code and the inode
geometry code, which came up as part of review comments on the scrub
inode fixes posted earlier.

The first cleanup constifies the struct xfs_owner_inode pointers that
get passed around to the rmap calls.  This enables the second cleanup,
which is to create some global const XFS_RMAP_OINFO_* objects for static
fs metadata.  This enables us to reduce stack usage here and there, and
remove a surprising amount of oinfo setup clutter in the scrub code.

The last three patches add some inode <-> block conversion macros and
adds precomputed inode cluster size and alignment geometry information
in struct xfs_mount instead of calculating it over and over.

These cleanups are being submitted ahead of v2 of the scrub inode fixes
and the repair patches to be sent later.

Comments and questions are, as always, welcome.

--D

[PATCH 1/5] xfs: const-ify xfs_owner_info arguments

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Only certain functions actually change the contents of an
xfs_owner_info; the rest can accept a const struct pointer.  This will
enable us to save stack space by hoisting static owner info types to
be const global variables.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_alloc.c  |    4 -
 fs/xfs/libxfs/xfs_alloc.h  |    4 -
 fs/xfs/libxfs/xfs_bmap.c   |    2 
 fs/xfs/libxfs/xfs_bmap.h   |    4 -
 fs/xfs/libxfs/xfs_rmap.c   |  212 ++++++++++++++++++++++----------------------
 fs/xfs/libxfs/xfs_rmap.h   |   12 +-
 fs/xfs/scrub/btree.c       |   45 +++++----
 fs/xfs/scrub/btree.h       |   22 ++---
 fs/xfs/scrub/common.c      |   14 +--
 fs/xfs/scrub/common.h      |    2 
 fs/xfs/scrub/refcount.c    |   11 +-
 fs/xfs/scrub/repair.c      |   50 +++++-----
 fs/xfs/scrub/repair.h      |    7 +
 fs/xfs/scrub/rmap.c        |   30 +++---
 fs/xfs/scrub/scrub.h       |    4 -
 fs/xfs/xfs_trace.h         |    4 -
 fs/xfs/xfs_trans.h         |    3 -
 fs/xfs/xfs_trans_extfree.c |    2 
 18 files changed, 218 insertions(+), 214 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index e1c0c0d2f1b0..bce46fa0ee38 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1699,7 +1699,7 @@ xfs_free_ag_extent(
 	xfs_agnumber_t		agno,
 	xfs_agblock_t		bno,
 	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	enum xfs_ag_resv_type	type)
 {
 	xfs_btree_cur_t	*bno_cur;	/* cursor for by-block btree */
@@ -3013,7 +3013,7 @@ __xfs_free_extent(
 	struct xfs_trans	*tp,	/* transaction pointer */
 	xfs_fsblock_t		bno,	/* starting block number of extent */
 	xfs_extlen_t		len,	/* length of extent */
-	struct xfs_owner_info	*oinfo,	/* extent owner */
+	const struct xfs_owner_info	*oinfo,	/* extent owner */
 	enum xfs_ag_resv_type	type,	/* block reservation type */
 	bool			skip_discard)
 {
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 00cd5ec4cb6b..d6ed5d2c07c2 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -182,7 +182,7 @@ __xfs_free_extent(
 	struct xfs_trans	*tp,	/* transaction pointer */
 	xfs_fsblock_t		bno,	/* starting block number of extent */
 	xfs_extlen_t		len,	/* length of extent */
-	struct xfs_owner_info	*oinfo,	/* extent owner */
+	const struct xfs_owner_info	*oinfo,	/* extent owner */
 	enum xfs_ag_resv_type	type,	/* block reservation type */
 	bool			skip_discard);
 
@@ -191,7 +191,7 @@ xfs_free_extent(
 	struct xfs_trans	*tp,
 	xfs_fsblock_t		bno,
 	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	enum xfs_ag_resv_type	type)
 {
 	return __xfs_free_extent(tp, bno, len, oinfo, type, false);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 74d7228e755b..60abaa2e8146 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -536,7 +536,7 @@ __xfs_bmap_add_free(
 	struct xfs_trans		*tp,
 	xfs_fsblock_t			bno,
 	xfs_filblks_t			len,
-	struct xfs_owner_info		*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	bool				skip_discard)
 {
 	struct xfs_extent_free_item	*new;		/* new element */
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 488dc8860fd7..09d3ea97cc15 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -186,7 +186,7 @@ int	xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
 int	xfs_bmap_set_attrforkoff(struct xfs_inode *ip, int size, int *version);
 void	xfs_bmap_local_to_extents_empty(struct xfs_inode *ip, int whichfork);
 void	__xfs_bmap_add_free(struct xfs_trans *tp, xfs_fsblock_t bno,
-		xfs_filblks_t len, struct xfs_owner_info *oinfo,
+		xfs_filblks_t len, const struct xfs_owner_info *oinfo,
 		bool skip_discard);
 void	xfs_bmap_compute_maxlevels(struct xfs_mount *mp, int whichfork);
 int	xfs_bmap_first_unused(struct xfs_trans *tp, struct xfs_inode *ip,
@@ -234,7 +234,7 @@ xfs_bmap_add_free(
 	struct xfs_trans		*tp,
 	xfs_fsblock_t			bno,
 	xfs_filblks_t			len,
-	struct xfs_owner_info		*oinfo)
+	const struct xfs_owner_info	*oinfo)
 {
 	__xfs_bmap_add_free(tp, bno, len, oinfo, false);
 }
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index 245af452840e..e7d06295338d 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -458,21 +458,21 @@ xfs_rmap_free_check_owner(
  */
 STATIC int
 xfs_rmap_unmap(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	bool			unwritten,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	bool				unwritten,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = cur->bc_mp;
-	struct xfs_rmap_irec	ltrec;
-	uint64_t		ltoff;
-	int			error = 0;
-	int			i;
-	uint64_t		owner;
-	uint64_t		offset;
-	unsigned int		flags;
-	bool			ignore_off;
+	struct xfs_mount		*mp = cur->bc_mp;
+	struct xfs_rmap_irec		ltrec;
+	uint64_t			ltoff;
+	int				error = 0;
+	int				i;
+	uint64_t			owner;
+	uint64_t			offset;
+	unsigned int			flags;
+	bool				ignore_off;
 
 	xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
 	ignore_off = XFS_RMAP_NON_INODE_OWNER(owner) ||
@@ -653,16 +653,16 @@ xfs_rmap_unmap(
  */
 int
 xfs_rmap_free(
-	struct xfs_trans	*tp,
-	struct xfs_buf		*agbp,
-	xfs_agnumber_t		agno,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_trans		*tp,
+	struct xfs_buf			*agbp,
+	xfs_agnumber_t			agno,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = tp->t_mountp;
-	struct xfs_btree_cur	*cur;
-	int			error;
+	struct xfs_mount		*mp = tp->t_mountp;
+	struct xfs_btree_cur		*cur;
+	int				error;
 
 	if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
 		return 0;
@@ -710,23 +710,23 @@ xfs_rmap_is_mergeable(
  */
 STATIC int
 xfs_rmap_map(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	bool			unwritten,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	bool				unwritten,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = cur->bc_mp;
-	struct xfs_rmap_irec	ltrec;
-	struct xfs_rmap_irec	gtrec;
-	int			have_gt;
-	int			have_lt;
-	int			error = 0;
-	int			i;
-	uint64_t		owner;
-	uint64_t		offset;
-	unsigned int		flags = 0;
-	bool			ignore_off;
+	struct xfs_mount		*mp = cur->bc_mp;
+	struct xfs_rmap_irec		ltrec;
+	struct xfs_rmap_irec		gtrec;
+	int				have_gt;
+	int				have_lt;
+	int				error = 0;
+	int				i;
+	uint64_t			owner;
+	uint64_t			offset;
+	unsigned int			flags = 0;
+	bool				ignore_off;
 
 	xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
 	ASSERT(owner != 0);
@@ -890,16 +890,16 @@ xfs_rmap_map(
  */
 int
 xfs_rmap_alloc(
-	struct xfs_trans	*tp,
-	struct xfs_buf		*agbp,
-	xfs_agnumber_t		agno,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_trans		*tp,
+	struct xfs_buf			*agbp,
+	xfs_agnumber_t			agno,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = tp->t_mountp;
-	struct xfs_btree_cur	*cur;
-	int			error;
+	struct xfs_mount		*mp = tp->t_mountp;
+	struct xfs_btree_cur		*cur;
+	int				error;
 
 	if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
 		return 0;
@@ -929,16 +929,16 @@ xfs_rmap_alloc(
  */
 STATIC int
 xfs_rmap_convert(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	bool			unwritten,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	bool				unwritten,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = cur->bc_mp;
-	struct xfs_rmap_irec	r[4];	/* neighbor extent entries */
-					/* left is 0, right is 1, prev is 2 */
-					/* new is 3 */
+	struct xfs_mount		*mp = cur->bc_mp;
+	struct xfs_rmap_irec		r[4];	/* neighbor extent entries */
+						/* left is 0, right is 1, */
+						/* prev is 2, new is 3 */
 	uint64_t		owner;
 	uint64_t		offset;
 	uint64_t		new_endoff;
@@ -1354,16 +1354,16 @@ xfs_rmap_convert(
  */
 STATIC int
 xfs_rmap_convert_shared(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	bool			unwritten,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	bool				unwritten,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = cur->bc_mp;
-	struct xfs_rmap_irec	r[4];	/* neighbor extent entries */
-					/* left is 0, right is 1, prev is 2 */
-					/* new is 3 */
+	struct xfs_mount		*mp = cur->bc_mp;
+	struct xfs_rmap_irec		r[4];	/* neighbor extent entries */
+						/* left is 0, right is 1, */
+						/* prev is 2, new is 3 */
 	uint64_t		owner;
 	uint64_t		offset;
 	uint64_t		new_endoff;
@@ -1743,20 +1743,20 @@ xfs_rmap_convert_shared(
  */
 STATIC int
 xfs_rmap_unmap_shared(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	bool			unwritten,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	bool				unwritten,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = cur->bc_mp;
-	struct xfs_rmap_irec	ltrec;
-	uint64_t		ltoff;
-	int			error = 0;
-	int			i;
-	uint64_t		owner;
-	uint64_t		offset;
-	unsigned int		flags;
+	struct xfs_mount		*mp = cur->bc_mp;
+	struct xfs_rmap_irec		ltrec;
+	uint64_t			ltoff;
+	int				error = 0;
+	int				i;
+	uint64_t			owner;
+	uint64_t			offset;
+	unsigned int			flags;
 
 	xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
 	if (unwritten)
@@ -1905,22 +1905,22 @@ xfs_rmap_unmap_shared(
  */
 STATIC int
 xfs_rmap_map_shared(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	bool			unwritten,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	bool				unwritten,
+	const struct xfs_owner_info	*oinfo)
 {
-	struct xfs_mount	*mp = cur->bc_mp;
-	struct xfs_rmap_irec	ltrec;
-	struct xfs_rmap_irec	gtrec;
-	int			have_gt;
-	int			have_lt;
-	int			error = 0;
-	int			i;
-	uint64_t		owner;
-	uint64_t		offset;
-	unsigned int		flags = 0;
+	struct xfs_mount		*mp = cur->bc_mp;
+	struct xfs_rmap_irec		ltrec;
+	struct xfs_rmap_irec		gtrec;
+	int				have_gt;
+	int				have_lt;
+	int				error = 0;
+	int				i;
+	uint64_t			owner;
+	uint64_t			offset;
+	unsigned int			flags = 0;
 
 	xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
 	if (unwritten)
@@ -2459,18 +2459,18 @@ xfs_rmap_has_record(
  */
 int
 xfs_rmap_record_exists(
-	struct xfs_btree_cur	*cur,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo,
-	bool			*has_rmap)
+	struct xfs_btree_cur		*cur,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	const struct xfs_owner_info	*oinfo,
+	bool				*has_rmap)
 {
-	uint64_t		owner;
-	uint64_t		offset;
-	unsigned int		flags;
-	int			has_record;
-	struct xfs_rmap_irec	irec;
-	int			error;
+	uint64_t			owner;
+	uint64_t			offset;
+	unsigned int			flags;
+	int				has_record;
+	struct xfs_rmap_irec		irec;
+	int				error;
 
 	xfs_owner_info_unpack(oinfo, &owner, &offset, &flags);
 	ASSERT(XFS_RMAP_NON_INODE_OWNER(owner) ||
@@ -2530,7 +2530,7 @@ xfs_rmap_has_other_keys(
 	struct xfs_btree_cur		*cur,
 	xfs_agblock_t			bno,
 	xfs_extlen_t			len,
-	struct xfs_owner_info		*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	bool				*has_rmap)
 {
 	struct xfs_rmap_irec		low = {0};
diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h
index 157dc722ad35..00e38505037f 100644
--- a/fs/xfs/libxfs/xfs_rmap.h
+++ b/fs/xfs/libxfs/xfs_rmap.h
@@ -52,7 +52,7 @@ xfs_rmap_skip_owner_update(
 
 static inline bool
 xfs_rmap_should_skip_owner_update(
-	struct xfs_owner_info	*oi)
+	const struct xfs_owner_info	*oi)
 {
 	return oi->oi_owner == XFS_RMAP_OWN_NULL;
 }
@@ -103,7 +103,7 @@ xfs_rmap_irec_offset_unpack(
 
 static inline void
 xfs_owner_info_unpack(
-	struct xfs_owner_info	*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	uint64_t		*owner,
 	uint64_t		*offset,
 	unsigned int		*flags)
@@ -137,10 +137,10 @@ xfs_owner_info_pack(
 
 int xfs_rmap_alloc(struct xfs_trans *tp, struct xfs_buf *agbp,
 		   xfs_agnumber_t agno, xfs_agblock_t bno, xfs_extlen_t len,
-		   struct xfs_owner_info *oinfo);
+		   const struct xfs_owner_info *oinfo);
 int xfs_rmap_free(struct xfs_trans *tp, struct xfs_buf *agbp,
 		  xfs_agnumber_t agno, xfs_agblock_t bno, xfs_extlen_t len,
-		  struct xfs_owner_info *oinfo);
+		  const struct xfs_owner_info *oinfo);
 
 int xfs_rmap_lookup_le(struct xfs_btree_cur *cur, xfs_agblock_t bno,
 		xfs_extlen_t len, uint64_t owner, uint64_t offset,
@@ -218,10 +218,10 @@ int xfs_rmap_btrec_to_irec(union xfs_btree_rec *rec,
 int xfs_rmap_has_record(struct xfs_btree_cur *cur, xfs_agblock_t bno,
 		xfs_extlen_t len, bool *exists);
 int xfs_rmap_record_exists(struct xfs_btree_cur *cur, xfs_agblock_t bno,
-		xfs_extlen_t len, struct xfs_owner_info *oinfo,
+		xfs_extlen_t len, const struct xfs_owner_info *oinfo,
 		bool *has_rmap);
 int xfs_rmap_has_other_keys(struct xfs_btree_cur *cur, xfs_agblock_t bno,
-		xfs_extlen_t len, struct xfs_owner_info *oinfo,
+		xfs_extlen_t len, const struct xfs_owner_info *oinfo,
 		bool *has_rmap);
 int xfs_rmap_map_raw(struct xfs_btree_cur *cur, struct xfs_rmap_irec *rmap);
 
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index 4ae959f7ad2c..6f94d1f7322d 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -583,31 +583,32 @@ xchk_btree_block_keys(
  */
 int
 xchk_btree(
-	struct xfs_scrub	*sc,
-	struct xfs_btree_cur	*cur,
-	xchk_btree_rec_fn	scrub_fn,
-	struct xfs_owner_info	*oinfo,
-	void			*private)
+	struct xfs_scrub		*sc,
+	struct xfs_btree_cur		*cur,
+	xchk_btree_rec_fn		scrub_fn,
+	const struct xfs_owner_info	*oinfo,
+	void				*private)
 {
-	struct xchk_btree	bs = { NULL };
-	union xfs_btree_ptr	ptr;
-	union xfs_btree_ptr	*pp;
-	union xfs_btree_rec	*recp;
-	struct xfs_btree_block	*block;
-	int			level;
-	struct xfs_buf		*bp;
-	struct check_owner	*co;
-	struct check_owner	*n;
-	int			i;
-	int			error = 0;
+	struct xchk_btree		bs = {
+		.cur			= cur,
+		.scrub_rec		= scrub_fn,
+		.oinfo			= oinfo,
+		.firstrec		= true,
+		.private		= private,
+		.sc			= sc,
+	};
+	union xfs_btree_ptr		ptr;
+	union xfs_btree_ptr		*pp;
+	union xfs_btree_rec		*recp;
+	struct xfs_btree_block		*block;
+	int				level;
+	struct xfs_buf			*bp;
+	struct check_owner		*co;
+	struct check_owner		*n;
+	int				i;
+	int				error = 0;
 
 	/* Initialize scrub state */
-	bs.cur = cur;
-	bs.scrub_rec = scrub_fn;
-	bs.oinfo = oinfo;
-	bs.firstrec = true;
-	bs.private = private;
-	bs.sc = sc;
 	for (i = 0; i < XFS_BTREE_MAXLEVELS; i++)
 		bs.firstkey[i] = true;
 	INIT_LIST_HEAD(&bs.to_check);
diff --git a/fs/xfs/scrub/btree.h b/fs/xfs/scrub/btree.h
index aada763cd006..5572e475f8ed 100644
--- a/fs/xfs/scrub/btree.h
+++ b/fs/xfs/scrub/btree.h
@@ -31,21 +31,21 @@ typedef int (*xchk_btree_rec_fn)(
 
 struct xchk_btree {
 	/* caller-provided scrub state */
-	struct xfs_scrub	*sc;
-	struct xfs_btree_cur	*cur;
-	xchk_btree_rec_fn	scrub_rec;
-	struct xfs_owner_info	*oinfo;
-	void			*private;
+	struct xfs_scrub		*sc;
+	struct xfs_btree_cur		*cur;
+	xchk_btree_rec_fn		scrub_rec;
+	const struct xfs_owner_info	*oinfo;
+	void				*private;
 
 	/* internal scrub state */
-	union xfs_btree_rec	lastrec;
-	bool			firstrec;
-	union xfs_btree_key	lastkey[XFS_BTREE_MAXLEVELS];
-	bool			firstkey[XFS_BTREE_MAXLEVELS];
-	struct list_head	to_check;
+	union xfs_btree_rec		lastrec;
+	bool				firstrec;
+	union xfs_btree_key		lastkey[XFS_BTREE_MAXLEVELS];
+	bool				firstkey[XFS_BTREE_MAXLEVELS];
+	struct list_head		to_check;
 };
 int xchk_btree(struct xfs_scrub *sc, struct xfs_btree_cur *cur,
-		xchk_btree_rec_fn scrub_fn, struct xfs_owner_info *oinfo,
+		xchk_btree_rec_fn scrub_fn, const struct xfs_owner_info *oinfo,
 		void *private);
 
 #endif /* __XFS_SCRUB_BTREE_H__ */
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 346b02abccf7..0c54ff55b901 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -313,8 +313,8 @@ xchk_set_incomplete(
  */
 
 struct xchk_rmap_ownedby_info {
-	struct xfs_owner_info	*oinfo;
-	xfs_filblks_t		*blocks;
+	const struct xfs_owner_info	*oinfo;
+	xfs_filblks_t			*blocks;
 };
 
 STATIC int
@@ -347,15 +347,15 @@ int
 xchk_count_rmap_ownedby_ag(
 	struct xfs_scrub		*sc,
 	struct xfs_btree_cur		*cur,
-	struct xfs_owner_info		*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	xfs_filblks_t			*blocks)
 {
-	struct xchk_rmap_ownedby_info	sroi;
+	struct xchk_rmap_ownedby_info	sroi = {
+		.oinfo			= oinfo,
+		.blocks			= blocks,
+	};
 
-	sroi.oinfo = oinfo;
 	*blocks = 0;
-	sroi.blocks = blocks;
-
 	return xfs_rmap_query_all(cur, xchk_count_rmap_ownedby_irec,
 			&sroi);
 }
diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index 2d4324d12f9a..e26a430bd466 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -116,7 +116,7 @@ int xchk_ag_read_headers(struct xfs_scrub *sc, xfs_agnumber_t agno,
 void xchk_ag_btcur_free(struct xchk_ag *sa);
 int xchk_ag_btcur_init(struct xfs_scrub *sc, struct xchk_ag *sa);
 int xchk_count_rmap_ownedby_ag(struct xfs_scrub *sc, struct xfs_btree_cur *cur,
-		struct xfs_owner_info *oinfo, xfs_filblks_t *blocks);
+		const struct xfs_owner_info *oinfo, xfs_filblks_t *blocks);
 
 int xchk_setup_ag_btree(struct xfs_scrub *sc, struct xfs_inode *ip,
 		bool force_log);
diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
index e8c82b026083..b7ade620acee 100644
--- a/fs/xfs/scrub/refcount.c
+++ b/fs/xfs/scrub/refcount.c
@@ -383,9 +383,9 @@ xchk_refcountbt_rec(
 STATIC void
 xchk_refcount_xref_rmap(
 	struct xfs_scrub	*sc,
-	struct xfs_owner_info	*oinfo,
 	xfs_filblks_t		cow_blocks)
 {
+	struct xfs_owner_info	oinfo;
 	xfs_extlen_t		refcbt_blocks = 0;
 	xfs_filblks_t		blocks;
 	int			error;
@@ -394,10 +394,11 @@ xchk_refcount_xref_rmap(
 		return;
 
 	/* Check that we saw as many refcbt blocks as the rmap knows about. */
+	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
 	error = xfs_btree_count_blocks(sc->sa.refc_cur, &refcbt_blocks);
 	if (!xchk_btree_process_error(sc, sc->sa.refc_cur, 0, &error))
 		return;
-	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, oinfo,
+	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
 			&blocks);
 	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
 		return;
@@ -405,8 +406,8 @@ xchk_refcount_xref_rmap(
 		xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
 
 	/* Check that we saw as many cow blocks as the rmap knows about. */
-	xfs_rmap_ag_owner(oinfo, XFS_RMAP_OWN_COW);
-	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, oinfo,
+	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_COW);
+	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
 			&blocks);
 	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
 		return;
@@ -429,7 +430,7 @@ xchk_refcountbt(
 	if (error)
 		return error;
 
-	xchk_refcount_xref_rmap(sc, &oinfo, cow_blocks);
+	xchk_refcount_xref_rmap(sc, cow_blocks);
 
 	return 0;
 }
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 4fc0a5ea7673..90ae9e173de7 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -299,14 +299,14 @@ xrep_calc_ag_resblks(
 /* Allocate a block in an AG. */
 int
 xrep_alloc_ag_block(
-	struct xfs_scrub	*sc,
-	struct xfs_owner_info	*oinfo,
-	xfs_fsblock_t		*fsbno,
-	enum xfs_ag_resv_type	resv)
+	struct xfs_scrub		*sc,
+	const struct xfs_owner_info	*oinfo,
+	xfs_fsblock_t			*fsbno,
+	enum xfs_ag_resv_type		resv)
 {
-	struct xfs_alloc_arg	args = {0};
-	xfs_agblock_t		bno;
-	int			error;
+	struct xfs_alloc_arg		args = {0};
+	xfs_agblock_t			bno;
+	int				error;
 
 	switch (resv) {
 	case XFS_AG_RESV_AGFL:
@@ -538,17 +538,17 @@ xrep_put_freelist(
 /* Dispose of a single block. */
 STATIC int
 xrep_reap_block(
-	struct xfs_scrub	*sc,
-	xfs_fsblock_t		fsbno,
-	struct xfs_owner_info	*oinfo,
-	enum xfs_ag_resv_type	resv)
+	struct xfs_scrub		*sc,
+	xfs_fsblock_t			fsbno,
+	const struct xfs_owner_info	*oinfo,
+	enum xfs_ag_resv_type		resv)
 {
-	struct xfs_btree_cur	*cur;
-	struct xfs_buf		*agf_bp = NULL;
-	xfs_agnumber_t		agno;
-	xfs_agblock_t		agbno;
-	bool			has_other_rmap;
-	int			error;
+	struct xfs_btree_cur		*cur;
+	struct xfs_buf			*agf_bp = NULL;
+	xfs_agnumber_t			agno;
+	xfs_agblock_t			agbno;
+	bool				has_other_rmap;
+	int				error;
 
 	agno = XFS_FSB_TO_AGNO(sc->mp, fsbno);
 	agbno = XFS_FSB_TO_AGBNO(sc->mp, fsbno);
@@ -612,15 +612,15 @@ xrep_reap_block(
 /* Dispose of every block of every extent in the bitmap. */
 int
 xrep_reap_extents(
-	struct xfs_scrub	*sc,
-	struct xfs_bitmap	*bitmap,
-	struct xfs_owner_info	*oinfo,
-	enum xfs_ag_resv_type	type)
+	struct xfs_scrub		*sc,
+	struct xfs_bitmap		*bitmap,
+	const struct xfs_owner_info	*oinfo,
+	enum xfs_ag_resv_type		type)
 {
-	struct xfs_bitmap_range	*bmr;
-	struct xfs_bitmap_range	*n;
-	xfs_fsblock_t		fsbno;
-	int			error = 0;
+	struct xfs_bitmap_range		*bmr;
+	struct xfs_bitmap_range		*n;
+	xfs_fsblock_t			fsbno;
+	int				error = 0;
 
 	ASSERT(xfs_sb_version_hasrmapbt(&sc->mp->m_sb));
 
diff --git a/fs/xfs/scrub/repair.h b/fs/xfs/scrub/repair.h
index 9de321eee4ab..f2fc18bb7605 100644
--- a/fs/xfs/scrub/repair.h
+++ b/fs/xfs/scrub/repair.h
@@ -21,8 +21,9 @@ int xrep_roll_ag_trans(struct xfs_scrub *sc);
 bool xrep_ag_has_space(struct xfs_perag *pag, xfs_extlen_t nr_blocks,
 		enum xfs_ag_resv_type type);
 xfs_extlen_t xrep_calc_ag_resblks(struct xfs_scrub *sc);
-int xrep_alloc_ag_block(struct xfs_scrub *sc, struct xfs_owner_info *oinfo,
-		xfs_fsblock_t *fsbno, enum xfs_ag_resv_type resv);
+int xrep_alloc_ag_block(struct xfs_scrub *sc,
+		const struct xfs_owner_info *oinfo, xfs_fsblock_t *fsbno,
+		enum xfs_ag_resv_type resv);
 int xrep_init_btblock(struct xfs_scrub *sc, xfs_fsblock_t fsb,
 		struct xfs_buf **bpp, xfs_btnum_t btnum,
 		const struct xfs_buf_ops *ops);
@@ -32,7 +33,7 @@ struct xfs_bitmap;
 int xrep_fix_freelist(struct xfs_scrub *sc, bool can_shrink);
 int xrep_invalidate_blocks(struct xfs_scrub *sc, struct xfs_bitmap *btlist);
 int xrep_reap_extents(struct xfs_scrub *sc, struct xfs_bitmap *exlist,
-		struct xfs_owner_info *oinfo, enum xfs_ag_resv_type type);
+		const struct xfs_owner_info *oinfo, enum xfs_ag_resv_type type);
 
 struct xrep_find_ag_btree {
 	/* in: rmap owner of the btree we're looking for */
diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
index 5e293c129813..8a5bf15d544d 100644
--- a/fs/xfs/scrub/rmap.c
+++ b/fs/xfs/scrub/rmap.c
@@ -184,14 +184,14 @@ xchk_rmapbt(
 /* xref check that the extent is owned by a given owner */
 static inline void
 xchk_xref_check_owner(
-	struct xfs_scrub	*sc,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo,
-	bool			should_have_rmap)
+	struct xfs_scrub		*sc,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	const struct xfs_owner_info	*oinfo,
+	bool				should_have_rmap)
 {
-	bool			has_rmap;
-	int			error;
+	bool				has_rmap;
+	int				error;
 
 	if (!sc->sa.rmap_cur || xchk_skip_xref(sc->sm))
 		return;
@@ -207,10 +207,10 @@ xchk_xref_check_owner(
 /* xref check that the extent is owned by a given owner */
 void
 xchk_xref_is_owned_by(
-	struct xfs_scrub	*sc,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_scrub		*sc,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	const struct xfs_owner_info	*oinfo)
 {
 	xchk_xref_check_owner(sc, bno, len, oinfo, true);
 }
@@ -218,10 +218,10 @@ xchk_xref_is_owned_by(
 /* xref check that the extent is not owned by a given owner */
 void
 xchk_xref_is_not_owned_by(
-	struct xfs_scrub	*sc,
-	xfs_agblock_t		bno,
-	xfs_extlen_t		len,
-	struct xfs_owner_info	*oinfo)
+	struct xfs_scrub		*sc,
+	xfs_agblock_t			bno,
+	xfs_extlen_t			len,
+	const struct xfs_owner_info	*oinfo)
 {
 	xchk_xref_check_owner(sc, bno, len, oinfo, false);
 }
diff --git a/fs/xfs/scrub/scrub.h b/fs/xfs/scrub/scrub.h
index af323b229c4b..22f754fba8e5 100644
--- a/fs/xfs/scrub/scrub.h
+++ b/fs/xfs/scrub/scrub.h
@@ -122,9 +122,9 @@ void xchk_xref_is_not_inode_chunk(struct xfs_scrub *sc, xfs_agblock_t agbno,
 void xchk_xref_is_inode_chunk(struct xfs_scrub *sc, xfs_agblock_t agbno,
 		xfs_extlen_t len);
 void xchk_xref_is_owned_by(struct xfs_scrub *sc, xfs_agblock_t agbno,
-		xfs_extlen_t len, struct xfs_owner_info *oinfo);
+		xfs_extlen_t len, const struct xfs_owner_info *oinfo);
 void xchk_xref_is_not_owned_by(struct xfs_scrub *sc, xfs_agblock_t agbno,
-		xfs_extlen_t len, struct xfs_owner_info *oinfo);
+		xfs_extlen_t len, const struct xfs_owner_info *oinfo);
 void xchk_xref_has_no_owner(struct xfs_scrub *sc, xfs_agblock_t agbno,
 		xfs_extlen_t len);
 void xchk_xref_is_cow_staging(struct xfs_scrub *sc, xfs_agblock_t bno,
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index fe019ea231c1..ca4bd3417e7f 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -2402,7 +2402,7 @@ DEFINE_BMAP_FREE_DEFERRED_EVENT(xfs_agfl_free_deferred);
 DECLARE_EVENT_CLASS(xfs_rmap_class,
 	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno,
 		 xfs_agblock_t agbno, xfs_extlen_t len, bool unwritten,
-		 struct xfs_owner_info *oinfo),
+		 const struct xfs_owner_info *oinfo),
 	TP_ARGS(mp, agno, agbno, len, unwritten, oinfo),
 	TP_STRUCT__entry(
 		__field(dev_t, dev)
@@ -2437,7 +2437,7 @@ DECLARE_EVENT_CLASS(xfs_rmap_class,
 DEFINE_EVENT(xfs_rmap_class, name, \
 	TP_PROTO(struct xfs_mount *mp, xfs_agnumber_t agno, \
 		 xfs_agblock_t agbno, xfs_extlen_t len, bool unwritten, \
-		 struct xfs_owner_info *oinfo), \
+		 const struct xfs_owner_info *oinfo), \
 	TP_ARGS(mp, agno, agbno, len, unwritten, oinfo))
 
 /* simple AG-based error/%ip tracepoint class */
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index 3ba14ebb7a3b..c6e1c5704a8c 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -228,7 +228,8 @@ struct xfs_efd_log_item	*xfs_trans_get_efd(struct xfs_trans *,
 				  uint);
 int		xfs_trans_free_extent(struct xfs_trans *,
 				      struct xfs_efd_log_item *, xfs_fsblock_t,
-				      xfs_extlen_t, struct xfs_owner_info *,
+				      xfs_extlen_t,
+				      const struct xfs_owner_info *,
 				      bool);
 int		xfs_trans_commit(struct xfs_trans *);
 int		xfs_trans_roll(struct xfs_trans **);
diff --git a/fs/xfs/xfs_trans_extfree.c b/fs/xfs/xfs_trans_extfree.c
index 73b11ed6795e..5210f86a9114 100644
--- a/fs/xfs/xfs_trans_extfree.c
+++ b/fs/xfs/xfs_trans_extfree.c
@@ -57,7 +57,7 @@ xfs_trans_free_extent(
 	struct xfs_efd_log_item	*efdp,
 	xfs_fsblock_t		start_block,
 	xfs_extlen_t		ext_len,
-	struct xfs_owner_info	*oinfo,
+	const struct xfs_owner_info	*oinfo,
 	bool			skip_discard)
 {
 	struct xfs_mount	*mp = tp->t_mountp;

Re: [PATCH 1/5] xfs: const-ify xfs_owner_info arguments

[Brian Foster]

On Thu, Nov 08, 2018 at 03:20:07PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Only certain functions actually change the contents of an
> xfs_owner_info; the rest can accept a const struct pointer.  This will
> enable us to save stack space by hoisting static owner info types to
> be const global variables.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c  |    4 -
>  fs/xfs/libxfs/xfs_alloc.h  |    4 -
>  fs/xfs/libxfs/xfs_bmap.c   |    2 
>  fs/xfs/libxfs/xfs_bmap.h   |    4 -
>  fs/xfs/libxfs/xfs_rmap.c   |  212 ++++++++++++++++++++++----------------------
>  fs/xfs/libxfs/xfs_rmap.h   |   12 +-
>  fs/xfs/scrub/btree.c       |   45 +++++----
>  fs/xfs/scrub/btree.h       |   22 ++---
>  fs/xfs/scrub/common.c      |   14 +--
>  fs/xfs/scrub/common.h      |    2 
>  fs/xfs/scrub/refcount.c    |   11 +-
>  fs/xfs/scrub/repair.c      |   50 +++++-----
>  fs/xfs/scrub/repair.h      |    7 +
>  fs/xfs/scrub/rmap.c        |   30 +++---
>  fs/xfs/scrub/scrub.h       |    4 -
>  fs/xfs/xfs_trace.h         |    4 -
>  fs/xfs/xfs_trans.h         |    3 -
>  fs/xfs/xfs_trans_extfree.c |    2 
>  18 files changed, 218 insertions(+), 214 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index e1c0c0d2f1b0..bce46fa0ee38 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1699,7 +1699,7 @@ xfs_free_ag_extent(
>  	xfs_agnumber_t		agno,
>  	xfs_agblock_t		bno,
>  	xfs_extlen_t		len,
> -	struct xfs_owner_info	*oinfo,
> +	const struct xfs_owner_info	*oinfo,
>  	enum xfs_ag_resv_type	type)
>  {
>  	xfs_btree_cur_t	*bno_cur;	/* cursor for by-block btree */
> @@ -3013,7 +3013,7 @@ __xfs_free_extent(
>  	struct xfs_trans	*tp,	/* transaction pointer */
>  	xfs_fsblock_t		bno,	/* starting block number of extent */
>  	xfs_extlen_t		len,	/* length of extent */
> -	struct xfs_owner_info	*oinfo,	/* extent owner */
> +	const struct xfs_owner_info	*oinfo,	/* extent owner */
>  	enum xfs_ag_resv_type	type,	/* block reservation type */
>  	bool			skip_discard)
>  {

Looks like we missed some function arg alignment fixups here.

...
> diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h
> index 157dc722ad35..00e38505037f 100644
> --- a/fs/xfs/libxfs/xfs_rmap.h
> +++ b/fs/xfs/libxfs/xfs_rmap.h
> @@ -52,7 +52,7 @@ xfs_rmap_skip_owner_update(
>  
>  static inline bool
>  xfs_rmap_should_skip_owner_update(
> -	struct xfs_owner_info	*oi)
> +	const struct xfs_owner_info	*oi)
>  {
>  	return oi->oi_owner == XFS_RMAP_OWN_NULL;
>  }
> @@ -103,7 +103,7 @@ xfs_rmap_irec_offset_unpack(
>  
>  static inline void
>  xfs_owner_info_unpack(
> -	struct xfs_owner_info	*oinfo,
> +	const struct xfs_owner_info	*oinfo,
>  	uint64_t		*owner,
>  	uint64_t		*offset,
>  	unsigned int		*flags)

... and here.

...
> diff --git a/fs/xfs/xfs_trans_extfree.c b/fs/xfs/xfs_trans_extfree.c
> index 73b11ed6795e..5210f86a9114 100644
> --- a/fs/xfs/xfs_trans_extfree.c
> +++ b/fs/xfs/xfs_trans_extfree.c
> @@ -57,7 +57,7 @@ xfs_trans_free_extent(
>  	struct xfs_efd_log_item	*efdp,
>  	xfs_fsblock_t		start_block,
>  	xfs_extlen_t		ext_len,
> -	struct xfs_owner_info	*oinfo,
> +	const struct xfs_owner_info	*oinfo,
>  	bool			skip_discard)

... and here.

With those fixed up:

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

>  {
>  	struct xfs_mount	*mp = tp->t_mountp;
> 

Re: [PATCH 1/5] xfs: const-ify xfs_owner_info arguments

[Darrick J. Wong]

On Tue, Nov 20, 2018 at 11:17:08AM -0500, Brian Foster wrote:
> On Thu, Nov 08, 2018 at 03:20:07PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Only certain functions actually change the contents of an
> > xfs_owner_info; the rest can accept a const struct pointer.  This will
> > enable us to save stack space by hoisting static owner info types to
> > be const global variables.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_alloc.c  |    4 -
> >  fs/xfs/libxfs/xfs_alloc.h  |    4 -
> >  fs/xfs/libxfs/xfs_bmap.c   |    2 
> >  fs/xfs/libxfs/xfs_bmap.h   |    4 -
> >  fs/xfs/libxfs/xfs_rmap.c   |  212 ++++++++++++++++++++++----------------------
> >  fs/xfs/libxfs/xfs_rmap.h   |   12 +-
> >  fs/xfs/scrub/btree.c       |   45 +++++----
> >  fs/xfs/scrub/btree.h       |   22 ++---
> >  fs/xfs/scrub/common.c      |   14 +--
> >  fs/xfs/scrub/common.h      |    2 
> >  fs/xfs/scrub/refcount.c    |   11 +-
> >  fs/xfs/scrub/repair.c      |   50 +++++-----
> >  fs/xfs/scrub/repair.h      |    7 +
> >  fs/xfs/scrub/rmap.c        |   30 +++---
> >  fs/xfs/scrub/scrub.h       |    4 -
> >  fs/xfs/xfs_trace.h         |    4 -
> >  fs/xfs/xfs_trans.h         |    3 -
> >  fs/xfs/xfs_trans_extfree.c |    2 
> >  18 files changed, 218 insertions(+), 214 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > index e1c0c0d2f1b0..bce46fa0ee38 100644
> > --- a/fs/xfs/libxfs/xfs_alloc.c
> > +++ b/fs/xfs/libxfs/xfs_alloc.c
> > @@ -1699,7 +1699,7 @@ xfs_free_ag_extent(
> >  	xfs_agnumber_t		agno,
> >  	xfs_agblock_t		bno,
> >  	xfs_extlen_t		len,
> > -	struct xfs_owner_info	*oinfo,
> > +	const struct xfs_owner_info	*oinfo,
> >  	enum xfs_ag_resv_type	type)
> >  {
> >  	xfs_btree_cur_t	*bno_cur;	/* cursor for by-block btree */
> > @@ -3013,7 +3013,7 @@ __xfs_free_extent(
> >  	struct xfs_trans	*tp,	/* transaction pointer */
> >  	xfs_fsblock_t		bno,	/* starting block number of extent */
> >  	xfs_extlen_t		len,	/* length of extent */
> > -	struct xfs_owner_info	*oinfo,	/* extent owner */
> > +	const struct xfs_owner_info	*oinfo,	/* extent owner */
> >  	enum xfs_ag_resv_type	type,	/* block reservation type */
> >  	bool			skip_discard)
> >  {
> 
> Looks like we missed some function arg alignment fixups here.

Oops, yeah, I'll fix those.

> ...
> > diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h
> > index 157dc722ad35..00e38505037f 100644
> > --- a/fs/xfs/libxfs/xfs_rmap.h
> > +++ b/fs/xfs/libxfs/xfs_rmap.h
> > @@ -52,7 +52,7 @@ xfs_rmap_skip_owner_update(
> >  
> >  static inline bool
> >  xfs_rmap_should_skip_owner_update(
> > -	struct xfs_owner_info	*oi)
> > +	const struct xfs_owner_info	*oi)
> >  {
> >  	return oi->oi_owner == XFS_RMAP_OWN_NULL;
> >  }
> > @@ -103,7 +103,7 @@ xfs_rmap_irec_offset_unpack(
> >  
> >  static inline void
> >  xfs_owner_info_unpack(
> > -	struct xfs_owner_info	*oinfo,
> > +	const struct xfs_owner_info	*oinfo,
> >  	uint64_t		*owner,
> >  	uint64_t		*offset,
> >  	unsigned int		*flags)
> 
> ... and here.
> 
> ...
> > diff --git a/fs/xfs/xfs_trans_extfree.c b/fs/xfs/xfs_trans_extfree.c
> > index 73b11ed6795e..5210f86a9114 100644
> > --- a/fs/xfs/xfs_trans_extfree.c
> > +++ b/fs/xfs/xfs_trans_extfree.c
> > @@ -57,7 +57,7 @@ xfs_trans_free_extent(
> >  	struct xfs_efd_log_item	*efdp,
> >  	xfs_fsblock_t		start_block,
> >  	xfs_extlen_t		ext_len,
> > -	struct xfs_owner_info	*oinfo,
> > +	const struct xfs_owner_info	*oinfo,
> >  	bool			skip_discard)
> 
> ... and here.

And those.

> With those fixed up:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>

Thank you for the review! :)

--D

> 
> >  {
> >  	struct xfs_mount	*mp = tp->t_mountp;
> > 

[PATCH 2/5] xfs: remove xfs_rmap_ag_owner and friends

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Owner information for static fs metadata can be defined readonly at
build time because it never changes across filesystems.  This enables us
to reduce stack usage (particularly in scrub) because we can use the
statically defined oinfo structures.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_ag.c             |    9 ++++-----
 fs/xfs/libxfs/xfs_alloc.c          |    8 +++-----
 fs/xfs/libxfs/xfs_bmap.c           |    4 ++--
 fs/xfs/libxfs/xfs_ialloc.c         |    8 +++-----
 fs/xfs/libxfs/xfs_ialloc_btree.c   |    7 ++-----
 fs/xfs/libxfs/xfs_refcount_btree.c |    6 ++----
 fs/xfs/libxfs/xfs_rmap.c           |   28 ++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_rmap.h           |   34 ++++++++++------------------------
 fs/xfs/scrub/agheader.c            |   25 +++++++------------------
 fs/xfs/scrub/agheader_repair.c     |    5 ++---
 fs/xfs/scrub/alloc.c               |    4 +---
 fs/xfs/scrub/ialloc.c              |   28 +++++++++-------------------
 fs/xfs/scrub/inode.c               |    4 +---
 fs/xfs/scrub/refcount.c            |   15 +++++----------
 fs/xfs/scrub/repair.c              |    4 +---
 fs/xfs/scrub/rmap.c                |    5 +----
 fs/xfs/xfs_extfree_item.c          |    5 ++---
 17 files changed, 83 insertions(+), 116 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index 9345802c99f7..999ad8d00d43 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -414,7 +414,6 @@ xfs_ag_extend_space(
 	struct aghdr_init_data	*id,
 	xfs_extlen_t		len)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_buf		*bp;
 	struct xfs_agi		*agi;
 	struct xfs_agf		*agf;
@@ -448,17 +447,17 @@ xfs_ag_extend_space(
 	/*
 	 * Free the new space.
 	 *
-	 * XFS_RMAP_OWN_NULL is used here to tell the rmap btree that
+	 * XFS_RMAP_OINFO_SKIP_UPDATE is used here to tell the rmap btree that
 	 * this doesn't actually exist in the rmap btree.
 	 */
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_NULL);
 	error = xfs_rmap_free(tp, bp, id->agno,
 				be32_to_cpu(agf->agf_length) - len,
-				len, &oinfo);
+				len, &XFS_RMAP_OINFO_SKIP_UPDATE);
 	if (error)
 		return error;
 
 	return  xfs_free_extent(tp, XFS_AGB_TO_FSB(mp, id->agno,
 					be32_to_cpu(agf->agf_length) - len),
-				len, &oinfo, XFS_AG_RESV_NONE);
+				len, &XFS_RMAP_OINFO_SKIP_UPDATE,
+				XFS_AG_RESV_NONE);
 }
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index bce46fa0ee38..0f5e10a0c024 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -1594,7 +1594,6 @@ xfs_alloc_ag_vextent_small(
 	xfs_extlen_t	*flenp,	/* result length */
 	int		*stat)	/* status: 0-freelist, 1-normal/none */
 {
-	struct xfs_owner_info	oinfo;
 	int		error;
 	xfs_agblock_t	fbno;
 	xfs_extlen_t	flen;
@@ -1648,9 +1647,8 @@ xfs_alloc_ag_vextent_small(
 			 * doesn't live in the free space, we need to clear
 			 * out the OWN_AG rmap.
 			 */
-			xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
 			error = xfs_rmap_free(args->tp, args->agbp, args->agno,
-					fbno, 1, &oinfo);
+					fbno, 1, &XFS_RMAP_OINFO_AG);
 			if (error)
 				goto error0;
 
@@ -2315,9 +2313,9 @@ xfs_alloc_fix_freelist(
 	 */
 	memset(&targs, 0, sizeof(targs));
 	if (flags & XFS_ALLOC_FLAG_NORMAP)
-		xfs_rmap_skip_owner_update(&targs.oinfo);
+		targs.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
 	else
-		xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);
+		targs.oinfo = XFS_RMAP_OINFO_AG;
 	while (!(flags & XFS_ALLOC_FLAG_NOSHRINK) && pag->pagf_flcount > need) {
 		error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);
 		if (error)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 60abaa2e8146..c8095db42e7b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -564,7 +564,7 @@ __xfs_bmap_add_free(
 	if (oinfo)
 		new->xefi_oinfo = *oinfo;
 	else
-		xfs_rmap_skip_owner_update(&new->xefi_oinfo);
+		new->xefi_oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
 	new->xefi_skip_discard = skip_discard;
 	trace_xfs_bmap_free_defer(tp->t_mountp,
 			XFS_FSB_TO_AGNO(tp->t_mountp, bno), 0,
@@ -3450,7 +3450,7 @@ xfs_bmap_btalloc(
 	args.tp = ap->tp;
 	args.mp = mp;
 	args.fsbno = ap->blkno;
-	xfs_rmap_skip_owner_update(&args.oinfo);
+	args.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
 
 	/* Trim the allocation back to the maximum an AG can fit. */
 	args.maxlen = min(ap->length, mp->m_ag_max_usable);
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index a8f6db735d5d..fcf0d17405d8 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -641,7 +641,7 @@ xfs_ialloc_ag_alloc(
 	args.tp = tp;
 	args.mp = tp->t_mountp;
 	args.fsbno = NULLFSBLOCK;
-	xfs_rmap_ag_owner(&args.oinfo, XFS_RMAP_OWN_INODES);
+	args.oinfo = XFS_RMAP_OINFO_INODES;
 
 #ifdef DEBUG
 	/* randomly do sparse inode allocations */
@@ -1849,14 +1849,12 @@ xfs_difree_inode_chunk(
 	int				nextbit;
 	xfs_agblock_t			agbno;
 	int				contigblk;
-	struct xfs_owner_info		oinfo;
 	DECLARE_BITMAP(holemask, XFS_INOBT_HOLEMASK_BITS);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
 
 	if (!xfs_inobt_issparse(rec->ir_holemask)) {
 		/* not sparse, calculate extent info directly */
 		xfs_bmap_add_free(tp, XFS_AGB_TO_FSB(mp, agno, sagbno),
-				  mp->m_ialloc_blks, &oinfo);
+				  mp->m_ialloc_blks, &XFS_RMAP_OINFO_INODES);
 		return;
 	}
 
@@ -1900,7 +1898,7 @@ xfs_difree_inode_chunk(
 		ASSERT(agbno % mp->m_sb.sb_spino_align == 0);
 		ASSERT(contigblk % mp->m_sb.sb_spino_align == 0);
 		xfs_bmap_add_free(tp, XFS_AGB_TO_FSB(mp, agno, agbno),
-				  contigblk, &oinfo);
+				  contigblk, &XFS_RMAP_OINFO_INODES);
 
 		/* reset range to current bit and carry on... */
 		startidx = endidx = nextbit;
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index 86c50208a143..2ab5bcd903a7 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -84,7 +84,7 @@ __xfs_inobt_alloc_block(
 	memset(&args, 0, sizeof(args));
 	args.tp = cur->bc_tp;
 	args.mp = cur->bc_mp;
-	xfs_rmap_ag_owner(&args.oinfo, XFS_RMAP_OWN_INOBT);
+	args.oinfo = XFS_RMAP_OINFO_INOBT;
 	args.fsbno = XFS_AGB_TO_FSB(args.mp, cur->bc_private.a.agno, sbno);
 	args.minlen = 1;
 	args.maxlen = 1;
@@ -136,12 +136,9 @@ __xfs_inobt_free_block(
 	struct xfs_buf		*bp,
 	enum xfs_ag_resv_type	resv)
 {
-	struct xfs_owner_info	oinfo;
-
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
 	return xfs_free_extent(cur->bc_tp,
 			XFS_DADDR_TO_FSB(cur->bc_mp, XFS_BUF_ADDR(bp)), 1,
-			&oinfo, resv);
+			&XFS_RMAP_OINFO_INOBT, resv);
 }
 
 STATIC int
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 1aaa01c97517..d9eab657b63e 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -70,7 +70,7 @@ xfs_refcountbt_alloc_block(
 	args.type = XFS_ALLOCTYPE_NEAR_BNO;
 	args.fsbno = XFS_AGB_TO_FSB(cur->bc_mp, cur->bc_private.a.agno,
 			xfs_refc_block(args.mp));
-	xfs_rmap_ag_owner(&args.oinfo, XFS_RMAP_OWN_REFC);
+	args.oinfo = XFS_RMAP_OINFO_REFC;
 	args.minlen = args.maxlen = args.prod = 1;
 	args.resv = XFS_AG_RESV_METADATA;
 
@@ -106,15 +106,13 @@ xfs_refcountbt_free_block(
 	struct xfs_buf		*agbp = cur->bc_private.a.agbp;
 	struct xfs_agf		*agf = XFS_BUF_TO_AGF(agbp);
 	xfs_fsblock_t		fsbno = XFS_DADDR_TO_FSB(mp, XFS_BUF_ADDR(bp));
-	struct xfs_owner_info	oinfo;
 	int			error;
 
 	trace_xfs_refcountbt_free_block(cur->bc_mp, cur->bc_private.a.agno,
 			XFS_FSB_TO_AGBNO(cur->bc_mp, fsbno), 1);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
 	be32_add_cpu(&agf->agf_refcount_blocks, -1);
 	xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_REFCOUNT_BLOCKS);
-	error = xfs_free_extent(cur->bc_tp, fsbno, 1, &oinfo,
+	error = xfs_free_extent(cur->bc_tp, fsbno, 1, &XFS_RMAP_OINFO_REFC,
 			XFS_AG_RESV_METADATA);
 	if (error)
 		return error;
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c
index e7d06295338d..8ed885507dd8 100644
--- a/fs/xfs/libxfs/xfs_rmap.c
+++ b/fs/xfs/libxfs/xfs_rmap.c
@@ -2550,3 +2550,31 @@ xfs_rmap_has_other_keys(
 	*has_rmap = rks.has_rmap;
 	return error;
 }
+
+const struct xfs_owner_info XFS_RMAP_OINFO_SKIP_UPDATE = {
+	.oi_owner = XFS_RMAP_OWN_NULL,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_ANY_OWNER = {
+	.oi_owner = XFS_RMAP_OWN_UNKNOWN,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_FS = {
+	.oi_owner = XFS_RMAP_OWN_FS,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_LOG = {
+	.oi_owner = XFS_RMAP_OWN_LOG,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_AG = {
+	.oi_owner = XFS_RMAP_OWN_AG,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_INOBT = {
+	.oi_owner = XFS_RMAP_OWN_INOBT,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_INODES = {
+	.oi_owner = XFS_RMAP_OWN_INODES,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_REFC = {
+	.oi_owner = XFS_RMAP_OWN_REFC,
+};
+const struct xfs_owner_info XFS_RMAP_OINFO_COW = {
+	.oi_owner = XFS_RMAP_OWN_COW,
+};
diff --git a/fs/xfs/libxfs/xfs_rmap.h b/fs/xfs/libxfs/xfs_rmap.h
index 00e38505037f..2c42b49b6b09 100644
--- a/fs/xfs/libxfs/xfs_rmap.h
+++ b/fs/xfs/libxfs/xfs_rmap.h
@@ -6,16 +6,6 @@
 #ifndef __XFS_RMAP_H__
 #define __XFS_RMAP_H__
 
-static inline void
-xfs_rmap_ag_owner(
-	struct xfs_owner_info	*oi,
-	uint64_t		owner)
-{
-	oi->oi_owner = owner;
-	oi->oi_offset = 0;
-	oi->oi_flags = 0;
-}
-
 static inline void
 xfs_rmap_ino_bmbt_owner(
 	struct xfs_owner_info	*oi,
@@ -43,13 +33,6 @@ xfs_rmap_ino_owner(
 		oi->oi_flags |= XFS_OWNER_INFO_ATTR_FORK;
 }
 
-static inline void
-xfs_rmap_skip_owner_update(
-	struct xfs_owner_info	*oi)
-{
-	xfs_rmap_ag_owner(oi, XFS_RMAP_OWN_NULL);
-}
-
 static inline bool
 xfs_rmap_should_skip_owner_update(
 	const struct xfs_owner_info	*oi)
@@ -57,13 +40,6 @@ xfs_rmap_should_skip_owner_update(
 	return oi->oi_owner == XFS_RMAP_OWN_NULL;
 }
 
-static inline void
-xfs_rmap_any_owner_update(
-	struct xfs_owner_info	*oi)
-{
-	xfs_rmap_ag_owner(oi, XFS_RMAP_OWN_UNKNOWN);
-}
-
 /* Reverse mapping functions. */
 
 struct xfs_buf;
@@ -225,4 +201,14 @@ int xfs_rmap_has_other_keys(struct xfs_btree_cur *cur, xfs_agblock_t bno,
 		bool *has_rmap);
 int xfs_rmap_map_raw(struct xfs_btree_cur *cur, struct xfs_rmap_irec *rmap);
 
+extern const struct xfs_owner_info XFS_RMAP_OINFO_SKIP_UPDATE;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_ANY_OWNER;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_FS;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_LOG;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_AG;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_INOBT;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_INODES;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_REFC;
+extern const struct xfs_owner_info XFS_RMAP_OINFO_COW;
+
 #endif	/* __XFS_RMAP_H__ */
diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
index 3068a9382feb..90955ab1e895 100644
--- a/fs/xfs/scrub/agheader.c
+++ b/fs/xfs/scrub/agheader.c
@@ -32,7 +32,6 @@ xchk_superblock_xref(
 	struct xfs_scrub	*sc,
 	struct xfs_buf		*bp)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agnumber_t		agno = sc->sm->sm_agno;
 	xfs_agblock_t		agbno;
@@ -49,8 +48,7 @@ xchk_superblock_xref(
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
-	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
+	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 
 	/* scrub teardown will take care of sc->sa for us */
@@ -484,7 +482,6 @@ STATIC void
 xchk_agf_xref(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agblock_t		agbno;
 	int			error;
@@ -502,8 +499,7 @@ xchk_agf_xref(
 	xchk_agf_xref_freeblks(sc);
 	xchk_agf_xref_cntbt(sc);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
-	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
+	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
 	xchk_agf_xref_btreeblks(sc);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 	xchk_agf_xref_refcblks(sc);
@@ -598,7 +594,6 @@ xchk_agf(
 /* AGFL */
 
 struct xchk_agfl_info {
-	struct xfs_owner_info	oinfo;
 	unsigned int		sz_entries;
 	unsigned int		nr_entries;
 	xfs_agblock_t		*entries;
@@ -609,15 +604,14 @@ struct xchk_agfl_info {
 STATIC void
 xchk_agfl_block_xref(
 	struct xfs_scrub	*sc,
-	xfs_agblock_t		agbno,
-	struct xfs_owner_info	*oinfo)
+	xfs_agblock_t		agbno)
 {
 	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
 		return;
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-	xchk_xref_is_owned_by(sc, agbno, 1, oinfo);
+	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_AG);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 }
 
@@ -638,7 +632,7 @@ xchk_agfl_block(
 	else
 		xchk_block_set_corrupt(sc, sc->sa.agfl_bp);
 
-	xchk_agfl_block_xref(sc, agbno, priv);
+	xchk_agfl_block_xref(sc, agbno);
 
 	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
 		return XFS_BTREE_QUERY_RANGE_ABORT;
@@ -662,7 +656,6 @@ STATIC void
 xchk_agfl_xref(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agblock_t		agbno;
 	int			error;
@@ -678,8 +671,7 @@ xchk_agfl_xref(
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
-	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
+	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 
 	/*
@@ -732,7 +724,6 @@ xchk_agfl(
 	}
 
 	/* Check the blocks in the AGFL. */
-	xfs_rmap_ag_owner(&sai.oinfo, XFS_RMAP_OWN_AG);
 	error = xfs_agfl_walk(sc->mp, XFS_BUF_TO_AGF(sc->sa.agf_bp),
 			sc->sa.agfl_bp, xchk_agfl_block, &sai);
 	if (error == XFS_BTREE_QUERY_RANGE_ABORT) {
@@ -791,7 +782,6 @@ STATIC void
 xchk_agi_xref(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_mount	*mp = sc->mp;
 	xfs_agblock_t		agbno;
 	int			error;
@@ -808,8 +798,7 @@ xchk_agi_xref(
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
 	xchk_agi_xref_icounts(sc);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
-	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
+	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 
 	/* scrub teardown will take care of sc->sa for us */
diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
index f7568a4b5fe5..03d1e15cceba 100644
--- a/fs/xfs/scrub/agheader_repair.c
+++ b/fs/xfs/scrub/agheader_repair.c
@@ -646,7 +646,6 @@ int
 xrep_agfl(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_bitmap	agfl_extents;
 	struct xfs_mount	*mp = sc->mp;
 	struct xfs_buf		*agf_bp;
@@ -708,8 +707,8 @@ xrep_agfl(
 		goto err;
 
 	/* Dump any AGFL overflow. */
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
-	return xrep_reap_extents(sc, &agfl_extents, &oinfo, XFS_AG_RESV_AGFL);
+	return xrep_reap_extents(sc, &agfl_extents, &XFS_RMAP_OINFO_AG,
+			XFS_AG_RESV_AGFL);
 err:
 	xfs_bitmap_destroy(&agfl_extents);
 	return error;
diff --git a/fs/xfs/scrub/alloc.c b/fs/xfs/scrub/alloc.c
index 376bcb585ae6..44883e9112ad 100644
--- a/fs/xfs/scrub/alloc.c
+++ b/fs/xfs/scrub/alloc.c
@@ -125,12 +125,10 @@ xchk_allocbt(
 	struct xfs_scrub	*sc,
 	xfs_btnum_t		which)
 {
-	struct xfs_owner_info	oinfo;
 	struct xfs_btree_cur	*cur;
 
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
 	cur = which == XFS_BTNUM_BNO ? sc->sa.bno_cur : sc->sa.cnt_cur;
-	return xchk_btree(sc, cur, xchk_allocbt_rec, &oinfo, NULL);
+	return xchk_btree(sc, cur, xchk_allocbt_rec, &XFS_RMAP_OINFO_AG, NULL);
 }
 
 int
diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
index 224dba937492..72f45b298fa5 100644
--- a/fs/xfs/scrub/ialloc.c
+++ b/fs/xfs/scrub/ialloc.c
@@ -82,15 +82,12 @@ xchk_iallocbt_chunk_xref(
 	xfs_agblock_t			agbno,
 	xfs_extlen_t			len)
 {
-	struct xfs_owner_info		oinfo;
-
 	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
 		return;
 
 	xchk_xref_is_used_space(sc, agbno, len);
 	xchk_iallocbt_chunk_xref_other(sc, irec, agino);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
-	xchk_xref_is_owned_by(sc, agbno, len, &oinfo);
+	xchk_xref_is_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
 	xchk_xref_is_not_shared(sc, agbno, len);
 }
 
@@ -186,7 +183,6 @@ xchk_iallocbt_check_freemask(
 	struct xchk_btree		*bs,
 	struct xfs_inobt_rec_incore	*irec)
 {
-	struct xfs_owner_info		oinfo;
 	struct xfs_imap			imap;
 	struct xfs_mount		*mp = bs->cur->bc_mp;
 	struct xfs_dinode		*dip;
@@ -205,7 +201,6 @@ xchk_iallocbt_check_freemask(
 	/* Make sure the freemask matches the inode records. */
 	blks_per_cluster = xfs_icluster_size_fsb(mp);
 	nr_inodes = XFS_OFFBNO_TO_AGINO(mp, blks_per_cluster, 0);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
 
 	for (agino = irec->ir_startino;
 	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
@@ -230,12 +225,13 @@ xchk_iallocbt_check_freemask(
 		/* If any part of this is a hole, skip it. */
 		if (ir_holemask) {
 			xchk_xref_is_not_owned_by(bs->sc, agbno,
-					blks_per_cluster, &oinfo);
+					blks_per_cluster,
+					&XFS_RMAP_OINFO_INODES);
 			continue;
 		}
 
 		xchk_xref_is_owned_by(bs->sc, agbno, blks_per_cluster,
-				&oinfo);
+				&XFS_RMAP_OINFO_INODES);
 
 		/* Grab the inode cluster buffer. */
 		imap.im_blkno = XFS_AGB_TO_DADDR(mp, bs->cur->bc_private.a.agno,
@@ -366,7 +362,6 @@ xchk_iallocbt_xref_rmap_btreeblks(
 	struct xfs_scrub	*sc,
 	int			which)
 {
-	struct xfs_owner_info	oinfo;
 	xfs_filblks_t		blocks;
 	xfs_extlen_t		inobt_blocks = 0;
 	xfs_extlen_t		finobt_blocks = 0;
@@ -388,9 +383,8 @@ xchk_iallocbt_xref_rmap_btreeblks(
 			return;
 	}
 
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
-	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
-			&blocks);
+	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
+			&XFS_RMAP_OINFO_INOBT, &blocks);
 	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
 		return;
 	if (blocks != inobt_blocks + finobt_blocks)
@@ -407,7 +401,6 @@ xchk_iallocbt_xref_rmap_inodes(
 	int			which,
 	xfs_filblks_t		inode_blocks)
 {
-	struct xfs_owner_info	oinfo;
 	xfs_filblks_t		blocks;
 	int			error;
 
@@ -415,9 +408,8 @@ xchk_iallocbt_xref_rmap_inodes(
 		return;
 
 	/* Check that we saw as many inode blocks as the rmap knows about. */
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
-	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
-			&blocks);
+	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
+			&XFS_RMAP_OINFO_INODES, &blocks);
 	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
 		return;
 	if (blocks != inode_blocks)
@@ -431,13 +423,11 @@ xchk_iallocbt(
 	xfs_btnum_t		which)
 {
 	struct xfs_btree_cur	*cur;
-	struct xfs_owner_info	oinfo;
 	xfs_filblks_t		inode_blocks = 0;
 	int			error;
 
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
 	cur = which == XFS_BTNUM_INO ? sc->sa.ino_cur : sc->sa.fino_cur;
-	error = xchk_btree(sc, cur, xchk_iallocbt_rec, &oinfo,
+	error = xchk_btree(sc, cur, xchk_iallocbt_rec, &XFS_RMAP_OINFO_INOBT,
 			&inode_blocks);
 	if (error)
 		return error;
diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index e386c9b0b4ab..e213efc194a1 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -509,7 +509,6 @@ xchk_inode_xref(
 	xfs_ino_t		ino,
 	struct xfs_dinode	*dip)
 {
-	struct xfs_owner_info	oinfo;
 	xfs_agnumber_t		agno;
 	xfs_agblock_t		agbno;
 	int			error;
@@ -526,8 +525,7 @@ xchk_inode_xref(
 
 	xchk_xref_is_used_space(sc, agbno, 1);
 	xchk_inode_xref_finobt(sc, ino);
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
-	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
+	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_INODES);
 	xchk_xref_is_not_shared(sc, agbno, 1);
 	xchk_inode_xref_bmap(sc, dip);
 
diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
index b7ade620acee..708b4158eb90 100644
--- a/fs/xfs/scrub/refcount.c
+++ b/fs/xfs/scrub/refcount.c
@@ -385,7 +385,6 @@ xchk_refcount_xref_rmap(
 	struct xfs_scrub	*sc,
 	xfs_filblks_t		cow_blocks)
 {
-	struct xfs_owner_info	oinfo;
 	xfs_extlen_t		refcbt_blocks = 0;
 	xfs_filblks_t		blocks;
 	int			error;
@@ -394,21 +393,19 @@ xchk_refcount_xref_rmap(
 		return;
 
 	/* Check that we saw as many refcbt blocks as the rmap knows about. */
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
 	error = xfs_btree_count_blocks(sc->sa.refc_cur, &refcbt_blocks);
 	if (!xchk_btree_process_error(sc, sc->sa.refc_cur, 0, &error))
 		return;
-	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
-			&blocks);
+	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
+			&XFS_RMAP_OINFO_REFC, &blocks);
 	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
 		return;
 	if (blocks != refcbt_blocks)
 		xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
 
 	/* Check that we saw as many cow blocks as the rmap knows about. */
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_COW);
-	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
-			&blocks);
+	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
+			&XFS_RMAP_OINFO_COW, &blocks);
 	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
 		return;
 	if (blocks != cow_blocks)
@@ -420,13 +417,11 @@ int
 xchk_refcountbt(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_owner_info	oinfo;
 	xfs_agblock_t		cow_blocks = 0;
 	int			error;
 
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
 	error = xchk_btree(sc, sc->sa.refc_cur, xchk_refcountbt_rec,
-			&oinfo, &cow_blocks);
+			&XFS_RMAP_OINFO_REFC, &cow_blocks);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
index 90ae9e173de7..1c8eecfe52b8 100644
--- a/fs/xfs/scrub/repair.c
+++ b/fs/xfs/scrub/repair.c
@@ -505,7 +505,6 @@ xrep_put_freelist(
 	struct xfs_scrub	*sc,
 	xfs_agblock_t		agbno)
 {
-	struct xfs_owner_info	oinfo;
 	int			error;
 
 	/* Make sure there's space on the freelist. */
@@ -518,9 +517,8 @@ xrep_put_freelist(
 	 * create an rmap for the block prior to merging it or else other
 	 * parts will break.
 	 */
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
 	error = xfs_rmap_alloc(sc->tp, sc->sa.agf_bp, sc->sa.agno, agbno, 1,
-			&oinfo);
+			&XFS_RMAP_OINFO_AG);
 	if (error)
 		return error;
 
diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
index 8a5bf15d544d..92a140c5b55e 100644
--- a/fs/xfs/scrub/rmap.c
+++ b/fs/xfs/scrub/rmap.c
@@ -174,11 +174,8 @@ int
 xchk_rmapbt(
 	struct xfs_scrub	*sc)
 {
-	struct xfs_owner_info	oinfo;
-
-	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
 	return xchk_btree(sc, sc->sa.rmap_cur, xchk_rmapbt_rec,
-			&oinfo, NULL);
+			&XFS_RMAP_OINFO_AG, NULL);
 }
 
 /* xref check that the extent is owned by a given owner */
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index d9da66c718bb..74ddf66f4cfe 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -494,7 +494,6 @@ xfs_efi_recover(
 	int			error = 0;
 	xfs_extent_t		*extp;
 	xfs_fsblock_t		startblock_fsb;
-	struct xfs_owner_info	oinfo;
 
 	ASSERT(!test_bit(XFS_EFI_RECOVERED, &efip->efi_flags));
 
@@ -526,11 +525,11 @@ xfs_efi_recover(
 		return error;
 	efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
 
-	xfs_rmap_any_owner_update(&oinfo);
 	for (i = 0; i < efip->efi_format.efi_nextents; i++) {
 		extp = &efip->efi_format.efi_extents[i];
 		error = xfs_trans_free_extent(tp, efdp, extp->ext_start,
-					      extp->ext_len, &oinfo, false);
+					      extp->ext_len,
+					      &XFS_RMAP_OINFO_ANY_OWNER, false);
 		if (error)
 			goto abort_error;
 

Re: [PATCH 2/5] xfs: remove xfs_rmap_ag_owner and friends

[Brian Foster]

On Thu, Nov 08, 2018 at 03:20:13PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Owner information for static fs metadata can be defined readonly at
> build time because it never changes across filesystems.  This enables us
> to reduce stack usage (particularly in scrub) because we can use the
> statically defined oinfo structures.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_ag.c             |    9 ++++-----
>  fs/xfs/libxfs/xfs_alloc.c          |    8 +++-----
>  fs/xfs/libxfs/xfs_bmap.c           |    4 ++--
>  fs/xfs/libxfs/xfs_ialloc.c         |    8 +++-----
>  fs/xfs/libxfs/xfs_ialloc_btree.c   |    7 ++-----
>  fs/xfs/libxfs/xfs_refcount_btree.c |    6 ++----
>  fs/xfs/libxfs/xfs_rmap.c           |   28 ++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_rmap.h           |   34 ++++++++++------------------------
>  fs/xfs/scrub/agheader.c            |   25 +++++++------------------
>  fs/xfs/scrub/agheader_repair.c     |    5 ++---
>  fs/xfs/scrub/alloc.c               |    4 +---
>  fs/xfs/scrub/ialloc.c              |   28 +++++++++-------------------
>  fs/xfs/scrub/inode.c               |    4 +---
>  fs/xfs/scrub/refcount.c            |   15 +++++----------
>  fs/xfs/scrub/repair.c              |    4 +---
>  fs/xfs/scrub/rmap.c                |    5 +----
>  fs/xfs/xfs_extfree_item.c          |    5 ++---
>  17 files changed, 83 insertions(+), 116 deletions(-)
> 
> 
...
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index bce46fa0ee38..0f5e10a0c024 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1594,7 +1594,6 @@ xfs_alloc_ag_vextent_small(
>  	xfs_extlen_t	*flenp,	/* result length */
>  	int		*stat)	/* status: 0-freelist, 1-normal/none */
>  {
> -	struct xfs_owner_info	oinfo;
>  	int		error;
>  	xfs_agblock_t	fbno;
>  	xfs_extlen_t	flen;
> @@ -1648,9 +1647,8 @@ xfs_alloc_ag_vextent_small(
>  			 * doesn't live in the free space, we need to clear
>  			 * out the OWN_AG rmap.
>  			 */
> -			xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
>  			error = xfs_rmap_free(args->tp, args->agbp, args->agno,
> -					fbno, 1, &oinfo);
> +					fbno, 1, &XFS_RMAP_OINFO_AG);
>  			if (error)
>  				goto error0;
>  
> @@ -2315,9 +2313,9 @@ xfs_alloc_fix_freelist(
>  	 */
>  	memset(&targs, 0, sizeof(targs));
>  	if (flags & XFS_ALLOC_FLAG_NORMAP)
> -		xfs_rmap_skip_owner_update(&targs.oinfo);
> +		targs.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
>  	else
> -		xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);
> +		targs.oinfo = XFS_RMAP_OINFO_AG;

Hmm, so these are actually struct copies. The broader changes to use the
global directly seem fine, but it's kind of unfortunate how close these
assignment patterns look to some kind of direct value assignment at a
glance, particularly with the macro looking name.

>  	while (!(flags & XFS_ALLOC_FLAG_NOSHRINK) && pag->pagf_flcount > need) {
>  		error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);
>  		if (error)
...
> diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> index 3068a9382feb..90955ab1e895 100644
> --- a/fs/xfs/scrub/agheader.c
> +++ b/fs/xfs/scrub/agheader.c
...
> @@ -638,7 +632,7 @@ xchk_agfl_block(
>  	else
>  		xchk_block_set_corrupt(sc, sc->sa.agfl_bp);
>  
> -	xchk_agfl_block_xref(sc, agbno, priv);
> +	xchk_agfl_block_xref(sc, agbno);

Heh, I'm not sure if that was an intentional hack or happy accident, but
this looks like it removes a bit of a landmine by passing the void
pointer to the struct xchk_agfl_info directly as the oinfo. :P

Brian

>  
>  	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
>  		return XFS_BTREE_QUERY_RANGE_ABORT;
> @@ -662,7 +656,6 @@ STATIC void
>  xchk_agfl_xref(
>  	struct xfs_scrub	*sc)
>  {
> -	struct xfs_owner_info	oinfo;
>  	struct xfs_mount	*mp = sc->mp;
>  	xfs_agblock_t		agbno;
>  	int			error;
> @@ -678,8 +671,7 @@ xchk_agfl_xref(
>  
>  	xchk_xref_is_used_space(sc, agbno, 1);
>  	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
> -	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
> +	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
>  	xchk_xref_is_not_shared(sc, agbno, 1);
>  
>  	/*
> @@ -732,7 +724,6 @@ xchk_agfl(
>  	}
>  
>  	/* Check the blocks in the AGFL. */
> -	xfs_rmap_ag_owner(&sai.oinfo, XFS_RMAP_OWN_AG);
>  	error = xfs_agfl_walk(sc->mp, XFS_BUF_TO_AGF(sc->sa.agf_bp),
>  			sc->sa.agfl_bp, xchk_agfl_block, &sai);
>  	if (error == XFS_BTREE_QUERY_RANGE_ABORT) {
> @@ -791,7 +782,6 @@ STATIC void
>  xchk_agi_xref(
>  	struct xfs_scrub	*sc)
>  {
> -	struct xfs_owner_info	oinfo;
>  	struct xfs_mount	*mp = sc->mp;
>  	xfs_agblock_t		agbno;
>  	int			error;
> @@ -808,8 +798,7 @@ xchk_agi_xref(
>  	xchk_xref_is_used_space(sc, agbno, 1);
>  	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
>  	xchk_agi_xref_icounts(sc);
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
> -	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
> +	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
>  	xchk_xref_is_not_shared(sc, agbno, 1);
>  
>  	/* scrub teardown will take care of sc->sa for us */
> diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> index f7568a4b5fe5..03d1e15cceba 100644
> --- a/fs/xfs/scrub/agheader_repair.c
> +++ b/fs/xfs/scrub/agheader_repair.c
> @@ -646,7 +646,6 @@ int
>  xrep_agfl(
>  	struct xfs_scrub	*sc)
>  {
> -	struct xfs_owner_info	oinfo;
>  	struct xfs_bitmap	agfl_extents;
>  	struct xfs_mount	*mp = sc->mp;
>  	struct xfs_buf		*agf_bp;
> @@ -708,8 +707,8 @@ xrep_agfl(
>  		goto err;
>  
>  	/* Dump any AGFL overflow. */
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
> -	return xrep_reap_extents(sc, &agfl_extents, &oinfo, XFS_AG_RESV_AGFL);
> +	return xrep_reap_extents(sc, &agfl_extents, &XFS_RMAP_OINFO_AG,
> +			XFS_AG_RESV_AGFL);
>  err:
>  	xfs_bitmap_destroy(&agfl_extents);
>  	return error;
> diff --git a/fs/xfs/scrub/alloc.c b/fs/xfs/scrub/alloc.c
> index 376bcb585ae6..44883e9112ad 100644
> --- a/fs/xfs/scrub/alloc.c
> +++ b/fs/xfs/scrub/alloc.c
> @@ -125,12 +125,10 @@ xchk_allocbt(
>  	struct xfs_scrub	*sc,
>  	xfs_btnum_t		which)
>  {
> -	struct xfs_owner_info	oinfo;
>  	struct xfs_btree_cur	*cur;
>  
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
>  	cur = which == XFS_BTNUM_BNO ? sc->sa.bno_cur : sc->sa.cnt_cur;
> -	return xchk_btree(sc, cur, xchk_allocbt_rec, &oinfo, NULL);
> +	return xchk_btree(sc, cur, xchk_allocbt_rec, &XFS_RMAP_OINFO_AG, NULL);
>  }
>  
>  int
> diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
> index 224dba937492..72f45b298fa5 100644
> --- a/fs/xfs/scrub/ialloc.c
> +++ b/fs/xfs/scrub/ialloc.c
> @@ -82,15 +82,12 @@ xchk_iallocbt_chunk_xref(
>  	xfs_agblock_t			agbno,
>  	xfs_extlen_t			len)
>  {
> -	struct xfs_owner_info		oinfo;
> -
>  	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
>  		return;
>  
>  	xchk_xref_is_used_space(sc, agbno, len);
>  	xchk_iallocbt_chunk_xref_other(sc, irec, agino);
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> -	xchk_xref_is_owned_by(sc, agbno, len, &oinfo);
> +	xchk_xref_is_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
>  	xchk_xref_is_not_shared(sc, agbno, len);
>  }
>  
> @@ -186,7 +183,6 @@ xchk_iallocbt_check_freemask(
>  	struct xchk_btree		*bs,
>  	struct xfs_inobt_rec_incore	*irec)
>  {
> -	struct xfs_owner_info		oinfo;
>  	struct xfs_imap			imap;
>  	struct xfs_mount		*mp = bs->cur->bc_mp;
>  	struct xfs_dinode		*dip;
> @@ -205,7 +201,6 @@ xchk_iallocbt_check_freemask(
>  	/* Make sure the freemask matches the inode records. */
>  	blks_per_cluster = xfs_icluster_size_fsb(mp);
>  	nr_inodes = XFS_OFFBNO_TO_AGINO(mp, blks_per_cluster, 0);
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
>  
>  	for (agino = irec->ir_startino;
>  	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
> @@ -230,12 +225,13 @@ xchk_iallocbt_check_freemask(
>  		/* If any part of this is a hole, skip it. */
>  		if (ir_holemask) {
>  			xchk_xref_is_not_owned_by(bs->sc, agbno,
> -					blks_per_cluster, &oinfo);
> +					blks_per_cluster,
> +					&XFS_RMAP_OINFO_INODES);
>  			continue;
>  		}
>  
>  		xchk_xref_is_owned_by(bs->sc, agbno, blks_per_cluster,
> -				&oinfo);
> +				&XFS_RMAP_OINFO_INODES);
>  
>  		/* Grab the inode cluster buffer. */
>  		imap.im_blkno = XFS_AGB_TO_DADDR(mp, bs->cur->bc_private.a.agno,
> @@ -366,7 +362,6 @@ xchk_iallocbt_xref_rmap_btreeblks(
>  	struct xfs_scrub	*sc,
>  	int			which)
>  {
> -	struct xfs_owner_info	oinfo;
>  	xfs_filblks_t		blocks;
>  	xfs_extlen_t		inobt_blocks = 0;
>  	xfs_extlen_t		finobt_blocks = 0;
> @@ -388,9 +383,8 @@ xchk_iallocbt_xref_rmap_btreeblks(
>  			return;
>  	}
>  
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
> -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> -			&blocks);
> +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> +			&XFS_RMAP_OINFO_INOBT, &blocks);
>  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
>  		return;
>  	if (blocks != inobt_blocks + finobt_blocks)
> @@ -407,7 +401,6 @@ xchk_iallocbt_xref_rmap_inodes(
>  	int			which,
>  	xfs_filblks_t		inode_blocks)
>  {
> -	struct xfs_owner_info	oinfo;
>  	xfs_filblks_t		blocks;
>  	int			error;
>  
> @@ -415,9 +408,8 @@ xchk_iallocbt_xref_rmap_inodes(
>  		return;
>  
>  	/* Check that we saw as many inode blocks as the rmap knows about. */
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> -			&blocks);
> +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> +			&XFS_RMAP_OINFO_INODES, &blocks);
>  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
>  		return;
>  	if (blocks != inode_blocks)
> @@ -431,13 +423,11 @@ xchk_iallocbt(
>  	xfs_btnum_t		which)
>  {
>  	struct xfs_btree_cur	*cur;
> -	struct xfs_owner_info	oinfo;
>  	xfs_filblks_t		inode_blocks = 0;
>  	int			error;
>  
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
>  	cur = which == XFS_BTNUM_INO ? sc->sa.ino_cur : sc->sa.fino_cur;
> -	error = xchk_btree(sc, cur, xchk_iallocbt_rec, &oinfo,
> +	error = xchk_btree(sc, cur, xchk_iallocbt_rec, &XFS_RMAP_OINFO_INOBT,
>  			&inode_blocks);
>  	if (error)
>  		return error;
> diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> index e386c9b0b4ab..e213efc194a1 100644
> --- a/fs/xfs/scrub/inode.c
> +++ b/fs/xfs/scrub/inode.c
> @@ -509,7 +509,6 @@ xchk_inode_xref(
>  	xfs_ino_t		ino,
>  	struct xfs_dinode	*dip)
>  {
> -	struct xfs_owner_info	oinfo;
>  	xfs_agnumber_t		agno;
>  	xfs_agblock_t		agbno;
>  	int			error;
> @@ -526,8 +525,7 @@ xchk_inode_xref(
>  
>  	xchk_xref_is_used_space(sc, agbno, 1);
>  	xchk_inode_xref_finobt(sc, ino);
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> -	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
> +	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_INODES);
>  	xchk_xref_is_not_shared(sc, agbno, 1);
>  	xchk_inode_xref_bmap(sc, dip);
>  
> diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
> index b7ade620acee..708b4158eb90 100644
> --- a/fs/xfs/scrub/refcount.c
> +++ b/fs/xfs/scrub/refcount.c
> @@ -385,7 +385,6 @@ xchk_refcount_xref_rmap(
>  	struct xfs_scrub	*sc,
>  	xfs_filblks_t		cow_blocks)
>  {
> -	struct xfs_owner_info	oinfo;
>  	xfs_extlen_t		refcbt_blocks = 0;
>  	xfs_filblks_t		blocks;
>  	int			error;
> @@ -394,21 +393,19 @@ xchk_refcount_xref_rmap(
>  		return;
>  
>  	/* Check that we saw as many refcbt blocks as the rmap knows about. */
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
>  	error = xfs_btree_count_blocks(sc->sa.refc_cur, &refcbt_blocks);
>  	if (!xchk_btree_process_error(sc, sc->sa.refc_cur, 0, &error))
>  		return;
> -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> -			&blocks);
> +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> +			&XFS_RMAP_OINFO_REFC, &blocks);
>  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
>  		return;
>  	if (blocks != refcbt_blocks)
>  		xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
>  
>  	/* Check that we saw as many cow blocks as the rmap knows about. */
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_COW);
> -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> -			&blocks);
> +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> +			&XFS_RMAP_OINFO_COW, &blocks);
>  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
>  		return;
>  	if (blocks != cow_blocks)
> @@ -420,13 +417,11 @@ int
>  xchk_refcountbt(
>  	struct xfs_scrub	*sc)
>  {
> -	struct xfs_owner_info	oinfo;
>  	xfs_agblock_t		cow_blocks = 0;
>  	int			error;
>  
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
>  	error = xchk_btree(sc, sc->sa.refc_cur, xchk_refcountbt_rec,
> -			&oinfo, &cow_blocks);
> +			&XFS_RMAP_OINFO_REFC, &cow_blocks);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
> index 90ae9e173de7..1c8eecfe52b8 100644
> --- a/fs/xfs/scrub/repair.c
> +++ b/fs/xfs/scrub/repair.c
> @@ -505,7 +505,6 @@ xrep_put_freelist(
>  	struct xfs_scrub	*sc,
>  	xfs_agblock_t		agbno)
>  {
> -	struct xfs_owner_info	oinfo;
>  	int			error;
>  
>  	/* Make sure there's space on the freelist. */
> @@ -518,9 +517,8 @@ xrep_put_freelist(
>  	 * create an rmap for the block prior to merging it or else other
>  	 * parts will break.
>  	 */
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
>  	error = xfs_rmap_alloc(sc->tp, sc->sa.agf_bp, sc->sa.agno, agbno, 1,
> -			&oinfo);
> +			&XFS_RMAP_OINFO_AG);
>  	if (error)
>  		return error;
>  
> diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
> index 8a5bf15d544d..92a140c5b55e 100644
> --- a/fs/xfs/scrub/rmap.c
> +++ b/fs/xfs/scrub/rmap.c
> @@ -174,11 +174,8 @@ int
>  xchk_rmapbt(
>  	struct xfs_scrub	*sc)
>  {
> -	struct xfs_owner_info	oinfo;
> -
> -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
>  	return xchk_btree(sc, sc->sa.rmap_cur, xchk_rmapbt_rec,
> -			&oinfo, NULL);
> +			&XFS_RMAP_OINFO_AG, NULL);
>  }
>  
>  /* xref check that the extent is owned by a given owner */
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index d9da66c718bb..74ddf66f4cfe 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -494,7 +494,6 @@ xfs_efi_recover(
>  	int			error = 0;
>  	xfs_extent_t		*extp;
>  	xfs_fsblock_t		startblock_fsb;
> -	struct xfs_owner_info	oinfo;
>  
>  	ASSERT(!test_bit(XFS_EFI_RECOVERED, &efip->efi_flags));
>  
> @@ -526,11 +525,11 @@ xfs_efi_recover(
>  		return error;
>  	efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
>  
> -	xfs_rmap_any_owner_update(&oinfo);
>  	for (i = 0; i < efip->efi_format.efi_nextents; i++) {
>  		extp = &efip->efi_format.efi_extents[i];
>  		error = xfs_trans_free_extent(tp, efdp, extp->ext_start,
> -					      extp->ext_len, &oinfo, false);
> +					      extp->ext_len,
> +					      &XFS_RMAP_OINFO_ANY_OWNER, false);
>  		if (error)
>  			goto abort_error;
>  
> 

Re: [PATCH 2/5] xfs: remove xfs_rmap_ag_owner and friends

[Darrick J. Wong]

On Tue, Nov 20, 2018 at 11:20:09AM -0500, Brian Foster wrote:
> On Thu, Nov 08, 2018 at 03:20:13PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Owner information for static fs metadata can be defined readonly at
> > build time because it never changes across filesystems.  This enables us
> > to reduce stack usage (particularly in scrub) because we can use the
> > statically defined oinfo structures.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_ag.c             |    9 ++++-----
> >  fs/xfs/libxfs/xfs_alloc.c          |    8 +++-----
> >  fs/xfs/libxfs/xfs_bmap.c           |    4 ++--
> >  fs/xfs/libxfs/xfs_ialloc.c         |    8 +++-----
> >  fs/xfs/libxfs/xfs_ialloc_btree.c   |    7 ++-----
> >  fs/xfs/libxfs/xfs_refcount_btree.c |    6 ++----
> >  fs/xfs/libxfs/xfs_rmap.c           |   28 ++++++++++++++++++++++++++++
> >  fs/xfs/libxfs/xfs_rmap.h           |   34 ++++++++++------------------------
> >  fs/xfs/scrub/agheader.c            |   25 +++++++------------------
> >  fs/xfs/scrub/agheader_repair.c     |    5 ++---
> >  fs/xfs/scrub/alloc.c               |    4 +---
> >  fs/xfs/scrub/ialloc.c              |   28 +++++++++-------------------
> >  fs/xfs/scrub/inode.c               |    4 +---
> >  fs/xfs/scrub/refcount.c            |   15 +++++----------
> >  fs/xfs/scrub/repair.c              |    4 +---
> >  fs/xfs/scrub/rmap.c                |    5 +----
> >  fs/xfs/xfs_extfree_item.c          |    5 ++---
> >  17 files changed, 83 insertions(+), 116 deletions(-)
> > 
> > 
> ...
> > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > index bce46fa0ee38..0f5e10a0c024 100644
> > --- a/fs/xfs/libxfs/xfs_alloc.c
> > +++ b/fs/xfs/libxfs/xfs_alloc.c
> > @@ -1594,7 +1594,6 @@ xfs_alloc_ag_vextent_small(
> >  	xfs_extlen_t	*flenp,	/* result length */
> >  	int		*stat)	/* status: 0-freelist, 1-normal/none */
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	int		error;
> >  	xfs_agblock_t	fbno;
> >  	xfs_extlen_t	flen;
> > @@ -1648,9 +1647,8 @@ xfs_alloc_ag_vextent_small(
> >  			 * doesn't live in the free space, we need to clear
> >  			 * out the OWN_AG rmap.
> >  			 */
> > -			xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
> >  			error = xfs_rmap_free(args->tp, args->agbp, args->agno,
> > -					fbno, 1, &oinfo);
> > +					fbno, 1, &XFS_RMAP_OINFO_AG);
> >  			if (error)
> >  				goto error0;
> >  
> > @@ -2315,9 +2313,9 @@ xfs_alloc_fix_freelist(
> >  	 */
> >  	memset(&targs, 0, sizeof(targs));
> >  	if (flags & XFS_ALLOC_FLAG_NORMAP)
> > -		xfs_rmap_skip_owner_update(&targs.oinfo);
> > +		targs.oinfo = XFS_RMAP_OINFO_SKIP_UPDATE;
> >  	else
> > -		xfs_rmap_ag_owner(&targs.oinfo, XFS_RMAP_OWN_AG);
> > +		targs.oinfo = XFS_RMAP_OINFO_AG;
> 
> Hmm, so these are actually struct copies. The broader changes to use the
> global directly seem fine, but it's kind of unfortunate how close these
> assignment patterns look to some kind of direct value assignment at a
> glance, particularly with the macro looking name.

I'll add a comment /* struct copy */ to this.

> >  	while (!(flags & XFS_ALLOC_FLAG_NOSHRINK) && pag->pagf_flcount > need) {
> >  		error = xfs_alloc_get_freelist(tp, agbp, &bno, 0);
> >  		if (error)
> ...
> > diff --git a/fs/xfs/scrub/agheader.c b/fs/xfs/scrub/agheader.c
> > index 3068a9382feb..90955ab1e895 100644
> > --- a/fs/xfs/scrub/agheader.c
> > +++ b/fs/xfs/scrub/agheader.c
> ...
> > @@ -638,7 +632,7 @@ xchk_agfl_block(
> >  	else
> >  		xchk_block_set_corrupt(sc, sc->sa.agfl_bp);
> >  
> > -	xchk_agfl_block_xref(sc, agbno, priv);
> > +	xchk_agfl_block_xref(sc, agbno);
> 
> Heh, I'm not sure if that was an intentional hack or happy accident, but
> this looks like it removes a bit of a landmine by passing the void
> pointer to the struct xchk_agfl_info directly as the oinfo. :P

Yep, I'm glad this weird code can go away. :)

--D

> Brian
> 
> >  
> >  	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> >  		return XFS_BTREE_QUERY_RANGE_ABORT;
> > @@ -662,7 +656,6 @@ STATIC void
> >  xchk_agfl_xref(
> >  	struct xfs_scrub	*sc)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	struct xfs_mount	*mp = sc->mp;
> >  	xfs_agblock_t		agbno;
> >  	int			error;
> > @@ -678,8 +671,7 @@ xchk_agfl_xref(
> >  
> >  	xchk_xref_is_used_space(sc, agbno, 1);
> >  	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
> > -	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
> > +	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
> >  	xchk_xref_is_not_shared(sc, agbno, 1);
> >  
> >  	/*
> > @@ -732,7 +724,6 @@ xchk_agfl(
> >  	}
> >  
> >  	/* Check the blocks in the AGFL. */
> > -	xfs_rmap_ag_owner(&sai.oinfo, XFS_RMAP_OWN_AG);
> >  	error = xfs_agfl_walk(sc->mp, XFS_BUF_TO_AGF(sc->sa.agf_bp),
> >  			sc->sa.agfl_bp, xchk_agfl_block, &sai);
> >  	if (error == XFS_BTREE_QUERY_RANGE_ABORT) {
> > @@ -791,7 +782,6 @@ STATIC void
> >  xchk_agi_xref(
> >  	struct xfs_scrub	*sc)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	struct xfs_mount	*mp = sc->mp;
> >  	xfs_agblock_t		agbno;
> >  	int			error;
> > @@ -808,8 +798,7 @@ xchk_agi_xref(
> >  	xchk_xref_is_used_space(sc, agbno, 1);
> >  	xchk_xref_is_not_inode_chunk(sc, agbno, 1);
> >  	xchk_agi_xref_icounts(sc);
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_FS);
> > -	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
> > +	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_FS);
> >  	xchk_xref_is_not_shared(sc, agbno, 1);
> >  
> >  	/* scrub teardown will take care of sc->sa for us */
> > diff --git a/fs/xfs/scrub/agheader_repair.c b/fs/xfs/scrub/agheader_repair.c
> > index f7568a4b5fe5..03d1e15cceba 100644
> > --- a/fs/xfs/scrub/agheader_repair.c
> > +++ b/fs/xfs/scrub/agheader_repair.c
> > @@ -646,7 +646,6 @@ int
> >  xrep_agfl(
> >  	struct xfs_scrub	*sc)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	struct xfs_bitmap	agfl_extents;
> >  	struct xfs_mount	*mp = sc->mp;
> >  	struct xfs_buf		*agf_bp;
> > @@ -708,8 +707,8 @@ xrep_agfl(
> >  		goto err;
> >  
> >  	/* Dump any AGFL overflow. */
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
> > -	return xrep_reap_extents(sc, &agfl_extents, &oinfo, XFS_AG_RESV_AGFL);
> > +	return xrep_reap_extents(sc, &agfl_extents, &XFS_RMAP_OINFO_AG,
> > +			XFS_AG_RESV_AGFL);
> >  err:
> >  	xfs_bitmap_destroy(&agfl_extents);
> >  	return error;
> > diff --git a/fs/xfs/scrub/alloc.c b/fs/xfs/scrub/alloc.c
> > index 376bcb585ae6..44883e9112ad 100644
> > --- a/fs/xfs/scrub/alloc.c
> > +++ b/fs/xfs/scrub/alloc.c
> > @@ -125,12 +125,10 @@ xchk_allocbt(
> >  	struct xfs_scrub	*sc,
> >  	xfs_btnum_t		which)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	struct xfs_btree_cur	*cur;
> >  
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
> >  	cur = which == XFS_BTNUM_BNO ? sc->sa.bno_cur : sc->sa.cnt_cur;
> > -	return xchk_btree(sc, cur, xchk_allocbt_rec, &oinfo, NULL);
> > +	return xchk_btree(sc, cur, xchk_allocbt_rec, &XFS_RMAP_OINFO_AG, NULL);
> >  }
> >  
> >  int
> > diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
> > index 224dba937492..72f45b298fa5 100644
> > --- a/fs/xfs/scrub/ialloc.c
> > +++ b/fs/xfs/scrub/ialloc.c
> > @@ -82,15 +82,12 @@ xchk_iallocbt_chunk_xref(
> >  	xfs_agblock_t			agbno,
> >  	xfs_extlen_t			len)
> >  {
> > -	struct xfs_owner_info		oinfo;
> > -
> >  	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> >  		return;
> >  
> >  	xchk_xref_is_used_space(sc, agbno, len);
> >  	xchk_iallocbt_chunk_xref_other(sc, irec, agino);
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> > -	xchk_xref_is_owned_by(sc, agbno, len, &oinfo);
> > +	xchk_xref_is_owned_by(sc, agbno, len, &XFS_RMAP_OINFO_INODES);
> >  	xchk_xref_is_not_shared(sc, agbno, len);
> >  }
> >  
> > @@ -186,7 +183,6 @@ xchk_iallocbt_check_freemask(
> >  	struct xchk_btree		*bs,
> >  	struct xfs_inobt_rec_incore	*irec)
> >  {
> > -	struct xfs_owner_info		oinfo;
> >  	struct xfs_imap			imap;
> >  	struct xfs_mount		*mp = bs->cur->bc_mp;
> >  	struct xfs_dinode		*dip;
> > @@ -205,7 +201,6 @@ xchk_iallocbt_check_freemask(
> >  	/* Make sure the freemask matches the inode records. */
> >  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> >  	nr_inodes = XFS_OFFBNO_TO_AGINO(mp, blks_per_cluster, 0);
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> >  
> >  	for (agino = irec->ir_startino;
> >  	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
> > @@ -230,12 +225,13 @@ xchk_iallocbt_check_freemask(
> >  		/* If any part of this is a hole, skip it. */
> >  		if (ir_holemask) {
> >  			xchk_xref_is_not_owned_by(bs->sc, agbno,
> > -					blks_per_cluster, &oinfo);
> > +					blks_per_cluster,
> > +					&XFS_RMAP_OINFO_INODES);
> >  			continue;
> >  		}
> >  
> >  		xchk_xref_is_owned_by(bs->sc, agbno, blks_per_cluster,
> > -				&oinfo);
> > +				&XFS_RMAP_OINFO_INODES);
> >  
> >  		/* Grab the inode cluster buffer. */
> >  		imap.im_blkno = XFS_AGB_TO_DADDR(mp, bs->cur->bc_private.a.agno,
> > @@ -366,7 +362,6 @@ xchk_iallocbt_xref_rmap_btreeblks(
> >  	struct xfs_scrub	*sc,
> >  	int			which)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	xfs_filblks_t		blocks;
> >  	xfs_extlen_t		inobt_blocks = 0;
> >  	xfs_extlen_t		finobt_blocks = 0;
> > @@ -388,9 +383,8 @@ xchk_iallocbt_xref_rmap_btreeblks(
> >  			return;
> >  	}
> >  
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
> > -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> > -			&blocks);
> > +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> > +			&XFS_RMAP_OINFO_INOBT, &blocks);
> >  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
> >  		return;
> >  	if (blocks != inobt_blocks + finobt_blocks)
> > @@ -407,7 +401,6 @@ xchk_iallocbt_xref_rmap_inodes(
> >  	int			which,
> >  	xfs_filblks_t		inode_blocks)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	xfs_filblks_t		blocks;
> >  	int			error;
> >  
> > @@ -415,9 +408,8 @@ xchk_iallocbt_xref_rmap_inodes(
> >  		return;
> >  
> >  	/* Check that we saw as many inode blocks as the rmap knows about. */
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> > -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> > -			&blocks);
> > +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> > +			&XFS_RMAP_OINFO_INODES, &blocks);
> >  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
> >  		return;
> >  	if (blocks != inode_blocks)
> > @@ -431,13 +423,11 @@ xchk_iallocbt(
> >  	xfs_btnum_t		which)
> >  {
> >  	struct xfs_btree_cur	*cur;
> > -	struct xfs_owner_info	oinfo;
> >  	xfs_filblks_t		inode_blocks = 0;
> >  	int			error;
> >  
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INOBT);
> >  	cur = which == XFS_BTNUM_INO ? sc->sa.ino_cur : sc->sa.fino_cur;
> > -	error = xchk_btree(sc, cur, xchk_iallocbt_rec, &oinfo,
> > +	error = xchk_btree(sc, cur, xchk_iallocbt_rec, &XFS_RMAP_OINFO_INOBT,
> >  			&inode_blocks);
> >  	if (error)
> >  		return error;
> > diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> > index e386c9b0b4ab..e213efc194a1 100644
> > --- a/fs/xfs/scrub/inode.c
> > +++ b/fs/xfs/scrub/inode.c
> > @@ -509,7 +509,6 @@ xchk_inode_xref(
> >  	xfs_ino_t		ino,
> >  	struct xfs_dinode	*dip)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	xfs_agnumber_t		agno;
> >  	xfs_agblock_t		agbno;
> >  	int			error;
> > @@ -526,8 +525,7 @@ xchk_inode_xref(
> >  
> >  	xchk_xref_is_used_space(sc, agbno, 1);
> >  	xchk_inode_xref_finobt(sc, ino);
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_INODES);
> > -	xchk_xref_is_owned_by(sc, agbno, 1, &oinfo);
> > +	xchk_xref_is_owned_by(sc, agbno, 1, &XFS_RMAP_OINFO_INODES);
> >  	xchk_xref_is_not_shared(sc, agbno, 1);
> >  	xchk_inode_xref_bmap(sc, dip);
> >  
> > diff --git a/fs/xfs/scrub/refcount.c b/fs/xfs/scrub/refcount.c
> > index b7ade620acee..708b4158eb90 100644
> > --- a/fs/xfs/scrub/refcount.c
> > +++ b/fs/xfs/scrub/refcount.c
> > @@ -385,7 +385,6 @@ xchk_refcount_xref_rmap(
> >  	struct xfs_scrub	*sc,
> >  	xfs_filblks_t		cow_blocks)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	xfs_extlen_t		refcbt_blocks = 0;
> >  	xfs_filblks_t		blocks;
> >  	int			error;
> > @@ -394,21 +393,19 @@ xchk_refcount_xref_rmap(
> >  		return;
> >  
> >  	/* Check that we saw as many refcbt blocks as the rmap knows about. */
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
> >  	error = xfs_btree_count_blocks(sc->sa.refc_cur, &refcbt_blocks);
> >  	if (!xchk_btree_process_error(sc, sc->sa.refc_cur, 0, &error))
> >  		return;
> > -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> > -			&blocks);
> > +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> > +			&XFS_RMAP_OINFO_REFC, &blocks);
> >  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
> >  		return;
> >  	if (blocks != refcbt_blocks)
> >  		xchk_btree_xref_set_corrupt(sc, sc->sa.rmap_cur, 0);
> >  
> >  	/* Check that we saw as many cow blocks as the rmap knows about. */
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_COW);
> > -	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur, &oinfo,
> > -			&blocks);
> > +	error = xchk_count_rmap_ownedby_ag(sc, sc->sa.rmap_cur,
> > +			&XFS_RMAP_OINFO_COW, &blocks);
> >  	if (!xchk_should_check_xref(sc, &error, &sc->sa.rmap_cur))
> >  		return;
> >  	if (blocks != cow_blocks)
> > @@ -420,13 +417,11 @@ int
> >  xchk_refcountbt(
> >  	struct xfs_scrub	*sc)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	xfs_agblock_t		cow_blocks = 0;
> >  	int			error;
> >  
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_REFC);
> >  	error = xchk_btree(sc, sc->sa.refc_cur, xchk_refcountbt_rec,
> > -			&oinfo, &cow_blocks);
> > +			&XFS_RMAP_OINFO_REFC, &cow_blocks);
> >  	if (error)
> >  		return error;
> >  
> > diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c
> > index 90ae9e173de7..1c8eecfe52b8 100644
> > --- a/fs/xfs/scrub/repair.c
> > +++ b/fs/xfs/scrub/repair.c
> > @@ -505,7 +505,6 @@ xrep_put_freelist(
> >  	struct xfs_scrub	*sc,
> >  	xfs_agblock_t		agbno)
> >  {
> > -	struct xfs_owner_info	oinfo;
> >  	int			error;
> >  
> >  	/* Make sure there's space on the freelist. */
> > @@ -518,9 +517,8 @@ xrep_put_freelist(
> >  	 * create an rmap for the block prior to merging it or else other
> >  	 * parts will break.
> >  	 */
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
> >  	error = xfs_rmap_alloc(sc->tp, sc->sa.agf_bp, sc->sa.agno, agbno, 1,
> > -			&oinfo);
> > +			&XFS_RMAP_OINFO_AG);
> >  	if (error)
> >  		return error;
> >  
> > diff --git a/fs/xfs/scrub/rmap.c b/fs/xfs/scrub/rmap.c
> > index 8a5bf15d544d..92a140c5b55e 100644
> > --- a/fs/xfs/scrub/rmap.c
> > +++ b/fs/xfs/scrub/rmap.c
> > @@ -174,11 +174,8 @@ int
> >  xchk_rmapbt(
> >  	struct xfs_scrub	*sc)
> >  {
> > -	struct xfs_owner_info	oinfo;
> > -
> > -	xfs_rmap_ag_owner(&oinfo, XFS_RMAP_OWN_AG);
> >  	return xchk_btree(sc, sc->sa.rmap_cur, xchk_rmapbt_rec,
> > -			&oinfo, NULL);
> > +			&XFS_RMAP_OINFO_AG, NULL);
> >  }
> >  
> >  /* xref check that the extent is owned by a given owner */
> > diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> > index d9da66c718bb..74ddf66f4cfe 100644
> > --- a/fs/xfs/xfs_extfree_item.c
> > +++ b/fs/xfs/xfs_extfree_item.c
> > @@ -494,7 +494,6 @@ xfs_efi_recover(
> >  	int			error = 0;
> >  	xfs_extent_t		*extp;
> >  	xfs_fsblock_t		startblock_fsb;
> > -	struct xfs_owner_info	oinfo;
> >  
> >  	ASSERT(!test_bit(XFS_EFI_RECOVERED, &efip->efi_flags));
> >  
> > @@ -526,11 +525,11 @@ xfs_efi_recover(
> >  		return error;
> >  	efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
> >  
> > -	xfs_rmap_any_owner_update(&oinfo);
> >  	for (i = 0; i < efip->efi_format.efi_nextents; i++) {
> >  		extp = &efip->efi_format.efi_extents[i];
> >  		error = xfs_trans_free_extent(tp, efdp, extp->ext_start,
> > -					      extp->ext_len, &oinfo, false);
> > +					      extp->ext_len,
> > +					      &XFS_RMAP_OINFO_ANY_OWNER, false);
> >  		if (error)
> >  			goto abort_error;
> >  
> > 

[PATCH 3/5] xfs: add a block to inode count converter

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Add new helpers to convert units of fs blocks into inodes, and AG blocks
into AG inodes, respectively.  Convert all the open-coded conversions
and XFS_OFFBNO_TO_AGINO(, , 0) calls to use them, as appropriate.  The
OFFBNO_TO_AGINO macro is retained for xfs_repair.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_format.h |    2 ++
 fs/xfs/libxfs/xfs_ialloc.c |   17 ++++++++---------
 fs/xfs/libxfs/xfs_types.c  |    4 ++--
 fs/xfs/scrub/ialloc.c      |    2 +-
 fs/xfs/xfs_fsops.c         |    2 +-
 fs/xfs/xfs_inode.c         |    2 +-
 fs/xfs/xfs_itable.c        |    2 +-
 fs/xfs/xfs_super.c         |    4 ++--
 8 files changed, 18 insertions(+), 17 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 9995d5ae380b..52cf83986936 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1083,6 +1083,8 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
 	((i) & XFS_INO_MASK(XFS_INO_OFFSET_BITS(mp)))
 #define	XFS_OFFBNO_TO_AGINO(mp,b,o)	\
 	((xfs_agino_t)(((b) << XFS_INO_OFFSET_BITS(mp)) | (o)))
+#define	XFS_FSB_TO_INO(mp, b)	((xfs_ino_t)((b) << (mp)->m_sb.sb_inopblog))
+#define	XFS_AGB_TO_AGINO(mp, b)	((xfs_agino_t)((b) << (mp)->m_sb.sb_inopblog))
 
 #define	XFS_MAXINUMBER		((xfs_ino_t)((1ULL << 56) - 1ULL))
 #define	XFS_MAXINUMBER_32	((xfs_ino_t)((1ULL << 32) - 1ULL))
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index fcf0d17405d8..cb46141f9d15 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -300,7 +300,7 @@ xfs_ialloc_inode_init(
 	 * blocks size.
 	 */
 	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
+	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
 	nbufs = length / blks_per_cluster;
 
 	/*
@@ -312,7 +312,7 @@ xfs_ialloc_inode_init(
 	 *
 	 * For v3 inodes, we also need to write the inode number into the inode,
 	 * so calculate the first inode number of the chunk here as
-	 * XFS_OFFBNO_TO_AGINO() only works within a filesystem block, not
+	 * XFS_AGB_TO_AGINO() only works within a filesystem block, not
 	 * across multiple filesystem blocks (such as a cluster) and so cannot
 	 * be used in the cluster buffer loop below.
 	 *
@@ -324,8 +324,7 @@ xfs_ialloc_inode_init(
 	 */
 	if (xfs_sb_version_hascrc(&mp->m_sb)) {
 		version = 3;
-		ino = XFS_AGINO_TO_INO(mp, agno,
-				       XFS_OFFBNO_TO_AGINO(mp, agbno, 0));
+		ino = XFS_AGINO_TO_INO(mp, agno, XFS_AGB_TO_AGINO(mp, agbno));
 
 		/*
 		 * log the initialisation that is about to take place as an
@@ -445,7 +444,7 @@ xfs_align_sparse_ino(
 		return;
 
 	/* calculate the inode offset and align startino */
-	offset = mod << mp->m_sb.sb_inopblog;
+	offset = XFS_AGB_TO_AGINO(mp, mod);
 	*startino -= offset;
 
 	/*
@@ -797,7 +796,7 @@ xfs_ialloc_ag_alloc(
 		if (error)
 			return error;
 
-		newlen = args.len << args.mp->m_sb.sb_inopblog;
+		newlen = XFS_AGB_TO_AGINO(args.mp, args.len);
 		ASSERT(newlen <= XFS_INODES_PER_CHUNK);
 		allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
 	}
@@ -825,7 +824,7 @@ xfs_ialloc_ag_alloc(
 	/*
 	 * Convert the results.
 	 */
-	newino = XFS_OFFBNO_TO_AGINO(args.mp, args.agbno, 0);
+	newino = XFS_FSB_TO_INO(args.mp, args.agbno);
 
 	if (xfs_inobt_issparse(~allocmask)) {
 		/*
@@ -2724,8 +2723,8 @@ xfs_ialloc_has_inodes_at_extent(
 	xfs_agino_t		low;
 	xfs_agino_t		high;
 
-	low = XFS_OFFBNO_TO_AGINO(cur->bc_mp, bno, 0);
-	high = XFS_OFFBNO_TO_AGINO(cur->bc_mp, bno + len, 0) - 1;
+	low = XFS_AGB_TO_AGINO(cur->bc_mp, bno);
+	high = XFS_AGB_TO_AGINO(cur->bc_mp, bno + len) - 1;
 
 	return xfs_ialloc_has_inode_record(cur, low, high, exists);
 }
diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
index 33a5ca346baf..895c232d63b7 100644
--- a/fs/xfs/libxfs/xfs_types.c
+++ b/fs/xfs/libxfs/xfs_types.c
@@ -89,14 +89,14 @@ xfs_agino_range(
 	 */
 	bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
 			xfs_ialloc_cluster_alignment(mp));
-	*first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
+	*first = XFS_AGB_TO_AGINO(mp, bno);
 
 	/*
 	 * Calculate the last inode, which will be at the end of the
 	 * last (aligned) cluster that can be allocated in the AG.
 	 */
 	bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
-	*last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
+	*last = XFS_AGB_TO_AGINO(mp, bno) - 1;
 }
 
 /*
diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
index 72f45b298fa5..426eb1a5503c 100644
--- a/fs/xfs/scrub/ialloc.c
+++ b/fs/xfs/scrub/ialloc.c
@@ -200,7 +200,7 @@ xchk_iallocbt_check_freemask(
 
 	/* Make sure the freemask matches the inode records. */
 	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	nr_inodes = XFS_OFFBNO_TO_AGINO(mp, blks_per_cluster, 0);
+	nr_inodes = XFS_FSB_TO_INO(mp, blks_per_cluster);
 
 	for (agino = irec->ir_startino;
 	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 093c2b8d7e20..ec2e63a7963b 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -252,7 +252,7 @@ xfs_growfs_data(
 	if (mp->m_sb.sb_imax_pct) {
 		uint64_t icount = mp->m_sb.sb_dblocks * mp->m_sb.sb_imax_pct;
 		do_div(icount, 100);
-		mp->m_maxicount = icount << mp->m_sb.sb_inopblog;
+		mp->m_maxicount = XFS_FSB_TO_INO(mp, icount);
 	} else
 		mp->m_maxicount = 0;
 
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 05db9540e459..55b865dadfaa 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2200,7 +2200,7 @@ xfs_ifree_cluster(
 	inum = xic->first_ino;
 	pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
 	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
+	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
 	nbufs = mp->m_ialloc_blks / blks_per_cluster;
 
 	for (j = 0; j < nbufs; j++, inum += inodes_per_cluster) {
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index e9508ba01ed1..18d8d3b812a7 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -173,7 +173,7 @@ xfs_bulkstat_ichunk_ra(
 
 	agbno = XFS_AGINO_TO_AGBNO(mp, irec->ir_startino);
 	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
+	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
 
 	blk_start_plug(&plug);
 	for (i = 0; i < XFS_INODES_PER_CHUNK;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index a2e944b80d2a..c9097cb0b955 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -608,7 +608,7 @@ xfs_set_inode_alloc(
 	}
 
 	/* Get the last possible inode in the filesystem */
-	agino =	XFS_OFFBNO_TO_AGINO(mp, sbp->sb_agblocks - 1, 0);
+	agino =	XFS_AGB_TO_AGINO(mp, sbp->sb_agblocks - 1);
 	ino = XFS_AGINO_TO_INO(mp, agcount - 1, agino);
 
 	/*
@@ -1150,7 +1150,7 @@ xfs_fs_statfs(
 	statp->f_bfree = fdblocks - mp->m_alloc_set_aside;
 	statp->f_bavail = statp->f_bfree;
 
-	fakeinos = statp->f_bfree << sbp->sb_inopblog;
+	fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
 	statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
 	if (mp->m_maxicount)
 		statp->f_files = min_t(typeof(statp->f_files),

Re: [PATCH 3/5] xfs: add a block to inode count converter

[Brian Foster]

On Thu, Nov 08, 2018 at 03:20:27PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Add new helpers to convert units of fs blocks into inodes, and AG blocks
> into AG inodes, respectively.  Convert all the open-coded conversions
> and XFS_OFFBNO_TO_AGINO(, , 0) calls to use them, as appropriate.  The
> OFFBNO_TO_AGINO macro is retained for xfs_repair.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_format.h |    2 ++
>  fs/xfs/libxfs/xfs_ialloc.c |   17 ++++++++---------
>  fs/xfs/libxfs/xfs_types.c  |    4 ++--
>  fs/xfs/scrub/ialloc.c      |    2 +-
>  fs/xfs/xfs_fsops.c         |    2 +-
>  fs/xfs/xfs_inode.c         |    2 +-
>  fs/xfs/xfs_itable.c        |    2 +-
>  fs/xfs/xfs_super.c         |    4 ++--
>  8 files changed, 18 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 9995d5ae380b..52cf83986936 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1083,6 +1083,8 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
>  	((i) & XFS_INO_MASK(XFS_INO_OFFSET_BITS(mp)))
>  #define	XFS_OFFBNO_TO_AGINO(mp,b,o)	\
>  	((xfs_agino_t)(((b) << XFS_INO_OFFSET_BITS(mp)) | (o)))
> +#define	XFS_FSB_TO_INO(mp, b)	((xfs_ino_t)((b) << (mp)->m_sb.sb_inopblog))
> +#define	XFS_AGB_TO_AGINO(mp, b)	((xfs_agino_t)((b) << (mp)->m_sb.sb_inopblog))

Any reason not to use XFS_INO_OFFSET_BITS() like the other macros do in
place of sb_inopblog?

>  
>  #define	XFS_MAXINUMBER		((xfs_ino_t)((1ULL << 56) - 1ULL))
>  #define	XFS_MAXINUMBER_32	((xfs_ino_t)((1ULL << 32) - 1ULL))
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index fcf0d17405d8..cb46141f9d15 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
...
> @@ -825,7 +824,7 @@ xfs_ialloc_ag_alloc(
>  	/*
>  	 * Convert the results.
>  	 */
> -	newino = XFS_OFFBNO_TO_AGINO(args.mp, args.agbno, 0);
> +	newino = XFS_FSB_TO_INO(args.mp, args.agbno);

Looks like this should be the AG variant.

Otherwise looks fine:

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

>  
>  	if (xfs_inobt_issparse(~allocmask)) {
>  		/*
> @@ -2724,8 +2723,8 @@ xfs_ialloc_has_inodes_at_extent(
>  	xfs_agino_t		low;
>  	xfs_agino_t		high;
>  
> -	low = XFS_OFFBNO_TO_AGINO(cur->bc_mp, bno, 0);
> -	high = XFS_OFFBNO_TO_AGINO(cur->bc_mp, bno + len, 0) - 1;
> +	low = XFS_AGB_TO_AGINO(cur->bc_mp, bno);
> +	high = XFS_AGB_TO_AGINO(cur->bc_mp, bno + len) - 1;
>  
>  	return xfs_ialloc_has_inode_record(cur, low, high, exists);
>  }
> diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> index 33a5ca346baf..895c232d63b7 100644
> --- a/fs/xfs/libxfs/xfs_types.c
> +++ b/fs/xfs/libxfs/xfs_types.c
> @@ -89,14 +89,14 @@ xfs_agino_range(
>  	 */
>  	bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
>  			xfs_ialloc_cluster_alignment(mp));
> -	*first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> +	*first = XFS_AGB_TO_AGINO(mp, bno);
>  
>  	/*
>  	 * Calculate the last inode, which will be at the end of the
>  	 * last (aligned) cluster that can be allocated in the AG.
>  	 */
>  	bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> -	*last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> +	*last = XFS_AGB_TO_AGINO(mp, bno) - 1;
>  }
>  
>  /*
> diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
> index 72f45b298fa5..426eb1a5503c 100644
> --- a/fs/xfs/scrub/ialloc.c
> +++ b/fs/xfs/scrub/ialloc.c
> @@ -200,7 +200,7 @@ xchk_iallocbt_check_freemask(
>  
>  	/* Make sure the freemask matches the inode records. */
>  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> -	nr_inodes = XFS_OFFBNO_TO_AGINO(mp, blks_per_cluster, 0);
> +	nr_inodes = XFS_FSB_TO_INO(mp, blks_per_cluster);
>  
>  	for (agino = irec->ir_startino;
>  	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 093c2b8d7e20..ec2e63a7963b 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -252,7 +252,7 @@ xfs_growfs_data(
>  	if (mp->m_sb.sb_imax_pct) {
>  		uint64_t icount = mp->m_sb.sb_dblocks * mp->m_sb.sb_imax_pct;
>  		do_div(icount, 100);
> -		mp->m_maxicount = icount << mp->m_sb.sb_inopblog;
> +		mp->m_maxicount = XFS_FSB_TO_INO(mp, icount);
>  	} else
>  		mp->m_maxicount = 0;
>  
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 05db9540e459..55b865dadfaa 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2200,7 +2200,7 @@ xfs_ifree_cluster(
>  	inum = xic->first_ino;
>  	pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
>  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> -	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
> +	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
>  	nbufs = mp->m_ialloc_blks / blks_per_cluster;
>  
>  	for (j = 0; j < nbufs; j++, inum += inodes_per_cluster) {
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index e9508ba01ed1..18d8d3b812a7 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -173,7 +173,7 @@ xfs_bulkstat_ichunk_ra(
>  
>  	agbno = XFS_AGINO_TO_AGBNO(mp, irec->ir_startino);
>  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> -	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
> +	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
>  
>  	blk_start_plug(&plug);
>  	for (i = 0; i < XFS_INODES_PER_CHUNK;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index a2e944b80d2a..c9097cb0b955 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -608,7 +608,7 @@ xfs_set_inode_alloc(
>  	}
>  
>  	/* Get the last possible inode in the filesystem */
> -	agino =	XFS_OFFBNO_TO_AGINO(mp, sbp->sb_agblocks - 1, 0);
> +	agino =	XFS_AGB_TO_AGINO(mp, sbp->sb_agblocks - 1);
>  	ino = XFS_AGINO_TO_INO(mp, agcount - 1, agino);
>  
>  	/*
> @@ -1150,7 +1150,7 @@ xfs_fs_statfs(
>  	statp->f_bfree = fdblocks - mp->m_alloc_set_aside;
>  	statp->f_bavail = statp->f_bfree;
>  
> -	fakeinos = statp->f_bfree << sbp->sb_inopblog;
> +	fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
>  	statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
>  	if (mp->m_maxicount)
>  		statp->f_files = min_t(typeof(statp->f_files),
> 

Re: [PATCH 3/5] xfs: add a block to inode count converter

[Darrick J. Wong]

On Tue, Nov 20, 2018 at 11:21:06AM -0500, Brian Foster wrote:
> On Thu, Nov 08, 2018 at 03:20:27PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Add new helpers to convert units of fs blocks into inodes, and AG blocks
> > into AG inodes, respectively.  Convert all the open-coded conversions
> > and XFS_OFFBNO_TO_AGINO(, , 0) calls to use them, as appropriate.  The
> > OFFBNO_TO_AGINO macro is retained for xfs_repair.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_format.h |    2 ++
> >  fs/xfs/libxfs/xfs_ialloc.c |   17 ++++++++---------
> >  fs/xfs/libxfs/xfs_types.c  |    4 ++--
> >  fs/xfs/scrub/ialloc.c      |    2 +-
> >  fs/xfs/xfs_fsops.c         |    2 +-
> >  fs/xfs/xfs_inode.c         |    2 +-
> >  fs/xfs/xfs_itable.c        |    2 +-
> >  fs/xfs/xfs_super.c         |    4 ++--
> >  8 files changed, 18 insertions(+), 17 deletions(-)
> > 
> > 
> > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> > index 9995d5ae380b..52cf83986936 100644
> > --- a/fs/xfs/libxfs/xfs_format.h
> > +++ b/fs/xfs/libxfs/xfs_format.h
> > @@ -1083,6 +1083,8 @@ static inline void xfs_dinode_put_rdev(struct xfs_dinode *dip, xfs_dev_t rdev)
> >  	((i) & XFS_INO_MASK(XFS_INO_OFFSET_BITS(mp)))
> >  #define	XFS_OFFBNO_TO_AGINO(mp,b,o)	\
> >  	((xfs_agino_t)(((b) << XFS_INO_OFFSET_BITS(mp)) | (o)))
> > +#define	XFS_FSB_TO_INO(mp, b)	((xfs_ino_t)((b) << (mp)->m_sb.sb_inopblog))
> > +#define	XFS_AGB_TO_AGINO(mp, b)	((xfs_agino_t)((b) << (mp)->m_sb.sb_inopblog))
> 
> Any reason not to use XFS_INO_OFFSET_BITS() like the other macros do in
> place of sb_inopblog?

Dislike of nested macros. :)

> >  
> >  #define	XFS_MAXINUMBER		((xfs_ino_t)((1ULL << 56) - 1ULL))
> >  #define	XFS_MAXINUMBER_32	((xfs_ino_t)((1ULL << 32) - 1ULL))
> > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> > index fcf0d17405d8..cb46141f9d15 100644
> > --- a/fs/xfs/libxfs/xfs_ialloc.c
> > +++ b/fs/xfs/libxfs/xfs_ialloc.c
> ...
> > @@ -825,7 +824,7 @@ xfs_ialloc_ag_alloc(
> >  	/*
> >  	 * Convert the results.
> >  	 */
> > -	newino = XFS_OFFBNO_TO_AGINO(args.mp, args.agbno, 0);
> > +	newino = XFS_FSB_TO_INO(args.mp, args.agbno);
> 
> Looks like this should be the AG variant.

Fixed.  Thanks for catching that. :)

--D

> Otherwise looks fine:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> >  
> >  	if (xfs_inobt_issparse(~allocmask)) {
> >  		/*
> > @@ -2724,8 +2723,8 @@ xfs_ialloc_has_inodes_at_extent(
> >  	xfs_agino_t		low;
> >  	xfs_agino_t		high;
> >  
> > -	low = XFS_OFFBNO_TO_AGINO(cur->bc_mp, bno, 0);
> > -	high = XFS_OFFBNO_TO_AGINO(cur->bc_mp, bno + len, 0) - 1;
> > +	low = XFS_AGB_TO_AGINO(cur->bc_mp, bno);
> > +	high = XFS_AGB_TO_AGINO(cur->bc_mp, bno + len) - 1;
> >  
> >  	return xfs_ialloc_has_inode_record(cur, low, high, exists);
> >  }
> > diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
> > index 33a5ca346baf..895c232d63b7 100644
> > --- a/fs/xfs/libxfs/xfs_types.c
> > +++ b/fs/xfs/libxfs/xfs_types.c
> > @@ -89,14 +89,14 @@ xfs_agino_range(
> >  	 */
> >  	bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
> >  			xfs_ialloc_cluster_alignment(mp));
> > -	*first = XFS_OFFBNO_TO_AGINO(mp, bno, 0);
> > +	*first = XFS_AGB_TO_AGINO(mp, bno);
> >  
> >  	/*
> >  	 * Calculate the last inode, which will be at the end of the
> >  	 * last (aligned) cluster that can be allocated in the AG.
> >  	 */
> >  	bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
> > -	*last = XFS_OFFBNO_TO_AGINO(mp, bno, 0) - 1;
> > +	*last = XFS_AGB_TO_AGINO(mp, bno) - 1;
> >  }
> >  
> >  /*
> > diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
> > index 72f45b298fa5..426eb1a5503c 100644
> > --- a/fs/xfs/scrub/ialloc.c
> > +++ b/fs/xfs/scrub/ialloc.c
> > @@ -200,7 +200,7 @@ xchk_iallocbt_check_freemask(
> >  
> >  	/* Make sure the freemask matches the inode records. */
> >  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> > -	nr_inodes = XFS_OFFBNO_TO_AGINO(mp, blks_per_cluster, 0);
> > +	nr_inodes = XFS_FSB_TO_INO(mp, blks_per_cluster);
> >  
> >  	for (agino = irec->ir_startino;
> >  	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
> > diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> > index 093c2b8d7e20..ec2e63a7963b 100644
> > --- a/fs/xfs/xfs_fsops.c
> > +++ b/fs/xfs/xfs_fsops.c
> > @@ -252,7 +252,7 @@ xfs_growfs_data(
> >  	if (mp->m_sb.sb_imax_pct) {
> >  		uint64_t icount = mp->m_sb.sb_dblocks * mp->m_sb.sb_imax_pct;
> >  		do_div(icount, 100);
> > -		mp->m_maxicount = icount << mp->m_sb.sb_inopblog;
> > +		mp->m_maxicount = XFS_FSB_TO_INO(mp, icount);
> >  	} else
> >  		mp->m_maxicount = 0;
> >  
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 05db9540e459..55b865dadfaa 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -2200,7 +2200,7 @@ xfs_ifree_cluster(
> >  	inum = xic->first_ino;
> >  	pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
> >  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> > -	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
> > +	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
> >  	nbufs = mp->m_ialloc_blks / blks_per_cluster;
> >  
> >  	for (j = 0; j < nbufs; j++, inum += inodes_per_cluster) {
> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index e9508ba01ed1..18d8d3b812a7 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -173,7 +173,7 @@ xfs_bulkstat_ichunk_ra(
> >  
> >  	agbno = XFS_AGINO_TO_AGBNO(mp, irec->ir_startino);
> >  	blks_per_cluster = xfs_icluster_size_fsb(mp);
> > -	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
> > +	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
> >  
> >  	blk_start_plug(&plug);
> >  	for (i = 0; i < XFS_INODES_PER_CHUNK;
> > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> > index a2e944b80d2a..c9097cb0b955 100644
> > --- a/fs/xfs/xfs_super.c
> > +++ b/fs/xfs/xfs_super.c
> > @@ -608,7 +608,7 @@ xfs_set_inode_alloc(
> >  	}
> >  
> >  	/* Get the last possible inode in the filesystem */
> > -	agino =	XFS_OFFBNO_TO_AGINO(mp, sbp->sb_agblocks - 1, 0);
> > +	agino =	XFS_AGB_TO_AGINO(mp, sbp->sb_agblocks - 1);
> >  	ino = XFS_AGINO_TO_INO(mp, agcount - 1, agino);
> >  
> >  	/*
> > @@ -1150,7 +1150,7 @@ xfs_fs_statfs(
> >  	statp->f_bfree = fdblocks - mp->m_alloc_set_aside;
> >  	statp->f_bavail = statp->f_bfree;
> >  
> > -	fakeinos = statp->f_bfree << sbp->sb_inopblog;
> > +	fakeinos = XFS_FSB_TO_INO(mp, statp->f_bfree);
> >  	statp->f_files = min(icount + fakeinos, (uint64_t)XFS_MAXINUMBER);
> >  	if (mp->m_maxicount)
> >  		statp->f_files = min_t(typeof(statp->f_files),
> > 

[PATCH 4/5] xfs: precalculate inodes and blocks per inode cluster

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Store the number of inodes and blocks per inode cluster in the mount
data so that we don't have to keep recalculating them.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_ialloc.c |   23 ++++++++++-------------
 fs/xfs/scrub/ialloc.c      |   14 ++++++--------
 fs/xfs/xfs_inode.c         |   14 +++++---------
 fs/xfs/xfs_itable.c        |   14 ++++++--------
 fs/xfs/xfs_log_recover.c   |    8 +++-----
 fs/xfs/xfs_mount.c         |    2 ++
 fs/xfs/xfs_mount.h         |    2 ++
 7 files changed, 34 insertions(+), 43 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index cb46141f9d15..dfcd739edcec 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -288,7 +288,7 @@ xfs_ialloc_inode_init(
 {
 	struct xfs_buf		*fbuf;
 	struct xfs_dinode	*free;
-	int			nbufs, blks_per_cluster, inodes_per_cluster;
+	int			nbufs;
 	int			version;
 	int			i, j;
 	xfs_daddr_t		d;
@@ -299,9 +299,7 @@ xfs_ialloc_inode_init(
 	 * sizes, manipulate the inodes in buffers  which are multiples of the
 	 * blocks size.
 	 */
-	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
-	nbufs = length / blks_per_cluster;
+	nbufs = length / mp->m_blocks_per_cluster;
 
 	/*
 	 * Figure out what version number to use in the inodes we create.  If
@@ -344,9 +342,10 @@ xfs_ialloc_inode_init(
 		/*
 		 * Get the block.
 		 */
-		d = XFS_AGB_TO_DADDR(mp, agno, agbno + (j * blks_per_cluster));
+		d = XFS_AGB_TO_DADDR(mp, agno, agbno +
+				(j * mp->m_blocks_per_cluster));
 		fbuf = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
-					 mp->m_bsize * blks_per_cluster,
+					 mp->m_bsize * mp->m_blocks_per_cluster,
 					 XBF_UNMAPPED);
 		if (!fbuf)
 			return -ENOMEM;
@@ -354,7 +353,7 @@ xfs_ialloc_inode_init(
 		/* Initialize the inode buffers and log them appropriately. */
 		fbuf->b_ops = &xfs_inode_buf_ops;
 		xfs_buf_zero(fbuf, 0, BBTOB(fbuf->b_length));
-		for (i = 0; i < inodes_per_cluster; i++) {
+		for (i = 0; i < mp->m_inodes_per_cluster; i++) {
 			int	ioffset = i << mp->m_sb.sb_inodelog;
 			uint	isize = xfs_dinode_size(version);
 
@@ -2289,7 +2288,6 @@ xfs_imap(
 	xfs_agblock_t	agbno;	/* block number of inode in the alloc group */
 	xfs_agino_t	agino;	/* inode number within alloc group */
 	xfs_agnumber_t	agno;	/* allocation group number */
-	int		blks_per_cluster; /* num blocks per inode cluster */
 	xfs_agblock_t	chunk_agbno;	/* first block in inode chunk */
 	xfs_agblock_t	cluster_agbno;	/* first block in inode cluster */
 	int		error;	/* error code */
@@ -2335,8 +2333,6 @@ xfs_imap(
 		return -EINVAL;
 	}
 
-	blks_per_cluster = xfs_icluster_size_fsb(mp);
-
 	/*
 	 * For bulkstat and handle lookups, we have an untrusted inode number
 	 * that we have to verify is valid. We cannot do this just by reading
@@ -2356,7 +2352,7 @@ xfs_imap(
 	 * If the inode cluster size is the same as the blocksize or
 	 * smaller we get to the buffer by simple arithmetics.
 	 */
-	if (blks_per_cluster == 1) {
+	if (mp->m_blocks_per_cluster == 1) {
 		offset = XFS_INO_TO_OFFSET(mp, ino);
 		ASSERT(offset < mp->m_sb.sb_inopblock);
 
@@ -2385,12 +2381,13 @@ xfs_imap(
 out_map:
 	ASSERT(agbno >= chunk_agbno);
 	cluster_agbno = chunk_agbno +
-		((offset_agbno / blks_per_cluster) * blks_per_cluster);
+		((offset_agbno / mp->m_blocks_per_cluster) *
+		 mp->m_blocks_per_cluster);
 	offset = ((agbno - cluster_agbno) * mp->m_sb.sb_inopblock) +
 		XFS_INO_TO_OFFSET(mp, ino);
 
 	imap->im_blkno = XFS_AGB_TO_DADDR(mp, agno, cluster_agbno);
-	imap->im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
+	imap->im_len = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
 	imap->im_boffset = (unsigned short)(offset << mp->m_sb.sb_inodelog);
 
 	/*
diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
index 426eb1a5503c..332ac7b61789 100644
--- a/fs/xfs/scrub/ialloc.c
+++ b/fs/xfs/scrub/ialloc.c
@@ -193,18 +193,16 @@ xchk_iallocbt_check_freemask(
 	xfs_agino_t			chunkino;
 	xfs_agino_t			clusterino;
 	xfs_agblock_t			agbno;
-	int				blks_per_cluster;
 	uint16_t			holemask;
 	uint16_t			ir_holemask;
 	int				error = 0;
 
 	/* Make sure the freemask matches the inode records. */
-	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	nr_inodes = XFS_FSB_TO_INO(mp, blks_per_cluster);
+	nr_inodes = mp->m_inodes_per_cluster;
 
 	for (agino = irec->ir_startino;
 	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
-	     agino += blks_per_cluster * mp->m_sb.sb_inopblock) {
+	     agino += mp->m_blocks_per_cluster * mp->m_sb.sb_inopblock) {
 		fsino = XFS_AGINO_TO_INO(mp, bs->cur->bc_private.a.agno, agino);
 		chunkino = agino - irec->ir_startino;
 		agbno = XFS_AGINO_TO_AGBNO(mp, agino);
@@ -225,18 +223,18 @@ xchk_iallocbt_check_freemask(
 		/* If any part of this is a hole, skip it. */
 		if (ir_holemask) {
 			xchk_xref_is_not_owned_by(bs->sc, agbno,
-					blks_per_cluster,
+					mp->m_blocks_per_cluster,
 					&XFS_RMAP_OINFO_INODES);
 			continue;
 		}
 
-		xchk_xref_is_owned_by(bs->sc, agbno, blks_per_cluster,
+		xchk_xref_is_owned_by(bs->sc, agbno, mp->m_blocks_per_cluster,
 				&XFS_RMAP_OINFO_INODES);
 
 		/* Grab the inode cluster buffer. */
 		imap.im_blkno = XFS_AGB_TO_DADDR(mp, bs->cur->bc_private.a.agno,
 				agbno);
-		imap.im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
+		imap.im_len = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
 		imap.im_boffset = 0;
 
 		error = xfs_imap_to_bp(mp, bs->cur->bc_tp, &imap,
@@ -303,7 +301,7 @@ xchk_iallocbt_rec(
 	/* Make sure this record is aligned to cluster and inoalignmnt size. */
 	agbno = XFS_AGINO_TO_AGBNO(mp, irec.ir_startino);
 	if ((agbno & (xfs_ialloc_cluster_alignment(mp) - 1)) ||
-	    (agbno & (xfs_icluster_size_fsb(mp) - 1)))
+	    (agbno & (mp->m_blocks_per_cluster - 1)))
 		xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
 
 	*inode_blocks += XFS_B_TO_FSB(mp,
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 55b865dadfaa..72607e413baf 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2184,8 +2184,6 @@ xfs_ifree_cluster(
 	struct xfs_icluster	*xic)
 {
 	xfs_mount_t		*mp = free_ip->i_mount;
-	int			blks_per_cluster;
-	int			inodes_per_cluster;
 	int			nbufs;
 	int			i, j;
 	int			ioffset;
@@ -2199,11 +2197,9 @@ xfs_ifree_cluster(
 
 	inum = xic->first_ino;
 	pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
-	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
-	nbufs = mp->m_ialloc_blks / blks_per_cluster;
+	nbufs = mp->m_ialloc_blks / mp->m_blocks_per_cluster;
 
-	for (j = 0; j < nbufs; j++, inum += inodes_per_cluster) {
+	for (j = 0; j < nbufs; j++, inum += mp->m_inodes_per_cluster) {
 		/*
 		 * The allocation bitmap tells us which inodes of the chunk were
 		 * physically allocated. Skip the cluster if an inode falls into
@@ -2211,7 +2207,7 @@ xfs_ifree_cluster(
 		 */
 		ioffset = inum - xic->first_ino;
 		if ((xic->alloc & XFS_INOBT_MASK(ioffset)) == 0) {
-			ASSERT(ioffset % inodes_per_cluster == 0);
+			ASSERT(ioffset % mp->m_inodes_per_cluster == 0);
 			continue;
 		}
 
@@ -2227,7 +2223,7 @@ xfs_ifree_cluster(
 		 * to mark all the active inodes on the buffer stale.
 		 */
 		bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, blkno,
-					mp->m_bsize * blks_per_cluster,
+					mp->m_bsize * mp->m_blocks_per_cluster,
 					XBF_UNMAPPED);
 
 		if (!bp)
@@ -2274,7 +2270,7 @@ xfs_ifree_cluster(
 		 * transaction stale above, which means there is no point in
 		 * even trying to lock them.
 		 */
-		for (i = 0; i < inodes_per_cluster; i++) {
+		for (i = 0; i < mp->m_inodes_per_cluster; i++) {
 retry:
 			rcu_read_lock();
 			ip = radix_tree_lookup(&pag->pag_ici_root,
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 18d8d3b812a7..00828b57e1f8 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -167,20 +167,18 @@ xfs_bulkstat_ichunk_ra(
 {
 	xfs_agblock_t			agbno;
 	struct blk_plug			plug;
-	int				blks_per_cluster;
-	int				inodes_per_cluster;
 	int				i;	/* inode chunk index */
 
 	agbno = XFS_AGINO_TO_AGBNO(mp, irec->ir_startino);
-	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
 
 	blk_start_plug(&plug);
 	for (i = 0; i < XFS_INODES_PER_CHUNK;
-	     i += inodes_per_cluster, agbno += blks_per_cluster) {
-		if (xfs_inobt_maskn(i, inodes_per_cluster) & ~irec->ir_free) {
-			xfs_btree_reada_bufs(mp, agno, agbno, blks_per_cluster,
-					     &xfs_inode_buf_ops);
+	     i += mp->m_inodes_per_cluster, agbno += mp->m_blocks_per_cluster) {
+		if (xfs_inobt_maskn(i, mp->m_inodes_per_cluster) &
+				~irec->ir_free) {
+			xfs_btree_reada_bufs(mp, agno, agbno,
+					mp->m_blocks_per_cluster,
+					&xfs_inode_buf_ops);
 		}
 	}
 	blk_finish_plug(&plug);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 1fc9e9042e0e..9fe88d125f0a 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3850,7 +3850,6 @@ xlog_recover_do_icreate_pass2(
 	unsigned int		count;
 	unsigned int		isize;
 	xfs_agblock_t		length;
-	int			blks_per_cluster;
 	int			bb_per_cluster;
 	int			cancel_count;
 	int			nbufs;
@@ -3918,14 +3917,13 @@ xlog_recover_do_icreate_pass2(
 	 * buffers for cancellation so we don't overwrite anything written after
 	 * a cancellation.
 	 */
-	blks_per_cluster = xfs_icluster_size_fsb(mp);
-	bb_per_cluster = XFS_FSB_TO_BB(mp, blks_per_cluster);
-	nbufs = length / blks_per_cluster;
+	bb_per_cluster = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
+	nbufs = length / mp->m_blocks_per_cluster;
 	for (i = 0, cancel_count = 0; i < nbufs; i++) {
 		xfs_daddr_t	daddr;
 
 		daddr = XFS_AGB_TO_DADDR(mp, agno,
-					 agbno + i * blks_per_cluster);
+					 agbno + i * mp->m_blocks_per_cluster);
 		if (xlog_check_buffer_cancelled(log, daddr, bb_per_cluster, 0))
 			cancel_count++;
 	}
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 02d15098dbee..56d374675fd5 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -798,6 +798,8 @@ xfs_mountfs(
 		if (mp->m_sb.sb_inoalignmt >= XFS_B_TO_FSBT(mp, new_size))
 			mp->m_inode_cluster_size = new_size;
 	}
+	mp->m_blocks_per_cluster = xfs_icluster_size_fsb(mp);
+	mp->m_inodes_per_cluster = XFS_FSB_TO_INO(mp, mp->m_blocks_per_cluster);
 
 	/*
 	 * If enabled, sparse inode chunk alignment is expected to match the
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 7964513c3128..58a037bfac22 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -101,6 +101,8 @@ typedef struct xfs_mount {
 	uint8_t			m_agno_log;	/* log #ag's */
 	uint8_t			m_agino_log;	/* #bits for agino in inum */
 	uint			m_inode_cluster_size;/* min inode buf size */
+	unsigned int		m_inodes_per_cluster;
+	unsigned int		m_blocks_per_cluster;
 	uint			m_blockmask;	/* sb_blocksize-1 */
 	uint			m_blockwsize;	/* sb_blocksize in words */
 	uint			m_blockwmask;	/* blockwsize-1 */

Re: [PATCH 4/5] xfs: precalculate inodes and blocks per inode cluster

[Brian Foster]

On Thu, Nov 08, 2018 at 03:20:33PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Store the number of inodes and blocks per inode cluster in the mount
> data so that we don't have to keep recalculating them.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_ialloc.c |   23 ++++++++++-------------
>  fs/xfs/scrub/ialloc.c      |   14 ++++++--------
>  fs/xfs/xfs_inode.c         |   14 +++++---------
>  fs/xfs/xfs_itable.c        |   14 ++++++--------
>  fs/xfs/xfs_log_recover.c   |    8 +++-----
>  fs/xfs/xfs_mount.c         |    2 ++
>  fs/xfs/xfs_mount.h         |    2 ++
>  7 files changed, 34 insertions(+), 43 deletions(-)
> 
> 
...
> diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
> index 426eb1a5503c..332ac7b61789 100644
> --- a/fs/xfs/scrub/ialloc.c
> +++ b/fs/xfs/scrub/ialloc.c
> @@ -193,18 +193,16 @@ xchk_iallocbt_check_freemask(
>  	xfs_agino_t			chunkino;
>  	xfs_agino_t			clusterino;
>  	xfs_agblock_t			agbno;
> -	int				blks_per_cluster;
>  	uint16_t			holemask;
>  	uint16_t			ir_holemask;
>  	int				error = 0;
>  
>  	/* Make sure the freemask matches the inode records. */
> -	blks_per_cluster = xfs_icluster_size_fsb(mp);
> -	nr_inodes = XFS_FSB_TO_INO(mp, blks_per_cluster);
> +	nr_inodes = mp->m_inodes_per_cluster;

Why keep nr_inodes? Doesn't look like it changes anywhere..

>  
>  	for (agino = irec->ir_startino;
>  	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
> -	     agino += blks_per_cluster * mp->m_sb.sb_inopblock) {
> +	     agino += mp->m_blocks_per_cluster * mp->m_sb.sb_inopblock) {

... and this looks like m_inodes_per_cluster.

>  		fsino = XFS_AGINO_TO_INO(mp, bs->cur->bc_private.a.agno, agino);
>  		chunkino = agino - irec->ir_startino;
>  		agbno = XFS_AGINO_TO_AGBNO(mp, agino);
> @@ -225,18 +223,18 @@ xchk_iallocbt_check_freemask(
>  		/* If any part of this is a hole, skip it. */
>  		if (ir_holemask) {
>  			xchk_xref_is_not_owned_by(bs->sc, agbno,
> -					blks_per_cluster,
> +					mp->m_blocks_per_cluster,
>  					&XFS_RMAP_OINFO_INODES);
>  			continue;
>  		}
>  
> -		xchk_xref_is_owned_by(bs->sc, agbno, blks_per_cluster,
> +		xchk_xref_is_owned_by(bs->sc, agbno, mp->m_blocks_per_cluster,
>  				&XFS_RMAP_OINFO_INODES);
>  
>  		/* Grab the inode cluster buffer. */
>  		imap.im_blkno = XFS_AGB_TO_DADDR(mp, bs->cur->bc_private.a.agno,
>  				agbno);
> -		imap.im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
> +		imap.im_len = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
>  		imap.im_boffset = 0;
>  
>  		error = xfs_imap_to_bp(mp, bs->cur->bc_tp, &imap,
> @@ -303,7 +301,7 @@ xchk_iallocbt_rec(
>  	/* Make sure this record is aligned to cluster and inoalignmnt size. */
>  	agbno = XFS_AGINO_TO_AGBNO(mp, irec.ir_startino);
>  	if ((agbno & (xfs_ialloc_cluster_alignment(mp) - 1)) ||
> -	    (agbno & (xfs_icluster_size_fsb(mp) - 1)))
> +	    (agbno & (mp->m_blocks_per_cluster - 1)))
>  		xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
>  
>  	*inode_blocks += XFS_B_TO_FSB(mp,
...
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 18d8d3b812a7..00828b57e1f8 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -167,20 +167,18 @@ xfs_bulkstat_ichunk_ra(
>  {
>  	xfs_agblock_t			agbno;
>  	struct blk_plug			plug;
> -	int				blks_per_cluster;
> -	int				inodes_per_cluster;
>  	int				i;	/* inode chunk index */
>  
>  	agbno = XFS_AGINO_TO_AGBNO(mp, irec->ir_startino);
> -	blks_per_cluster = xfs_icluster_size_fsb(mp);
> -	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
>  
>  	blk_start_plug(&plug);
>  	for (i = 0; i < XFS_INODES_PER_CHUNK;
> -	     i += inodes_per_cluster, agbno += blks_per_cluster) {
> -		if (xfs_inobt_maskn(i, inodes_per_cluster) & ~irec->ir_free) {
> -			xfs_btree_reada_bufs(mp, agno, agbno, blks_per_cluster,
> -					     &xfs_inode_buf_ops);
> +	     i += mp->m_inodes_per_cluster, agbno += mp->m_blocks_per_cluster) {
> +		if (xfs_inobt_maskn(i, mp->m_inodes_per_cluster) &
> +				~irec->ir_free) {

Just another nit, this should probably be aligned like so:

		if (xfs_inobt_maskn(i, mp->m_inodes_per_cluster) &
		    ~irec->ir_free) {
			...
		}

... right?

Brian

> +			xfs_btree_reada_bufs(mp, agno, agbno,
> +					mp->m_blocks_per_cluster,
> +					&xfs_inode_buf_ops);
>  		}
>  	}
>  	blk_finish_plug(&plug);
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 1fc9e9042e0e..9fe88d125f0a 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3850,7 +3850,6 @@ xlog_recover_do_icreate_pass2(
>  	unsigned int		count;
>  	unsigned int		isize;
>  	xfs_agblock_t		length;
> -	int			blks_per_cluster;
>  	int			bb_per_cluster;
>  	int			cancel_count;
>  	int			nbufs;
> @@ -3918,14 +3917,13 @@ xlog_recover_do_icreate_pass2(
>  	 * buffers for cancellation so we don't overwrite anything written after
>  	 * a cancellation.
>  	 */
> -	blks_per_cluster = xfs_icluster_size_fsb(mp);
> -	bb_per_cluster = XFS_FSB_TO_BB(mp, blks_per_cluster);
> -	nbufs = length / blks_per_cluster;
> +	bb_per_cluster = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
> +	nbufs = length / mp->m_blocks_per_cluster;
>  	for (i = 0, cancel_count = 0; i < nbufs; i++) {
>  		xfs_daddr_t	daddr;
>  
>  		daddr = XFS_AGB_TO_DADDR(mp, agno,
> -					 agbno + i * blks_per_cluster);
> +					 agbno + i * mp->m_blocks_per_cluster);
>  		if (xlog_check_buffer_cancelled(log, daddr, bb_per_cluster, 0))
>  			cancel_count++;
>  	}
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 02d15098dbee..56d374675fd5 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -798,6 +798,8 @@ xfs_mountfs(
>  		if (mp->m_sb.sb_inoalignmt >= XFS_B_TO_FSBT(mp, new_size))
>  			mp->m_inode_cluster_size = new_size;
>  	}
> +	mp->m_blocks_per_cluster = xfs_icluster_size_fsb(mp);
> +	mp->m_inodes_per_cluster = XFS_FSB_TO_INO(mp, mp->m_blocks_per_cluster);
>  
>  	/*
>  	 * If enabled, sparse inode chunk alignment is expected to match the
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 7964513c3128..58a037bfac22 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -101,6 +101,8 @@ typedef struct xfs_mount {
>  	uint8_t			m_agno_log;	/* log #ag's */
>  	uint8_t			m_agino_log;	/* #bits for agino in inum */
>  	uint			m_inode_cluster_size;/* min inode buf size */
> +	unsigned int		m_inodes_per_cluster;
> +	unsigned int		m_blocks_per_cluster;
>  	uint			m_blockmask;	/* sb_blocksize-1 */
>  	uint			m_blockwsize;	/* sb_blocksize in words */
>  	uint			m_blockwmask;	/* blockwsize-1 */
> 

Re: [PATCH 4/5] xfs: precalculate inodes and blocks per inode cluster

[Darrick J. Wong]

On Tue, Nov 20, 2018 at 11:23:00AM -0500, Brian Foster wrote:
> On Thu, Nov 08, 2018 at 03:20:33PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Store the number of inodes and blocks per inode cluster in the mount
> > data so that we don't have to keep recalculating them.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_ialloc.c |   23 ++++++++++-------------
> >  fs/xfs/scrub/ialloc.c      |   14 ++++++--------
> >  fs/xfs/xfs_inode.c         |   14 +++++---------
> >  fs/xfs/xfs_itable.c        |   14 ++++++--------
> >  fs/xfs/xfs_log_recover.c   |    8 +++-----
> >  fs/xfs/xfs_mount.c         |    2 ++
> >  fs/xfs/xfs_mount.h         |    2 ++
> >  7 files changed, 34 insertions(+), 43 deletions(-)
> > 
> > 
> ...
> > diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
> > index 426eb1a5503c..332ac7b61789 100644
> > --- a/fs/xfs/scrub/ialloc.c
> > +++ b/fs/xfs/scrub/ialloc.c
> > @@ -193,18 +193,16 @@ xchk_iallocbt_check_freemask(
> >  	xfs_agino_t			chunkino;
> >  	xfs_agino_t			clusterino;
> >  	xfs_agblock_t			agbno;
> > -	int				blks_per_cluster;
> >  	uint16_t			holemask;
> >  	uint16_t			ir_holemask;
> >  	int				error = 0;
> >  
> >  	/* Make sure the freemask matches the inode records. */
> > -	blks_per_cluster = xfs_icluster_size_fsb(mp);
> > -	nr_inodes = XFS_FSB_TO_INO(mp, blks_per_cluster);
> > +	nr_inodes = mp->m_inodes_per_cluster;
> 
> Why keep nr_inodes? Doesn't look like it changes anywhere..

IIRC it changes in the inobt scrubber cleanup series that comes later,
because this code is actually broken -- nr_inodes can't exceed inodes
per cluster or inodes per chunk.

> >  
> >  	for (agino = irec->ir_startino;
> >  	     agino < irec->ir_startino + XFS_INODES_PER_CHUNK;
> > -	     agino += blks_per_cluster * mp->m_sb.sb_inopblock) {
> > +	     agino += mp->m_blocks_per_cluster * mp->m_sb.sb_inopblock) {
> 
> ... and this looks like m_inodes_per_cluster.

Yep.

> >  		fsino = XFS_AGINO_TO_INO(mp, bs->cur->bc_private.a.agno, agino);
> >  		chunkino = agino - irec->ir_startino;
> >  		agbno = XFS_AGINO_TO_AGBNO(mp, agino);
> > @@ -225,18 +223,18 @@ xchk_iallocbt_check_freemask(
> >  		/* If any part of this is a hole, skip it. */
> >  		if (ir_holemask) {
> >  			xchk_xref_is_not_owned_by(bs->sc, agbno,
> > -					blks_per_cluster,
> > +					mp->m_blocks_per_cluster,
> >  					&XFS_RMAP_OINFO_INODES);
> >  			continue;
> >  		}
> >  
> > -		xchk_xref_is_owned_by(bs->sc, agbno, blks_per_cluster,
> > +		xchk_xref_is_owned_by(bs->sc, agbno, mp->m_blocks_per_cluster,
> >  				&XFS_RMAP_OINFO_INODES);
> >  
> >  		/* Grab the inode cluster buffer. */
> >  		imap.im_blkno = XFS_AGB_TO_DADDR(mp, bs->cur->bc_private.a.agno,
> >  				agbno);
> > -		imap.im_len = XFS_FSB_TO_BB(mp, blks_per_cluster);
> > +		imap.im_len = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
> >  		imap.im_boffset = 0;
> >  
> >  		error = xfs_imap_to_bp(mp, bs->cur->bc_tp, &imap,
> > @@ -303,7 +301,7 @@ xchk_iallocbt_rec(
> >  	/* Make sure this record is aligned to cluster and inoalignmnt size. */
> >  	agbno = XFS_AGINO_TO_AGBNO(mp, irec.ir_startino);
> >  	if ((agbno & (xfs_ialloc_cluster_alignment(mp) - 1)) ||
> > -	    (agbno & (xfs_icluster_size_fsb(mp) - 1)))
> > +	    (agbno & (mp->m_blocks_per_cluster - 1)))
> >  		xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
> >  
> >  	*inode_blocks += XFS_B_TO_FSB(mp,
> ...
> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index 18d8d3b812a7..00828b57e1f8 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -167,20 +167,18 @@ xfs_bulkstat_ichunk_ra(
> >  {
> >  	xfs_agblock_t			agbno;
> >  	struct blk_plug			plug;
> > -	int				blks_per_cluster;
> > -	int				inodes_per_cluster;
> >  	int				i;	/* inode chunk index */
> >  
> >  	agbno = XFS_AGINO_TO_AGBNO(mp, irec->ir_startino);
> > -	blks_per_cluster = xfs_icluster_size_fsb(mp);
> > -	inodes_per_cluster = XFS_FSB_TO_INO(mp, blks_per_cluster);
> >  
> >  	blk_start_plug(&plug);
> >  	for (i = 0; i < XFS_INODES_PER_CHUNK;
> > -	     i += inodes_per_cluster, agbno += blks_per_cluster) {
> > -		if (xfs_inobt_maskn(i, inodes_per_cluster) & ~irec->ir_free) {
> > -			xfs_btree_reada_bufs(mp, agno, agbno, blks_per_cluster,
> > -					     &xfs_inode_buf_ops);
> > +	     i += mp->m_inodes_per_cluster, agbno += mp->m_blocks_per_cluster) {
> > +		if (xfs_inobt_maskn(i, mp->m_inodes_per_cluster) &
> > +				~irec->ir_free) {
> 
> Just another nit, this should probably be aligned like so:
> 
> 		if (xfs_inobt_maskn(i, mp->m_inodes_per_cluster) &
> 		    ~irec->ir_free) {
> 			...
> 		}
> 
> ... right?

Yes, fixed.

--D

> Brian
> 
> > +			xfs_btree_reada_bufs(mp, agno, agbno,
> > +					mp->m_blocks_per_cluster,
> > +					&xfs_inode_buf_ops);
> >  		}
> >  	}
> >  	blk_finish_plug(&plug);
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 1fc9e9042e0e..9fe88d125f0a 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -3850,7 +3850,6 @@ xlog_recover_do_icreate_pass2(
> >  	unsigned int		count;
> >  	unsigned int		isize;
> >  	xfs_agblock_t		length;
> > -	int			blks_per_cluster;
> >  	int			bb_per_cluster;
> >  	int			cancel_count;
> >  	int			nbufs;
> > @@ -3918,14 +3917,13 @@ xlog_recover_do_icreate_pass2(
> >  	 * buffers for cancellation so we don't overwrite anything written after
> >  	 * a cancellation.
> >  	 */
> > -	blks_per_cluster = xfs_icluster_size_fsb(mp);
> > -	bb_per_cluster = XFS_FSB_TO_BB(mp, blks_per_cluster);
> > -	nbufs = length / blks_per_cluster;
> > +	bb_per_cluster = XFS_FSB_TO_BB(mp, mp->m_blocks_per_cluster);
> > +	nbufs = length / mp->m_blocks_per_cluster;
> >  	for (i = 0, cancel_count = 0; i < nbufs; i++) {
> >  		xfs_daddr_t	daddr;
> >  
> >  		daddr = XFS_AGB_TO_DADDR(mp, agno,
> > -					 agbno + i * blks_per_cluster);
> > +					 agbno + i * mp->m_blocks_per_cluster);
> >  		if (xlog_check_buffer_cancelled(log, daddr, bb_per_cluster, 0))
> >  			cancel_count++;
> >  	}
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index 02d15098dbee..56d374675fd5 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -798,6 +798,8 @@ xfs_mountfs(
> >  		if (mp->m_sb.sb_inoalignmt >= XFS_B_TO_FSBT(mp, new_size))
> >  			mp->m_inode_cluster_size = new_size;
> >  	}
> > +	mp->m_blocks_per_cluster = xfs_icluster_size_fsb(mp);
> > +	mp->m_inodes_per_cluster = XFS_FSB_TO_INO(mp, mp->m_blocks_per_cluster);
> >  
> >  	/*
> >  	 * If enabled, sparse inode chunk alignment is expected to match the
> > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > index 7964513c3128..58a037bfac22 100644
> > --- a/fs/xfs/xfs_mount.h
> > +++ b/fs/xfs/xfs_mount.h
> > @@ -101,6 +101,8 @@ typedef struct xfs_mount {
> >  	uint8_t			m_agno_log;	/* log #ag's */
> >  	uint8_t			m_agino_log;	/* #bits for agino in inum */
> >  	uint			m_inode_cluster_size;/* min inode buf size */
> > +	unsigned int		m_inodes_per_cluster;
> > +	unsigned int		m_blocks_per_cluster;
> >  	uint			m_blockmask;	/* sb_blocksize-1 */
> >  	uint			m_blockwsize;	/* sb_blocksize in words */
> >  	uint			m_blockwmask;	/* blockwsize-1 */
> > 

[PATCH 5/5] xfs: precalculate cluster alignment in inodes and blocks

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Store the inode cluster alignment information in units of inodes and
blocks in the mount data so that we don't have to keep recalculating
them.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_ialloc.c |    8 ++++----
 fs/xfs/libxfs/xfs_types.c  |    5 ++---
 fs/xfs/scrub/ialloc.c      |    2 +-
 fs/xfs/xfs_mount.c         |    2 ++
 fs/xfs/xfs_mount.h         |    2 ++
 5 files changed, 11 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index dfcd739edcec..e69ce96e83e1 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -690,7 +690,7 @@ xfs_ialloc_ag_alloc(
 		 * but not to use them in the actual exact allocation.
 		 */
 		args.alignment = 1;
-		args.minalignslop = xfs_ialloc_cluster_alignment(args.mp) - 1;
+		args.minalignslop = args.mp->m_cluster_align - 1;
 
 		/* Allow space for the inode btree to split. */
 		args.minleft = args.mp->m_in_maxlevels - 1;
@@ -725,7 +725,7 @@ xfs_ialloc_ag_alloc(
 			args.alignment = args.mp->m_dalign;
 			isaligned = 1;
 		} else
-			args.alignment = xfs_ialloc_cluster_alignment(args.mp);
+			args.alignment = args.mp->m_cluster_align;
 		/*
 		 * Need to figure out where to allocate the inode blocks.
 		 * Ideally they should be spaced out through the a.g.
@@ -754,7 +754,7 @@ xfs_ialloc_ag_alloc(
 		args.type = XFS_ALLOCTYPE_NEAR_BNO;
 		args.agbno = be32_to_cpu(agi->agi_root);
 		args.fsbno = XFS_AGB_TO_FSB(args.mp, agno, args.agbno);
-		args.alignment = xfs_ialloc_cluster_alignment(args.mp);
+		args.alignment = args.mp->m_cluster_align;
 		if ((error = xfs_alloc_vextent(&args)))
 			return error;
 	}
@@ -1017,7 +1017,7 @@ xfs_ialloc_ag_select(
 		 */
 		ineed = mp->m_ialloc_min_blks;
 		if (flags && ineed > 1)
-			ineed += xfs_ialloc_cluster_alignment(mp);
+			ineed += mp->m_cluster_align;
 		longest = pag->pagf_longest;
 		if (!longest)
 			longest = pag->pagf_flcount > 0;
diff --git a/fs/xfs/libxfs/xfs_types.c b/fs/xfs/libxfs/xfs_types.c
index 895c232d63b7..3306fc42cfad 100644
--- a/fs/xfs/libxfs/xfs_types.c
+++ b/fs/xfs/libxfs/xfs_types.c
@@ -87,15 +87,14 @@ xfs_agino_range(
 	 * Calculate the first inode, which will be in the first
 	 * cluster-aligned block after the AGFL.
 	 */
-	bno = round_up(XFS_AGFL_BLOCK(mp) + 1,
-			xfs_ialloc_cluster_alignment(mp));
+	bno = round_up(XFS_AGFL_BLOCK(mp) + 1, mp->m_cluster_align);
 	*first = XFS_AGB_TO_AGINO(mp, bno);
 
 	/*
 	 * Calculate the last inode, which will be at the end of the
 	 * last (aligned) cluster that can be allocated in the AG.
 	 */
-	bno = round_down(eoag, xfs_ialloc_cluster_alignment(mp));
+	bno = round_down(eoag, mp->m_cluster_align);
 	*last = XFS_AGB_TO_AGINO(mp, bno) - 1;
 }
 
diff --git a/fs/xfs/scrub/ialloc.c b/fs/xfs/scrub/ialloc.c
index 332ac7b61789..26b07e0abc0c 100644
--- a/fs/xfs/scrub/ialloc.c
+++ b/fs/xfs/scrub/ialloc.c
@@ -300,7 +300,7 @@ xchk_iallocbt_rec(
 
 	/* Make sure this record is aligned to cluster and inoalignmnt size. */
 	agbno = XFS_AGINO_TO_AGBNO(mp, irec.ir_startino);
-	if ((agbno & (xfs_ialloc_cluster_alignment(mp) - 1)) ||
+	if ((agbno & (mp->m_cluster_align - 1)) ||
 	    (agbno & (mp->m_blocks_per_cluster - 1)))
 		xchk_btree_set_corrupt(bs->sc, bs->cur, 0);
 
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 56d374675fd5..b4d8c318be3c 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -800,6 +800,8 @@ xfs_mountfs(
 	}
 	mp->m_blocks_per_cluster = xfs_icluster_size_fsb(mp);
 	mp->m_inodes_per_cluster = XFS_FSB_TO_INO(mp, mp->m_blocks_per_cluster);
+	mp->m_cluster_align = xfs_ialloc_cluster_alignment(mp);
+	mp->m_cluster_align_inodes = XFS_FSB_TO_INO(mp, mp->m_cluster_align);
 
 	/*
 	 * If enabled, sparse inode chunk alignment is expected to match the
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 58a037bfac22..0ad025e7f3cf 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -103,6 +103,8 @@ typedef struct xfs_mount {
 	uint			m_inode_cluster_size;/* min inode buf size */
 	unsigned int		m_inodes_per_cluster;
 	unsigned int		m_blocks_per_cluster;
+	unsigned int		m_cluster_align;
+	unsigned int		m_cluster_align_inodes;
 	uint			m_blockmask;	/* sb_blocksize-1 */
 	uint			m_blockwsize;	/* sb_blocksize in words */
 	uint			m_blockwmask;	/* blockwsize-1 */

Re: [PATCH 5/5] xfs: precalculate cluster alignment in inodes and blocks

[Brian Foster]

On Thu, Nov 08, 2018 at 03:20:39PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Store the inode cluster alignment information in units of inodes and
> blocks in the mount data so that we don't have to keep recalculating
> them.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_ialloc.c |    8 ++++----
>  fs/xfs/libxfs/xfs_types.c  |    5 ++---
>  fs/xfs/scrub/ialloc.c      |    2 +-
>  fs/xfs/xfs_mount.c         |    2 ++
>  fs/xfs/xfs_mount.h         |    2 ++
>  5 files changed, 11 insertions(+), 8 deletions(-)
> 
> 
...
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 58a037bfac22..0ad025e7f3cf 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -103,6 +103,8 @@ typedef struct xfs_mount {
>  	uint			m_inode_cluster_size;/* min inode buf size */
>  	unsigned int		m_inodes_per_cluster;
>  	unsigned int		m_blocks_per_cluster;
> +	unsigned int		m_cluster_align;
> +	unsigned int		m_cluster_align_inodes;

The m_cluster_align change looks fine:

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

... but no users of m_cluster_align_inodes?

>  	uint			m_blockmask;	/* sb_blocksize-1 */
>  	uint			m_blockwsize;	/* sb_blocksize in words */
>  	uint			m_blockwmask;	/* blockwsize-1 */
> 

Re: [PATCH 5/5] xfs: precalculate cluster alignment in inodes and blocks

[Darrick J. Wong]

On Tue, Nov 20, 2018 at 11:23:30AM -0500, Brian Foster wrote:
> On Thu, Nov 08, 2018 at 03:20:39PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Store the inode cluster alignment information in units of inodes and
> > blocks in the mount data so that we don't have to keep recalculating
> > them.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/xfs/libxfs/xfs_ialloc.c |    8 ++++----
> >  fs/xfs/libxfs/xfs_types.c  |    5 ++---
> >  fs/xfs/scrub/ialloc.c      |    2 +-
> >  fs/xfs/xfs_mount.c         |    2 ++
> >  fs/xfs/xfs_mount.h         |    2 ++
> >  5 files changed, 11 insertions(+), 8 deletions(-)
> > 
> > 
> ...
> > diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> > index 58a037bfac22..0ad025e7f3cf 100644
> > --- a/fs/xfs/xfs_mount.h
> > +++ b/fs/xfs/xfs_mount.h
> > @@ -103,6 +103,8 @@ typedef struct xfs_mount {
> >  	uint			m_inode_cluster_size;/* min inode buf size */
> >  	unsigned int		m_inodes_per_cluster;
> >  	unsigned int		m_blocks_per_cluster;
> > +	unsigned int		m_cluster_align;
> > +	unsigned int		m_cluster_align_inodes;
> 
> The m_cluster_align change looks fine:
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> 
> ... but no users of m_cluster_align_inodes?

We'll get there in the inode scrub fixes patchset.

--D

> >  	uint			m_blockmask;	/* sb_blocksize-1 */
> >  	uint			m_blockwsize;	/* sb_blocksize in words */
> >  	uint			m_blockwmask;	/* blockwsize-1 */
> >