cleanup btree record / key / ptr addressing macros

PATCH In review Christoph Hellwig 2 participants 4 messages Sep 15, 2008 – Sep 17, 2008 XFS filesystem

Share & tools

Christoph Hellwig

author

2008-09-15 00:45 UTC #0 lore raw

PATCH 4/6 cleanup btree record / key / ptr addressing macros

Replace the generic record / key / ptr addressing macros that use cpp token pasting with simpler macros that do the job for just one given btree type. The new macros lose the cur argument and thus can be used outside the core btree code, but also gain an xfs_mount * argument to allow for checking the CRC flag in the near future. Note that many of these macros aren't actually used in the kernel code, but only in userspace (mostly in xfs_repair).

Signed-off-by Christoph Hellwig <[email protected]>

fs/xfs/xfs_alloc_btree.c +1 -1

Index: linux-2.6-xfs/fs/xfs/xfs_alloc_btree.c
===================================================================
@@ -179,7 +179,7 @@ xfs_allocbt_update_lastrec(
 		if (numrecs) {
 			xfs_alloc_rec_t *rrp;
 
-			rrp = XFS_ALLOC_REC_ADDR(block, numrecs, cur);
+			rrp = XFS_ALLOC_REC_ADDR(cur->bc_mp, block, numrecs);
 			len = rrp->ar_blockcount;
 		} else {
 			len = 0;

fs/xfs/xfs_bmap_btree.c +7 -8

Index: linux-2.6-xfs/fs/xfs/xfs_bmap_btree.c
===================================================================
@@ -44,7 +44,6 @@
 #include "xfs_error.h"
 #include "xfs_quota.h"
 
-
 /*
  * Determine the extent state.
  */

@@ -85,9 +84,9 @@ xfs_bmdr_to_bmbt(
 	rblock->bb_leftsib = cpu_to_be64(NULLDFSBNO);
 	rblock->bb_rightsib = cpu_to_be64(NULLDFSBNO);
 	dmxr = xfs_bmdr_maxrecs(mp, dblocklen, 0);
-	fkp = XFS_BTREE_KEY_ADDR(xfs_bmdr, dblock, 1);
-	tkp = XFS_BMAP_BROOT_KEY_ADDR(rblock, 1, rblocklen);
-	fpp = XFS_BTREE_PTR_ADDR(xfs_bmdr, dblock, 1, dmxr);
+	fkp = XFS_BMDR_KEY_ADDR(dblock, 1);
+	tkp = XFS_BMBT_KEY_ADDR(mp, rblock, 1);
+	fpp = XFS_BMDR_PTR_ADDR(dblock, 1, dmxr);
 	tpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);
 	dmxr = be16_to_cpu(dblock->bb_numrecs);
 	memcpy(tkp, fkp, sizeof(*fkp) * dmxr);

@@ -448,10 +447,10 @@ xfs_bmbt_to_bmdr(
 	dblock->bb_level = rblock->bb_level;
 	dblock->bb_numrecs = rblock->bb_numrecs;
 	dmxr = xfs_bmdr_maxrecs(mp, dblocklen, 0);
-	fkp = XFS_BMAP_BROOT_KEY_ADDR(rblock, 1, rblocklen);
-	tkp = XFS_BTREE_KEY_ADDR(xfs_bmdr, dblock, 1);
-	fpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);
-	tpp = XFS_BTREE_PTR_ADDR(xfs_bmdr, dblock, 1, dmxr);
+	fkp = XFS_BMBT_KEY_ADDR(mp, rblock, 1);
+	tkp = XFS_BMDR_KEY_ADDR(dblock, 1);
+  	fpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);
+	tpp = XFS_BMDR_PTR_ADDR(dblock, 1, dmxr);
 	dmxr = be16_to_cpu(dblock->bb_numrecs);
 	memcpy(tkp, fkp, sizeof(*fkp) * dmxr);
 	memcpy(tpp, fpp, sizeof(*fpp) * dmxr);

fs/xfs/xfs_alloc_btree.h +16 -7

Index: linux-2.6-xfs/fs/xfs/xfs_alloc_btree.h
===================================================================
@@ -79,15 +79,24 @@ typedef	struct xfs_btree_sblock xfs_allo
 /*
  * Record, key, and pointer address macros for btree blocks.
  */
-#define	XFS_ALLOC_REC_ADDR(bb,i,cur)	\
-	XFS_BTREE_REC_ADDR(xfs_alloc, bb, i)
+#define XFS_ALLOC_REC_ADDR(mp, block, index) \
+	((xfs_alloc_rec_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 (((index) - 1) * sizeof(xfs_alloc_rec_t))))
 
-#define	XFS_ALLOC_KEY_ADDR(bb,i,cur)	\
-	XFS_BTREE_KEY_ADDR(xfs_alloc, bb, i)
-
-#define	XFS_ALLOC_PTR_ADDR(bb,i,cur)	\
-	XFS_BTREE_PTR_ADDR(xfs_alloc, bb, i, XFS_ALLOC_BLOCK_MAXRECS(1, cur))
+#define XFS_ALLOC_KEY_ADDR(mp, block, index) \
+	((xfs_alloc_key_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 ((index) - 1) * sizeof(xfs_alloc_key_t)))
 
+#define XFS_ALLOC_PTR_ADDR(mp, block, index, maxrecs) \
+	((xfs_alloc_ptr_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 (maxrecs) * sizeof(xfs_alloc_key_t) + \
+		 ((index) - 1) * sizeof(xfs_alloc_ptr_t)))
 
 extern struct xfs_btree_cur *xfs_allocbt_init_cursor(struct xfs_mount *,
 		struct xfs_trans *, struct xfs_buf *,

fs/xfs/xfs_ialloc_btree.h +16 -7

Index: linux-2.6-xfs/fs/xfs/xfs_ialloc_btree.h
===================================================================
@@ -98,15 +98,24 @@ typedef	struct xfs_btree_sblock xfs_inob
 /*
  * Record, key, and pointer address macros for btree blocks.
  */
-#define XFS_INOBT_REC_ADDR(bb,i,cur) \
-	(XFS_BTREE_REC_ADDR(xfs_inobt, bb, i))
+#define XFS_INOBT_REC_ADDR(mp, block, index) \
+	((xfs_inobt_rec_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 (((index) - 1) * sizeof(xfs_inobt_rec_t))))
 
-#define	XFS_INOBT_KEY_ADDR(bb,i,cur) \
-	(XFS_BTREE_KEY_ADDR(xfs_inobt, bb, i))
+#define XFS_INOBT_KEY_ADDR(mp, block, index) \
+	((xfs_inobt_key_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 ((index) - 1) * sizeof(xfs_inobt_key_t)))
 
-#define	XFS_INOBT_PTR_ADDR(bb,i,cur) \
-	(XFS_BTREE_PTR_ADDR(xfs_inobt, bb, \
-				i, XFS_INOBT_BLOCK_MAXRECS(1, cur)))
+#define XFS_INOBT_PTR_ADDR(mp, block, index, maxrecs) \
+	((xfs_inobt_ptr_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 (maxrecs) * sizeof(xfs_inobt_key_t) + \
+		 ((index) - 1) * sizeof(xfs_inobt_ptr_t)))
 
 extern struct xfs_btree_cur *xfs_inobt_init_cursor(struct xfs_mount *,
 		struct xfs_trans *, struct xfs_buf *, xfs_agnumber_t);

fs/xfs/xfs_bmap.c +24 ~~-30~~

143 lines not on this page · raw message

fs/xfs/xfs_bmap_btree.h +40 ~~-22~~

79 lines not on this page · raw message

fs/xfs/xfs_fsops.c +2 -2

Index: linux-2.6-xfs/fs/xfs/xfs_fsops.c
===================================================================
@@ -258,7 +258,7 @@ xfs_growfs_data_private(
 		block->bb_numrecs = cpu_to_be16(1);
 		block->bb_leftsib = cpu_to_be32(NULLAGBLOCK);
 		block->bb_rightsib = cpu_to_be32(NULLAGBLOCK);
-		arec = XFS_BTREE_REC_ADDR(xfs_alloc, block, 1);
+		arec = XFS_ALLOC_REC_ADDR(mp, block, 1);
 		arec->ar_startblock = cpu_to_be32(XFS_PREALLOC_BLOCKS(mp));
 		arec->ar_blockcount = cpu_to_be32(
 			agsize - be32_to_cpu(arec->ar_startblock));

@@ -279,7 +279,7 @@ xfs_growfs_data_private(
 		block->bb_numrecs = cpu_to_be16(1);
 		block->bb_leftsib = cpu_to_be32(NULLAGBLOCK);
 		block->bb_rightsib = cpu_to_be32(NULLAGBLOCK);
-		arec = XFS_BTREE_REC_ADDR(xfs_alloc, block, 1);
+		arec = XFS_ALLOC_REC_ADDR(mp, block, 1);
 		arec->ar_startblock = cpu_to_be32(XFS_PREALLOC_BLOCKS(mp));
 		arec->ar_blockcount = cpu_to_be32(
 			agsize - be32_to_cpu(arec->ar_startblock));

fs/xfs/xfs_inode.c +2 -4

Index: linux-2.6-xfs/fs/xfs/xfs_inode.c
===================================================================
@@ -2435,10 +2435,8 @@ xfs_iroot_realloc(
 		/*
 		 * First copy the records.
 		 */
-		op = (char *)XFS_BMAP_BROOT_REC_ADDR(ifp->if_broot, 1,
-						     ifp->if_broot_bytes);
-		np = (char *)XFS_BMAP_BROOT_REC_ADDR(new_broot, 1,
-						     (int)new_size);
+		op = (char *)XFS_BMBT_REC_ADDR(mp, ifp->if_broot, 1);
+		np = (char *)XFS_BMBT_REC_ADDR(mp, new_broot, 1);
 		memcpy(np, op, new_max * (uint)sizeof(xfs_bmbt_rec_t));
 
 		/*

fs/xfs/xfs_btree.h +0 ~~-15~~

Index: linux-2.6-xfs/fs/xfs/xfs_btree.h
===================================================================
@@ -149,21 +149,6 @@ do {    \
 	}       \
 } while (0)
 
-/*
- * Record, key, and pointer address calculation macros.
- * Given block size, type prefix, block pointer, and index of requested entry
- * (first entry numbered 1).
- */
-#define	XFS_BTREE_REC_ADDR(t,bb,i)	\
-	((t ## _rec_t *)((char *)(bb) + sizeof(t ## _block_t) + \
-	 ((i) - 1) * sizeof(t ## _rec_t)))
-#define	XFS_BTREE_KEY_ADDR(t,bb,i)	\
-	((t ## _key_t *)((char *)(bb) + sizeof(t ## _block_t) + \
-	 ((i) - 1) * sizeof(t ## _key_t)))
-#define	XFS_BTREE_PTR_ADDR(t,bb,i,mxr)	\
-	((t ## _ptr_t *)((char *)(bb) + sizeof(t ## _block_t) + \
-	 (mxr) * sizeof(t ## _key_t) + ((i) - 1) * sizeof(t ## _ptr_t)))
-
 #define	XFS_BTREE_MAXLEVELS	8	/* max of all btrees */
 
 struct xfs_btree_ops {

Dave Chinner

2008-09-16 05:52 UTC #1 lore raw

Quoted 6 lines · Christoph Hellwig

Replace the generic record / key / ptr addressing macros that use cpp token pasting with simpler macros that do the job for just one given btree type. The new macros lose the cur argument and thus can be used outside the core btree code, but also gain an xfs_mount * argument to allow for checking the CRC flag in the near future. Note that many of these macros aren't actually used in the kernel code, but only in userspace (mostly in xfs_repair).

......

fs/xfs/xfs_bmap_btree.c xfs_bmdr_to_bmbt( · line 90

Quoted 11 lines

fs/xfs/xfs_bmap_btree.c +3 -3

@@ -85,9 +84,9 @@ xfs_bmdr_to_bmbt(
 	rblock->bb_leftsib = cpu_to_be64(NULLDFSBNO);
 	rblock->bb_rightsib = cpu_to_be64(NULLDFSBNO);
 	dmxr = xfs_bmdr_maxrecs(mp, dblocklen, 0);
-	fkp = XFS_BTREE_KEY_ADDR(xfs_bmdr, dblock, 1);
-	tkp = XFS_BMAP_BROOT_KEY_ADDR(rblock, 1, rblocklen);
-	fpp = XFS_BTREE_PTR_ADDR(xfs_bmdr, dblock, 1, dmxr);
+	fkp = XFS_BMDR_KEY_ADDR(dblock, 1);
+	tkp = XFS_BMBT_KEY_ADDR(mp, rblock, 1);
+	fpp = XFS_BMDR_PTR_ADDR(dblock, 1, dmxr);
 	tpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);

Why did you change XFS_BMAP_BROOT_KEY_ADDR and not XFS_BMAP_BROOT_PTR_ADDR?

fs/xfs/xfs_bmap_btree.c xfs_bmbt_to_bmdr( · line 452

Quoted 13 lines

fs/xfs/xfs_bmap_btree.c +3 -4

 	dmxr = be16_to_cpu(dblock->bb_numrecs);
 	memcpy(tkp, fkp, sizeof(*fkp) * dmxr);

@@ -448,10 +447,10 @@ xfs_bmbt_to_bmdr(
 	dblock->bb_level = rblock->bb_level;
 	dblock->bb_numrecs = rblock->bb_numrecs;
 	dmxr = xfs_bmdr_maxrecs(mp, dblocklen, 0);
-	fkp = XFS_BMAP_BROOT_KEY_ADDR(rblock, 1, rblocklen);
-	tkp = XFS_BTREE_KEY_ADDR(xfs_bmdr, dblock, 1);
-	fpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);
-	tpp = XFS_BTREE_PTR_ADDR(xfs_bmdr, dblock, 1, dmxr);
+	fkp = XFS_BMBT_KEY_ADDR(mp, rblock, 1);
+	tkp = XFS_BMDR_KEY_ADDR(dblock, 1);
+  	fpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);

Stray whitespace.

fs/xfs/xfs_alloc_btree.h typedef struct xfs_btree_sblock xfs_allo · line 86

Quoted 11 lines

fs/xfs/xfs_alloc_btree.h +5 -2

@@ -79,15 +79,24 @@ typedef	struct xfs_btree_sblock xfs_allo
 /*
  * Record, key, and pointer address macros for btree blocks.
  */
-#define	XFS_ALLOC_REC_ADDR(bb,i,cur)	\
-	XFS_BTREE_REC_ADDR(xfs_alloc, bb, i)
+#define XFS_ALLOC_REC_ADDR(mp, block, index) \
+	((xfs_alloc_rec_t *) \
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 (((index) - 1) * sizeof(xfs_alloc_rec_t))))

Shouldn't these become inline functions rather than macros now that the token substitution is gone? Same for rest as well? That would remove a bunch of shouting that you're changing anyway....

Quoted 13 lines

fs/xfs/xfs_bmap.c +1 -6

-
-		if (root) {
-			keyp = XFS_BMAP_BROOT_KEY_ADDR(block, i, sz);
-		} else {
-			keyp = XFS_BTREE_KEY_ADDR(xfs_bmbt, block, i);
-		}
+		keyp = XFS_BMBT_KEY_ADDR(mp, block, i);
 
 		if (prevp) {
 			ASSERT(be64_to_cpu(prevp->br_startoff) <

@@ -6203,19 +6200,16 @@ xfs_check_block(
 		/*
 		 * Compare the block numbers to see if there are dups.

fs/xfs/xfs_bmap.c xfs_check_block( · line 6206

Quoted 10 lines

fs/xfs/xfs_bmap.c +3 -5

 		 */
-
-		if (root) {
+		if (root)
 			pp = XFS_BMAP_BROOT_PTR_ADDR(mp, block, i, sz);
-		} else {
-			pp = XFS_BTREE_PTR_ADDR(xfs_bmbt, block, i, dmxr);
-		}
+		else
+			pp = XFS_BMBT_PTR_ADDR(mp, block, i, dmxr);

Why the assymetry in the interfaces for key and ptr?

fs/xfs/xfs_bmap_btree.h typedef struct xfs_btree_lblock xfs_bmbt · line 198

Quoted 8 lines

fs/xfs/xfs_bmap_btree.h +2 -6

-#define XFS_BMAP_BROOT_REC_ADDR(bb,i,sz) \
-	(XFS_BTREE_REC_ADDR(xfs_bmbt,bb,i))
-#define XFS_BMAP_BROOT_KEY_ADDR(bb,i,sz) \
-	(XFS_BTREE_KEY_ADDR(xfs_bmbt,bb,i))
-#define XFS_BMAP_BROOT_PTR_ADDR(mp, bb,i,sz) \
-	(XFS_BTREE_PTR_ADDR(xfs_bmbt,bb,i,xfs_bmbt_maxrecs(mp, sz, 0)))
+#define XFS_BMAP_BROOT_PTR_ADDR(mp, bb, i, sz) \
+	XFS_BMBT_PTR_ADDR(mp, bb, i, xfs_bmbt_maxrecs(mp, sz, 0))

Ah, that explains why that macro didn't change. Why keep just this one?

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Christoph Hellwig

author

2008-09-16 17:40 UTC #2 lore raw

Quoted 8 lines · Dave Chinner

Quoted 6 lines · Christoph Hellwig
Replace the generic record / key / ptr addressing macros that use cpp token pasting with simpler macros that do the job for just one given btree type. The new macros lose the cur argument and thus can be used outside the core btree code, but also gain an xfs_mount * argument to allow for checking the CRC flag in the near future. Note that many of these macros aren't actually used in the kernel code, but only in userspace (mostly in xfs_repair).
......

Quoted 14 lines

fs/xfs/xfs_bmap_btree.c xfs_bmdr_to_bmbt( · line 90

Quoted 11 lines

fs/xfs/xfs_bmap_btree.c +3 -3

@@ -85,9 +84,9 @@ xfs_bmdr_to_bmbt(
 	rblock->bb_leftsib = cpu_to_be64(NULLDFSBNO);
 	rblock->bb_rightsib = cpu_to_be64(NULLDFSBNO);
 	dmxr = xfs_bmdr_maxrecs(mp, dblocklen, 0);
-	fkp = XFS_BTREE_KEY_ADDR(xfs_bmdr, dblock, 1);
-	tkp = XFS_BMAP_BROOT_KEY_ADDR(rblock, 1, rblocklen);
-	fpp = XFS_BTREE_PTR_ADDR(xfs_bmdr, dblock, 1, dmxr);
+	fkp = XFS_BMDR_KEY_ADDR(dblock, 1);
+	tkp = XFS_BMBT_KEY_ADDR(mp, rblock, 1);
+	fpp = XFS_BMDR_PTR_ADDR(dblock, 1, dmxr);
 	tpp = XFS_BMAP_BROOT_PTR_ADDR(mp, rblock, 1, rblocklen);

Why did you change XFS_BMAP_BROOT_KEY_ADDR and not XFS_BMAP_BROOT_PTR_ADDR?

Because XFS_BMAP_BROOT_KEY_ADDR was just another name for XFS_BMBT_KEY_ADDR, while XFS_BMAP_BROOT_PTR_ADDR actually adds some value (a call to xfs_bmbt_maxrecs).

Quoted 4 lines

fs/xfs/xfs_alloc_btree.h +2 -2

-#define	XFS_ALLOC_REC_ADDR(bb,i,cur)	\
-	XFS_BTREE_REC_ADDR(xfs_alloc, bb, i)
+#define XFS_ALLOC_REC_ADDR(mp, block, index) \
+	((xfs_alloc_rec_t *) \

Quoted 8 lines

fs/xfs/xfs_alloc_btree.h typedef struct xfs_btree_sblock xfs_allo · line 86
Quoted 3 lines
fs/xfs/xfs_alloc_btree.h +3 -0
+		((char *)(block) + \
+		 sizeof(struct xfs_btree_sblock) + \
+		 (((index) - 1) * sizeof(xfs_alloc_rec_t))))
Shouldn't these become inline functions rather than macros now that the token substitution is gone? Same for rest as well? That would remove a bunch of shouting that you're changing anyway....

Inlines don't work due to header ordering problems. I tried to move them out of line, but it actually increased the code size.

Quoted 6 lines

fs/xfs/xfs_bmap.c xfs_check_block( · line 6206
Quoted 4 lines
fs/xfs/xfs_bmap.c +3 -0
+		if (root)
 			pp = XFS_BMAP_BROOT_PTR_ADDR(mp, block, i, sz);
+		else
+			pp = XFS_BMBT_PTR_ADDR(mp, block, i, dmxr);
Why the assymetry in the interfaces for key and ptr?

The only real difference is that the ptr needs the maxrecs value passed because it's used in the calculation, while the key address calculation doesn't need it and thus doesn't get it passed.

Quoted 5 lines

fs/xfs/xfs_bmap_btree.h typedef struct xfs_btree_lblock xfs_bmbt · line 198
Quoted 2 lines
fs/xfs/xfs_bmap_btree.h +2 -0
+#define XFS_BMAP_BROOT_PTR_ADDR(mp, bb, i, sz) \
+	XFS_BMBT_PTR_ADDR(mp, bb, i, xfs_bmbt_maxrecs(mp, sz, 0))
Ah, that explains why that macro didn't change. Why keep just this one?

It seems borderline useful, but if you care strongly I can kill it.

Dave Chinner

2008-09-17 00:56 UTC #3 lore raw

Quoted 9 lines · Christoph Hellwig

Quoted 6 lines · Dave Chinner
fs/xfs/xfs_bmap_btree.h typedef struct xfs_btree_lblock xfs_bmbt · line 198
Quoted 2 lines · Christoph Hellwig
fs/xfs/xfs_bmap_btree.h +2 -0
+#define XFS_BMAP_BROOT_PTR_ADDR(mp, bb, i, sz) \
+	XFS_BMBT_PTR_ADDR(mp, bb, i, xfs_bmbt_maxrecs(mp, sz, 0))
Ah, that explains why that macro didn't change. Why keep just this one?
It seems borderline useful, but if you care strongly I can kill it.

I don't really mind - i was just curious as to why that particular macro didn't get changed like all the others. Seems reasonable to leave it there....

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]