[PATCH 0/8] libxfs: cleanup & leak detection

[PATCH 0/8] libxfs: cleanup & leak detection

[Eric Sandeen]

This series is motivated by 2 things; moving libxfs transaction
code slightly closer to how it looks in the kernel, and also
adding (or fixing) infrastructure to better detect leaks in
libxfs.

The first 3 patches just tidy up the use of some
of the private buffer fields in xfs_buf, to match the kernel.

The next 5 revolve around using pseudo memory zones in libxfs
and detecting whether all allocated items have been freed when
libxfs is torn down.  By default there should be no functional
change here, but if the LIBXFS_LEAK_CHECK env var is set, it
will squawk about leaks and exit with exit(1):

[sandeen@sandeen xfsprogs]$ mkfs/mkfs.xfs -dfile,name=fsfile2,size=1g
meta-data=fsfile2                isize=512    agcount=4, agsize=65536 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=0, rmapbt=0, reflink=0
data     =                       bsize=4096   blocks=262144, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal log           bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
zone xfs_buf_log_item freed with 3 items allocated
[sandeen@sandeen xfsprogs]$ echo $?
1

I've got more going on to fix up these leaks, but this lays the groundwork
in a hopefully easy series to review.

Thanks,
-Eric

[PATCH 1/8] xfs: remove wrappers around b_fspriv

[Eric Sandeen]

From: Christoph Hellwig <hch@lst.de>

Source kernel commit adadbeefb34f755a3477da51035eeeec2c1fde38

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alex Elder <aelder@sgi.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/libxfs_io.h |  2 --
 libxfs/logitem.c   |  6 +++---
 libxfs/trans.c     | 48 +++++++++++++++++++++---------------------------
 libxfs/util.c      |  2 +-
 4 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 1209e52..29fb1c7 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -111,8 +111,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
 #define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
 
-#define XFS_BUF_FSPRIVATE(bp,type)	((type)(bp)->b_fspriv)
-#define XFS_BUF_SET_FSPRIVATE(bp,val)	(bp)->b_fspriv = (void *)(val)
 #define XFS_BUF_FSPRIVATE2(bp,type)	((type)(bp)->b_fsprivate2)
 #define XFS_BUF_SET_FSPRIVATE2(bp,val)	(bp)->b_fsprivate2 = (void *)(val)
 #define XFS_BUF_FSPRIVATE3(bp,type)	((type)(bp)->b_fsprivate3)
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 4dcc506..0c183b5 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -99,8 +99,8 @@ xfs_buf_item_init(
 	if (XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *) != mp)
 		XFS_BUF_SET_FSPRIVATE3(bp, mp);
 	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
-	if (XFS_BUF_FSPRIVATE(bp, void *) != NULL) {
-		lip = XFS_BUF_FSPRIVATE(bp, xfs_log_item_t *);
+	if (bp->b_fspriv != NULL) {
+		lip = bp->b_fspriv;
 		if (lip->li_type == XFS_LI_BUF) {
 #ifdef LI_DEBUG
 			fprintf(stderr,
@@ -123,7 +123,7 @@ xfs_buf_item_init(
 	bip->bli_format.blf_type = XFS_LI_BUF;
 	bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
 	bip->bli_format.blf_len = (unsigned short)BTOBB(XFS_BUF_COUNT(bp));
-	XFS_BUF_SET_FSPRIVATE(bp, bip);
+	bp->b_fspriv = bip;
 }
 
 
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 3d64f26..6a1901b 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -297,11 +297,10 @@ libxfs_trans_inode_alloc_buf(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
-	xfs_buf_log_item_t	*bip;
+	xfs_buf_log_item_t	*bip = bp->b_fspriv;;
 
 	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
+	ASSERT(bip != NULL);
 	bip->bli_flags |= XFS_BLI_INODE_ALLOC_BUF;
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
 }
@@ -364,12 +363,11 @@ libxfs_trans_dirty_buf(
 	struct xfs_trans	*tp,
 	struct xfs_buf		*bp)
 {
-	struct xfs_buf_log_item	*bip;
+	struct xfs_buf_log_item	*bip = bp->b_fspriv;
 
 	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
+	ASSERT(bip != NULL);
 
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
 #endif
@@ -393,12 +391,10 @@ libxfs_trans_log_buf(
 	uint			first,
 	uint			last)
 {
-	struct xfs_buf_log_item	*bip;
+	struct xfs_buf_log_item	*bip = bp->b_fspriv;
 
 	ASSERT((first <= last) && (last < XFS_BUF_COUNT(bp)));
 
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
-
 	xfs_trans_dirty_buf(tp, bp);
 	xfs_buf_item_log(bip, first, last);
 }
@@ -439,7 +435,7 @@ libxfs_trans_brelse(
 		return;
 	}
 	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
+	bip = bp->b_fspriv;
 	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
 	if (bip->bli_recur > 0) {
 		bip->bli_recur--;
@@ -462,15 +458,14 @@ libxfs_trans_binval(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
-	xfs_buf_log_item_t	*bip;
+	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 #ifdef XACT_DEBUG
 	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
 #endif
 
 	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
+	ASSERT(bip != NULL);
 
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
 	if (bip->bli_flags & XFS_BLI_STALE)
 		return;
 	XFS_BUF_UNDELAYWRITE(bp);
@@ -496,7 +491,7 @@ libxfs_trans_bjoin(
 #endif
 
 	xfs_buf_item_init(bp, tp->t_mountp);
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
+	bip = bp->b_fspriv;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 	XFS_BUF_SET_FSPRIVATE2(bp, tp);
 }
@@ -506,15 +501,14 @@ libxfs_trans_bhold(
 	xfs_trans_t		*tp,
 	xfs_buf_t		*bp)
 {
-	xfs_buf_log_item_t	*bip;
+	xfs_buf_log_item_t	*bip =bp->b_fspriv;
 
 	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
+	ASSERT(bp->b_fspriv != NULL);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
 #endif
 
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
 	bip->bli_flags |= XFS_BLI_HOLD;
 }
 
@@ -535,7 +529,7 @@ libxfs_trans_get_buf_map(
 	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
 	if (bp != NULL) {
 		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-		bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
+		bip = bp->b_fspriv;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
 		return bp;
@@ -549,7 +543,7 @@ libxfs_trans_get_buf_map(
 #endif
 
 	xfs_buf_item_init(bp, tp->t_mountp);
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t*);
+	bip = bp->b_fspriv;
 	bip->bli_recur = 0;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 
@@ -575,7 +569,7 @@ libxfs_trans_getsb(
 	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
 	if (bp != NULL) {
 		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-		bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
+		bip = bp->b_fspriv;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
 		return bp;
@@ -587,7 +581,7 @@ libxfs_trans_getsb(
 #endif
 
 	xfs_buf_item_init(bp, mp);
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t*);
+	bip = bp->b_fspriv;
 	bip->bli_recur = 0;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 
@@ -626,8 +620,8 @@ libxfs_trans_read_buf_map(
 	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
 	if (bp != NULL) {
 		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
-		ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
-		bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t*);
+		ASSERT(bp->b_fspriv != NULL);
+		bip = bp->b_fspriv;
 		bip->bli_recur++;
 		goto done;
 	}
@@ -644,7 +638,7 @@ libxfs_trans_read_buf_map(
 #endif
 
 	xfs_buf_item_init(bp, tp->t_mountp);
-	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
+	bip = bp->b_fspriv;
 	bip->bli_recur = 0;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 
@@ -732,7 +726,7 @@ inode_item_done(
 		return;
 	}
 
-	XFS_BUF_SET_FSPRIVATE(bp, iip);
+	bp->b_fspriv = iip;
 	error = libxfs_iflush_int(ip, bp);
 	if (error) {
 		fprintf(stderr, _("%s: warning - iflush_int failed (%d)\n"),
@@ -741,7 +735,7 @@ inode_item_done(
 	}
 
 	ip->i_transp = NULL;	/* disassociate from transaction */
-	XFS_BUF_SET_FSPRIVATE(bp, NULL);	/* remove log item */
+	bp->b_fspriv = NULL;			/* remove log item */
 	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
 	libxfs_writebuf(bp, 0);
 #ifdef XACT_DEBUG
@@ -760,7 +754,7 @@ buf_item_done(
 
 	bp = bip->bli_buf;
 	ASSERT(bp != NULL);
-	XFS_BUF_SET_FSPRIVATE(bp, NULL);	/* remove log item */
+	bp->b_fspriv = NULL;			/* remove log item */
 	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
 
 	hold = (bip->bli_flags & XFS_BLI_HOLD);
diff --git a/libxfs/util.c b/libxfs/util.c
index 8003b26..5f49b82 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -466,7 +466,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
 	xfs_dinode_t		*dip;
 	xfs_mount_t		*mp;
 
-	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
+	ASSERT(bp-b_fspriv != NULL);
 	ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE ||
 		ip->i_d.di_nextents > ip->i_df.if_ext_max);
 	ASSERT(ip->i_d.di_version > 1);
-- 
1.8.3.1

[PATCH 2/8] xfs: add a proper transaction pointer to struct xfs_buf

[Eric Sandeen]

From: Christoph Hellwig <hch@lst.de>

Source kernel commit bf9d9013a2a559858efb590bf922377be9d6d969

Replace the typeless b_fspriv2 and the ugly macros around it with a properly
typed transaction pointer.  As a fallout the log buffer state debug checks
are also removed.  We could have kept them using casts, but as they do
not have a real purpose we can as well just remove them.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alex Elder <aelder@sgi.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/libxfs_io.h |  4 +---
 libxfs/trans.c     | 44 ++++++++++++++++++++++----------------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 29fb1c7..5acd3df 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -69,7 +69,7 @@ typedef struct xfs_buf {
 	pthread_t		b_holder;
 	unsigned int		b_recur;
 	void			*b_fspriv;
-	void			*b_fsprivate2;
+	void			*b_transp;
 	void			*b_fsprivate3;
 	void			*b_addr;
 	int			b_error;
@@ -111,8 +111,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
 #define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
 
-#define XFS_BUF_FSPRIVATE2(bp,type)	((type)(bp)->b_fsprivate2)
-#define XFS_BUF_SET_FSPRIVATE2(bp,val)	(bp)->b_fsprivate2 = (void *)(val)
 #define XFS_BUF_FSPRIVATE3(bp,type)	((type)(bp)->b_fsprivate3)
 #define XFS_BUF_SET_FSPRIVATE3(bp,val)	(bp)->b_fsprivate3 = (void *)(val)
 
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 6a1901b..57ff3ea 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -299,7 +299,7 @@ libxfs_trans_inode_alloc_buf(
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;;
 
-	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+	ASSERT(bp->bp_transp == tp);
 	ASSERT(bip != NULL);
 	bip->bli_flags |= XFS_BLI_INODE_ALLOC_BUF;
 	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
@@ -365,7 +365,7 @@ libxfs_trans_dirty_buf(
 {
 	struct xfs_buf_log_item	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+	ASSERT(bp->bp_transp == tp);
 	ASSERT(bip != NULL);
 
 #ifdef XACT_DEBUG
@@ -430,11 +430,11 @@ libxfs_trans_brelse(
 #endif
 
 	if (tp == NULL) {
-		ASSERT(XFS_BUF_FSPRIVATE2(bp, void *) == NULL);
+		ASSERT(bp->bp_transp == NULL);
 		libxfs_putbuf(bp);
 		return;
 	}
-	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+	ASSERT(bp->bp_transp == tp);
 	bip = bp->b_fspriv;
 	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
 	if (bip->bli_recur > 0) {
@@ -449,7 +449,7 @@ libxfs_trans_brelse(
 	xfs_trans_del_item(&bip->bli_item);
 	if (bip->bli_flags & XFS_BLI_HOLD)
 		bip->bli_flags &= ~XFS_BLI_HOLD;
-	XFS_BUF_SET_FSPRIVATE2(bp, NULL);
+	bp->b_transp = NULL;
 	libxfs_putbuf(bp);
 }
 
@@ -463,7 +463,7 @@ libxfs_trans_binval(
 	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
 #endif
 
-	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+	ASSERT(bp->bp_transp == tp);
 	ASSERT(bip != NULL);
 
 	if (bip->bli_flags & XFS_BLI_STALE)
@@ -485,7 +485,7 @@ libxfs_trans_bjoin(
 {
 	xfs_buf_log_item_t	*bip;
 
-	ASSERT(XFS_BUF_FSPRIVATE2(bp, void *) == NULL);
+	ASSERT(bp->bp_transp == NULL);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
 #endif
@@ -493,7 +493,7 @@ libxfs_trans_bjoin(
 	xfs_buf_item_init(bp, tp->t_mountp);
 	bip = bp->b_fspriv;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
-	XFS_BUF_SET_FSPRIVATE2(bp, tp);
+	bp->b_transp = tp;
 }
 
 void
@@ -503,7 +503,7 @@ libxfs_trans_bhold(
 {
 	xfs_buf_log_item_t	*bip =bp->b_fspriv;
 
-	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+	ASSERT(bp->bp_transp == tp);
 	ASSERT(bp->b_fspriv != NULL);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
@@ -528,7 +528,7 @@ libxfs_trans_get_buf_map(
 
 	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
 	if (bp != NULL) {
-		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+		ASSERT(bp->bp_transp == tp);
 		bip = bp->b_fspriv;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
@@ -547,8 +547,8 @@ libxfs_trans_get_buf_map(
 	bip->bli_recur = 0;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 
-	/* initialize b_fsprivate2 so we can find it incore */
-	XFS_BUF_SET_FSPRIVATE2(bp, tp);
+	/* initialize b_transp so we can find it incore */
+	bp->b_transp = tp;
 	return bp;
 }
 
@@ -568,7 +568,7 @@ libxfs_trans_getsb(
 
 	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
 	if (bp != NULL) {
-		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+		ASSERT(bp->bp_transp == tp);
 		bip = bp->b_fspriv;
 		ASSERT(bip != NULL);
 		bip->bli_recur++;
@@ -585,8 +585,8 @@ libxfs_trans_getsb(
 	bip->bli_recur = 0;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 
-	/* initialize b_fsprivate2 so we can find it incore */
-	XFS_BUF_SET_FSPRIVATE2(bp, tp);
+	/* initialize b_transp so we can find it incore */
+	bp->b_transp = tp;
 	return bp;
 }
 
@@ -619,7 +619,7 @@ libxfs_trans_read_buf_map(
 
 	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
 	if (bp != NULL) {
-		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
+		ASSERT(bp->bp_transp == tp);
 		ASSERT(bp->b_fspriv != NULL);
 		bip = bp->b_fspriv;
 		bip->bli_recur++;
@@ -642,8 +642,8 @@ libxfs_trans_read_buf_map(
 	bip->bli_recur = 0;
 	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
 
-	/* initialise b_fsprivate2 so we can find it incore */
-	XFS_BUF_SET_FSPRIVATE2(bp, tp);
+	/* initialise b_transp so we can find it incore */
+	bp->b_transp = tp;
 done:
 	*bpp = bp;
 	return 0;
@@ -735,8 +735,8 @@ inode_item_done(
 	}
 
 	ip->i_transp = NULL;	/* disassociate from transaction */
-	bp->b_fspriv = NULL;			/* remove log item */
-	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
+	bp->b_fspriv = NULL;	/* remove log item */
+	bp->b_transp = NULL;	/* remove xact ptr */
 	libxfs_writebuf(bp, 0);
 #ifdef XACT_DEBUG
 	fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
@@ -755,7 +755,7 @@ buf_item_done(
 	bp = bip->bli_buf;
 	ASSERT(bp != NULL);
 	bp->b_fspriv = NULL;			/* remove log item */
-	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
+	bp->b_transp = NULL;			/* remove xact ptr */
 
 	hold = (bip->bli_flags & XFS_BLI_HOLD);
 	if (bip->bli_flags & XFS_BLI_DIRTY) {
@@ -804,7 +804,7 @@ buf_item_unlock(
 	uint			hold;
 
 	/* Clear the buffer's association with this transaction. */
-	XFS_BUF_SET_FSPRIVATE2(bip->bli_buf, NULL);
+	bip->bli_buf->b_transp = NULL;
 
 	hold = bip->bli_flags & XFS_BLI_HOLD;
 	bip->bli_flags &= ~XFS_BLI_HOLD;
-- 
1.8.3.1

[PATCH 3/8] xfs: remove unused buf_fsprivate3

[Eric Sandeen]

The buf_fsprivate3 field has no actual use, other than a pointless
"if it's not set, set it" in xfs_buf_item_init; nobody cares after
that.  Remove it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/libxfs_io.h | 4 ----
 libxfs/logitem.c   | 2 --
 2 files changed, 6 deletions(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 5acd3df..2fce04d 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -70,7 +70,6 @@ typedef struct xfs_buf {
 	unsigned int		b_recur;
 	void			*b_fspriv;
 	void			*b_transp;
-	void			*b_fsprivate3;
 	void			*b_addr;
 	int			b_error;
 	const struct xfs_buf_ops *b_ops;
@@ -111,9 +110,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
 #define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
 
-#define XFS_BUF_FSPRIVATE3(bp,type)	((type)(bp)->b_fsprivate3)
-#define XFS_BUF_SET_FSPRIVATE3(bp,val)	(bp)->b_fsprivate3 = (void *)(val)
-
 #define XFS_BUF_SET_PRIORITY(bp,pri)	cache_node_set_priority( \
 						libxfs_bcache, \
 						(struct cache_node *)(bp), \
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 0c183b5..0c50fcf 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -96,8 +96,6 @@ xfs_buf_item_init(
 	 * the first.  If we do already have one, there is
 	 * nothing to do here so return.
 	 */
-	if (XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *) != mp)
-		XFS_BUF_SET_FSPRIVATE3(bp, mp);
 	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
 	if (bp->b_fspriv != NULL) {
 		lip = bp->b_fspriv;
-- 
1.8.3.1

[PATCH 4/8] libxfs: use a memory zone for transactions

[Eric Sandeen]

In addition to more closely matching the kernel, this will
help us catch any leaks from these allocations.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/init.c  |  4 ++++
 libxfs/trans.c | 29 ++++++++++++++++++-----------
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/libxfs/init.c b/libxfs/init.c
index 302f088..7bde8b7 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -384,6 +384,7 @@ manage_zones(int release)
 	extern kmem_zone_t	*xfs_da_state_zone;
 	extern kmem_zone_t	*xfs_btree_cur_zone;
 	extern kmem_zone_t	*xfs_bmap_free_item_zone;
+	extern kmem_zone_t	*xfs_trans_zone;
 	extern kmem_zone_t	*xfs_log_item_desc_zone;
 	extern void		xfs_dir_startup();
 
@@ -395,6 +396,7 @@ manage_zones(int release)
 		kmem_free(xfs_da_state_zone);
 		kmem_free(xfs_btree_cur_zone);
 		kmem_free(xfs_bmap_free_item_zone);
+		kmem_free(xfs_trans_zone);
 		kmem_free(xfs_log_item_desc_zone);
 		return;
 	}
@@ -413,6 +415,8 @@ manage_zones(int release)
 	xfs_bmap_free_item_zone = kmem_zone_init(
 			sizeof(struct xfs_extent_free_item),
 			"xfs_bmap_free_item");
+	xfs_trans_zone = kmem_zone_init(
+			sizeof(struct xfs_trans), "xfs_trans");
 	xfs_log_item_desc_zone = kmem_zone_init(
 			sizeof(struct xfs_log_item_desc), "xfs_log_item_desc");
 	xfs_dir_startup();
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 57ff3ea..035cc22 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -36,6 +36,7 @@ static void xfs_trans_free_items(struct xfs_trans *tp);
  * Simple transaction interface
  */
 
+kmem_zone_t	*xfs_trans_zone;
 kmem_zone_t	*xfs_log_item_desc_zone;
 
 /*
@@ -143,6 +144,18 @@ libxfs_trans_roll(
 	return 0;
 }
 
+/*
+ * Free the transaction structure.  If there is more clean up
+ * to do when the structure is freed, add it here.
+ */
+static void
+xfs_trans_free(
+	struct xfs_trans	*tp)
+{
+	kmem_zone_free(xfs_trans_zone, tp);
+	tp = NULL;
+}
+
 int
 libxfs_trans_alloc(
 	struct xfs_mount	*mp,
@@ -166,11 +179,8 @@ libxfs_trans_alloc(
 			return -ENOSPC;
 	}
 
-	if ((ptr = calloc(sizeof(xfs_trans_t), 1)) == NULL) {
-		fprintf(stderr, _("%s: xact calloc failed (%d bytes): %s\n"),
-			progname, (int)sizeof(xfs_trans_t), strerror(errno));
-		exit(1);
-	}
+	ptr = kmem_zone_zalloc(xfs_trans_zone,
+		(flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
 	ptr->t_mountp = mp;
 	ptr->t_blk_res = blocks;
 	INIT_LIST_HEAD(&ptr->t_items);
@@ -212,8 +222,7 @@ libxfs_trans_cancel(
 #endif
 	if (tp != NULL) {
 		xfs_trans_free_items(tp);
-		free(tp);
-		tp = NULL;
+		xfs_trans_free(tp);
 	}
 #ifdef XACT_DEBUG
 	fprintf(stderr, "## cancelled transaction %p\n", otp);
@@ -867,8 +876,7 @@ libxfs_trans_commit(
 		fprintf(stderr, "committed clean transaction %p\n", tp);
 #endif
 		xfs_trans_free_items(tp);
-		free(tp);
-		tp = NULL;
+		xfs_trans_free(tp);
 		return 0;
 	}
 
@@ -891,7 +899,6 @@ libxfs_trans_commit(
 	trans_committed(tp);
 
 	/* That's it for the transaction structure.  Free it. */
-	free(tp);
-	tp = NULL;
+	xfs_trans_free(tp);
 	return 0;
 }
-- 
1.8.3.1

[PATCH 5/8] libxfs: use a memory zone for log items

[Eric Sandeen]

In addition to more closely matching the kernel, this will
help us catch any leaks from these allocations.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/trans.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/libxfs/trans.c b/libxfs/trans.c
index 035cc22..f79fae9 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -65,13 +65,7 @@ libxfs_trans_add_item(
 	ASSERT(lip->li_mountp == tp->t_mountp);
 	ASSERT(lip->li_ailp == tp->t_mountp->m_ail);
 
-	lidp = calloc(sizeof(struct xfs_log_item_desc), 1);
-	if (!lidp) {
-		fprintf(stderr, _("%s: lidp calloc failed (%d bytes): %s\n"),
-			progname, (int)sizeof(struct xfs_log_item_desc),
-			strerror(errno));
-		exit(1);
-	}
+	lidp = kmem_zone_zalloc(xfs_log_item_desc_zone, KM_SLEEP | KM_NOFS);
 
 	lidp->lid_item = lip;
 	lidp->lid_flags = 0;
@@ -80,6 +74,14 @@ libxfs_trans_add_item(
 	lip->li_desc = lidp;
 }
 
+static void
+libxfs_trans_free_item_desc(
+        struct xfs_log_item_desc *lidp)
+{
+	list_del_init(&lidp->lid_trans);
+	kmem_zone_free(xfs_log_item_desc_zone, lidp);
+}
+
 /*
  * Unlink and free the given descriptor.
  */
@@ -87,8 +89,7 @@ void
 libxfs_trans_del_item(
 	struct xfs_log_item	*lip)
 {
-	list_del_init(&lip->li_desc->lid_trans);
-	free(lip->li_desc);
+	libxfs_trans_free_item_desc(lip->li_desc);
 	lip->li_desc = NULL;
 }
 
-- 
1.8.3.1

[PATCH 6/8] libxfs: manage xfs_buf_zone count within cache

[Eric Sandeen]

Fake up the xfs_buf_zone allocated count a bit to account
for buffers freed to and allocated from cache.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/rdwr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 474e5eb..c5ffd4d 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -665,6 +665,7 @@ __libxfs_getbufr(int blen)
 				free(bp->b_maps);
 			bp->b_maps = NULL;
 		}
+		xfs_buf_zone->allocated++;
 	} else
 		bp = kmem_zone_zalloc(xfs_buf_zone, 0);
 	pthread_mutex_unlock(&xfs_buf_freelist.cm_mutex);
@@ -1245,6 +1246,7 @@ libxfs_brelse(
 			"releasing dirty buffer to free list!");
 
 	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
+	xfs_buf_zone->allocated--;
 	list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
 	pthread_mutex_unlock(&xfs_buf_freelist.cm_mutex);
 }
@@ -1268,6 +1270,7 @@ libxfs_bulkrelse(
 	}
 
 	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
+	xfs_buf_zone->allocated -= count;
 	list_splice(list, &xfs_buf_freelist.cm_list);
 	pthread_mutex_unlock(&xfs_buf_freelist.cm_mutex);
 
-- 
1.8.3.1

[PATCH 7/8] libxfs: add function to free all bufferse in bcache

[Eric Sandeen]

libxfs_bcache_purge simply moves all "free" buffers
onto the xfs_buf_freelist mru list; add a new function to
actually free them when we tear everything down, so leak
checkers don't go nuts about lots of unfreed xfs_bufs
at exit.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 libxfs/init.c      |  2 ++
 libxfs/libxfs_io.h |  1 +
 libxfs/rdwr.c      | 16 ++++++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/libxfs/init.c b/libxfs/init.c
index 7bde8b7..aea308b 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -889,6 +889,8 @@ void
 libxfs_destroy(void)
 {
 	manage_zones(1);
+	libxfs_bcache_purge();
+	libxfs_bcache_free();
 	cache_destroy(libxfs_bcache);
 }
 
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 2fce04d..81d2804 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -191,6 +191,7 @@ extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
 			const struct xfs_buf_ops *ops);
 extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
 extern void	libxfs_bcache_purge(void);
+extern void	libxfs_bcache_free(void);
 extern void	libxfs_bcache_flush(void);
 extern void	libxfs_purgebuf(xfs_buf_t *);
 extern int	libxfs_bcache_overflowed(void);
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index c5ffd4d..1dcabdd 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1278,6 +1278,22 @@ libxfs_bulkrelse(
 }
 
 /*
+ * Free everything from the xfs_buf_freelist MRU, used at final teardown
+ */
+void
+libxfs_bcache_free(void)
+{
+	struct list_head	*cm_list;
+	xfs_buf_t		*bp, *next;
+
+ 	cm_list = &xfs_buf_freelist.cm_list;
+	list_for_each_entry_safe(bp, next, cm_list, b_node.cn_mru) {
+		free(bp->b_addr);
+		free(bp);
+	}
+}
+
+/*
  * When a buffer is marked dirty, the error is cleared. Hence if we are trying
  * to flush a buffer prior to cache reclaim that has an error on it it means
  * we've already tried to flush it and it failed. Prevent repeated corruption
-- 
1.8.3.1

[PATCH 8/8] libxfs: Catch non-empty zones on destroy

[Eric Sandeen]

Move xfs_inode_zone definition to its primary file, and add
it to the list of zones to release; properly release
xfs_ili_zone as well.

Create and use a kmem_zone_destroy which warns if we are
releasing a non-empty zone when the LIBXFS_LEAK_CHECK
environment variable is set, wire this into libxfs_destroy(),
and call that when various tools exit.

The LIBXFS_LEAK_CHECK environment variable also causes
the program to exit with failure when a leak is detected.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 copy/xfs_copy.c     |  1 +
 db/init.c           |  2 ++
 include/kmem.h      |  1 +
 libxfs/init.c       | 38 +++++++++++++++++++++++---------------
 libxfs/kmem.c       | 14 ++++++++++++++
 libxfs/rdwr.c       |  2 +-
 mkfs/xfs_mkfs.c     |  1 +
 repair/xfs_repair.c |  1 +
 8 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index fb37375..648db86 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -1214,6 +1214,7 @@ main(int argc, char **argv)
 
 	check_errors();
 	libxfs_umount(mp);
+	libxfs_destroy();
 
 	return 0;
 }
diff --git a/db/init.c b/db/init.c
index b108a06..29fc344 100644
--- a/db/init.c
+++ b/db/init.c
@@ -236,5 +236,7 @@ close_devices:
 		libxfs_device_close(x.logdev);
 	if (x.rtdev)
 		libxfs_device_close(x.rtdev);
+	libxfs_destroy();
+
 	return exitcode;
 }
diff --git a/include/kmem.h b/include/kmem.h
index 65f0ade..80ce25b 100644
--- a/include/kmem.h
+++ b/include/kmem.h
@@ -31,6 +31,7 @@ typedef struct kmem_zone {
 } kmem_zone_t;
 
 extern kmem_zone_t *kmem_zone_init(int, char *);
+extern int	kmem_zone_destroy(kmem_zone_t *);
 extern void	*kmem_zone_alloc(kmem_zone_t *, int);
 extern void	*kmem_zone_zalloc(kmem_zone_t *, int);
 
diff --git a/libxfs/init.c b/libxfs/init.c
index aea308b..e32f27c 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -43,9 +43,7 @@ int libxfs_bhash_size;		/* #buckets in bcache */
 
 int	use_xfs_buf_lock;	/* global flag: use xfs_buf_t locks for MT */
 
-static void manage_zones(int);	/* setup global zones */
-
-kmem_zone_t	*xfs_inode_zone;
+static int manage_zones(int);	/* setup/teardown global zones */
 
 /*
  * dev_map - map open devices to fd.
@@ -374,11 +372,12 @@ done:
 /*
  * Initialize/destroy all of the zone allocators we use.
  */
-static void
+static int 
 manage_zones(int release)
 {
 	extern kmem_zone_t	*xfs_buf_zone;
 	extern kmem_zone_t	*xfs_ili_zone;
+	extern kmem_zone_t	*xfs_inode_zone;
 	extern kmem_zone_t	*xfs_ifork_zone;
 	extern kmem_zone_t	*xfs_buf_item_zone;
 	extern kmem_zone_t	*xfs_da_state_zone;
@@ -389,16 +388,19 @@ manage_zones(int release)
 	extern void		xfs_dir_startup();
 
 	if (release) {	/* free zone allocation */
-		kmem_free(xfs_buf_zone);
-		kmem_free(xfs_inode_zone);
-		kmem_free(xfs_ifork_zone);
-		kmem_free(xfs_buf_item_zone);
-		kmem_free(xfs_da_state_zone);
-		kmem_free(xfs_btree_cur_zone);
-		kmem_free(xfs_bmap_free_item_zone);
-		kmem_free(xfs_trans_zone);
-		kmem_free(xfs_log_item_desc_zone);
-		return;
+		int	leaked = 0;
+
+		leaked += kmem_zone_destroy(xfs_buf_zone);
+		leaked += kmem_zone_destroy(xfs_ili_zone);
+		leaked += kmem_zone_destroy(xfs_inode_zone);
+		leaked += kmem_zone_destroy(xfs_ifork_zone);
+		leaked += kmem_zone_destroy(xfs_buf_item_zone);
+		leaked += kmem_zone_destroy(xfs_da_state_zone);
+		leaked += kmem_zone_destroy(xfs_btree_cur_zone);
+		leaked += kmem_zone_destroy(xfs_bmap_free_item_zone);
+		leaked += kmem_zone_destroy(xfs_trans_zone);
+		leaked += kmem_zone_destroy(xfs_log_item_desc_zone);
+		return leaked;
 	}
 	/* otherwise initialise zone allocation */
 	xfs_buf_zone = kmem_zone_init(sizeof(xfs_buf_t), "xfs_buffer");
@@ -420,6 +422,8 @@ manage_zones(int release)
 	xfs_log_item_desc_zone = kmem_zone_init(
 			sizeof(struct xfs_log_item_desc), "xfs_log_item_desc");
 	xfs_dir_startup();
+
+	return 0;
 }
 
 /*
@@ -888,10 +892,14 @@ libxfs_umount(xfs_mount_t *mp)
 void
 libxfs_destroy(void)
 {
-	manage_zones(1);
+	int	leaked;
+
+	leaked = manage_zones(1);
 	libxfs_bcache_purge();
 	libxfs_bcache_free();
 	cache_destroy(libxfs_bcache);
+	if (getenv("LIBXFS_LEAK_CHECK") && leaked)
+		exit(1);
 }
 
 int
diff --git a/libxfs/kmem.c b/libxfs/kmem.c
index c8bcb50..d599b3d 100644
--- a/libxfs/kmem.c
+++ b/libxfs/kmem.c
@@ -23,6 +23,20 @@ kmem_zone_init(int size, char *name)
 	return ptr;
 }
 
+int
+kmem_zone_destroy(kmem_zone_t *zone)
+{
+	int	leaked = 0;
+
+	if (getenv("LIBXFS_LEAK_CHECK") && zone->allocated) {
+		leaked = 1;
+		printf("zone %s freed with %d items allocated\n",
+					zone->zone_name, zone->allocated);
+	}
+	free(zone);
+	return leaked;
+}
+
 void *
 kmem_zone_alloc(kmem_zone_t *zone, int flags)
 {
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 1dcabdd..fdb2865 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -1352,8 +1352,8 @@ struct cache_operations libxfs_bcache_operations = {
  * Inode cache stubs.
  */
 
+kmem_zone_t		*xfs_inode_zone;
 extern kmem_zone_t	*xfs_ili_zone;
-extern kmem_zone_t	*xfs_inode_zone;
 
 int
 libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 5f1ac9f..e08d1d1 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4028,6 +4028,7 @@ main(
 	if (xi.logdev && xi.logdev != xi.ddev)
 		libxfs_device_close(xi.logdev);
 	libxfs_device_close(xi.ddev);
+	libxfs_destroy();
 
 	return 0;
 }
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index b2dd91b..312a0d0 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -1082,6 +1082,7 @@ _("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\
 	if (x.logdev && x.logdev != x.ddev)
 		libxfs_device_close(x.logdev);
 	libxfs_device_close(x.ddev);
+	libxfs_destroy();
 
 	if (verbose)
 		summary_report();
-- 
1.8.3.1

Re: [PATCH 1/8] xfs: remove wrappers around b_fspriv

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:31PM -0600, Eric Sandeen wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> Source kernel commit adadbeefb34f755a3477da51035eeeec2c1fde38
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Alex Elder <aelder@sgi.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  libxfs/libxfs_io.h |  2 --
>  libxfs/logitem.c   |  6 +++---
>  libxfs/trans.c     | 48 +++++++++++++++++++++---------------------------
>  libxfs/util.c      |  2 +-
>  4 files changed, 25 insertions(+), 33 deletions(-)
> 
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 1209e52..29fb1c7 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -111,8 +111,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
>  #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
>  #define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
>  
> -#define XFS_BUF_FSPRIVATE(bp,type)	((type)(bp)->b_fspriv)
> -#define XFS_BUF_SET_FSPRIVATE(bp,val)	(bp)->b_fspriv = (void *)(val)

/me still hates the unhelpful 'priv' name, but yay for demacroing.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  #define XFS_BUF_FSPRIVATE2(bp,type)	((type)(bp)->b_fsprivate2)
>  #define XFS_BUF_SET_FSPRIVATE2(bp,val)	(bp)->b_fsprivate2 = (void *)(val)
>  #define XFS_BUF_FSPRIVATE3(bp,type)	((type)(bp)->b_fsprivate3)
> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
> index 4dcc506..0c183b5 100644
> --- a/libxfs/logitem.c
> +++ b/libxfs/logitem.c
> @@ -99,8 +99,8 @@ xfs_buf_item_init(
>  	if (XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *) != mp)
>  		XFS_BUF_SET_FSPRIVATE3(bp, mp);
>  	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
> -	if (XFS_BUF_FSPRIVATE(bp, void *) != NULL) {
> -		lip = XFS_BUF_FSPRIVATE(bp, xfs_log_item_t *);
> +	if (bp->b_fspriv != NULL) {
> +		lip = bp->b_fspriv;
>  		if (lip->li_type == XFS_LI_BUF) {
>  #ifdef LI_DEBUG
>  			fprintf(stderr,
> @@ -123,7 +123,7 @@ xfs_buf_item_init(
>  	bip->bli_format.blf_type = XFS_LI_BUF;
>  	bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>  	bip->bli_format.blf_len = (unsigned short)BTOBB(XFS_BUF_COUNT(bp));
> -	XFS_BUF_SET_FSPRIVATE(bp, bip);
> +	bp->b_fspriv = bip;
>  }
>  
>  
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 3d64f26..6a1901b 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -297,11 +297,10 @@ libxfs_trans_inode_alloc_buf(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> -	xfs_buf_log_item_t	*bip;
> +	xfs_buf_log_item_t	*bip = bp->b_fspriv;;
>  
>  	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> +	ASSERT(bip != NULL);
>  	bip->bli_flags |= XFS_BLI_INODE_ALLOC_BUF;
>  	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
>  }
> @@ -364,12 +363,11 @@ libxfs_trans_dirty_buf(
>  	struct xfs_trans	*tp,
>  	struct xfs_buf		*bp)
>  {
> -	struct xfs_buf_log_item	*bip;
> +	struct xfs_buf_log_item	*bip = bp->b_fspriv;
>  
>  	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
> +	ASSERT(bip != NULL);
>  
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
>  #endif
> @@ -393,12 +391,10 @@ libxfs_trans_log_buf(
>  	uint			first,
>  	uint			last)
>  {
> -	struct xfs_buf_log_item	*bip;
> +	struct xfs_buf_log_item	*bip = bp->b_fspriv;
>  
>  	ASSERT((first <= last) && (last < XFS_BUF_COUNT(bp)));
>  
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> -
>  	xfs_trans_dirty_buf(tp, bp);
>  	xfs_buf_item_log(bip, first, last);
>  }
> @@ -439,7 +435,7 @@ libxfs_trans_brelse(
>  		return;
>  	}
>  	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> +	bip = bp->b_fspriv;
>  	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
>  	if (bip->bli_recur > 0) {
>  		bip->bli_recur--;
> @@ -462,15 +458,14 @@ libxfs_trans_binval(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> -	xfs_buf_log_item_t	*bip;
> +	xfs_buf_log_item_t	*bip = bp->b_fspriv;
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
>  #endif
>  
>  	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
> +	ASSERT(bip != NULL);
>  
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
>  	if (bip->bli_flags & XFS_BLI_STALE)
>  		return;
>  	XFS_BUF_UNDELAYWRITE(bp);
> @@ -496,7 +491,7 @@ libxfs_trans_bjoin(
>  #endif
>  
>  	xfs_buf_item_init(bp, tp->t_mountp);
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> +	bip = bp->b_fspriv;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  	XFS_BUF_SET_FSPRIVATE2(bp, tp);
>  }
> @@ -506,15 +501,14 @@ libxfs_trans_bhold(
>  	xfs_trans_t		*tp,
>  	xfs_buf_t		*bp)
>  {
> -	xfs_buf_log_item_t	*bip;
> +	xfs_buf_log_item_t	*bip =bp->b_fspriv;
>  
>  	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
> +	ASSERT(bp->b_fspriv != NULL);
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
>  #endif
>  
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
>  	bip->bli_flags |= XFS_BLI_HOLD;
>  }
>  
> @@ -535,7 +529,7 @@ libxfs_trans_get_buf_map(
>  	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
>  	if (bp != NULL) {
>  		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -		bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> +		bip = bp->b_fspriv;
>  		ASSERT(bip != NULL);
>  		bip->bli_recur++;
>  		return bp;
> @@ -549,7 +543,7 @@ libxfs_trans_get_buf_map(
>  #endif
>  
>  	xfs_buf_item_init(bp, tp->t_mountp);
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t*);
> +	bip = bp->b_fspriv;
>  	bip->bli_recur = 0;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  
> @@ -575,7 +569,7 @@ libxfs_trans_getsb(
>  	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
>  	if (bp != NULL) {
>  		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -		bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> +		bip = bp->b_fspriv;
>  		ASSERT(bip != NULL);
>  		bip->bli_recur++;
>  		return bp;
> @@ -587,7 +581,7 @@ libxfs_trans_getsb(
>  #endif
>  
>  	xfs_buf_item_init(bp, mp);
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t*);
> +	bip = bp->b_fspriv;
>  	bip->bli_recur = 0;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  
> @@ -626,8 +620,8 @@ libxfs_trans_read_buf_map(
>  	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
>  	if (bp != NULL) {
>  		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> -		ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
> -		bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t*);
> +		ASSERT(bp->b_fspriv != NULL);
> +		bip = bp->b_fspriv;
>  		bip->bli_recur++;
>  		goto done;
>  	}
> @@ -644,7 +638,7 @@ libxfs_trans_read_buf_map(
>  #endif
>  
>  	xfs_buf_item_init(bp, tp->t_mountp);
> -	bip = XFS_BUF_FSPRIVATE(bp, xfs_buf_log_item_t *);
> +	bip = bp->b_fspriv;
>  	bip->bli_recur = 0;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  
> @@ -732,7 +726,7 @@ inode_item_done(
>  		return;
>  	}
>  
> -	XFS_BUF_SET_FSPRIVATE(bp, iip);
> +	bp->b_fspriv = iip;
>  	error = libxfs_iflush_int(ip, bp);
>  	if (error) {
>  		fprintf(stderr, _("%s: warning - iflush_int failed (%d)\n"),
> @@ -741,7 +735,7 @@ inode_item_done(
>  	}
>  
>  	ip->i_transp = NULL;	/* disassociate from transaction */
> -	XFS_BUF_SET_FSPRIVATE(bp, NULL);	/* remove log item */
> +	bp->b_fspriv = NULL;			/* remove log item */
>  	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
>  	libxfs_writebuf(bp, 0);
>  #ifdef XACT_DEBUG
> @@ -760,7 +754,7 @@ buf_item_done(
>  
>  	bp = bip->bli_buf;
>  	ASSERT(bp != NULL);
> -	XFS_BUF_SET_FSPRIVATE(bp, NULL);	/* remove log item */
> +	bp->b_fspriv = NULL;			/* remove log item */
>  	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
>  
>  	hold = (bip->bli_flags & XFS_BLI_HOLD);
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 8003b26..5f49b82 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -466,7 +466,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
>  	xfs_dinode_t		*dip;
>  	xfs_mount_t		*mp;
>  
> -	ASSERT(XFS_BUF_FSPRIVATE(bp, void *) != NULL);
> +	ASSERT(bp-b_fspriv != NULL);
>  	ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE ||
>  		ip->i_d.di_nextents > ip->i_df.if_ext_max);
>  	ASSERT(ip->i_d.di_version > 1);
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/8] xfs: add a proper transaction pointer to struct xfs_buf

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:32PM -0600, Eric Sandeen wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> Source kernel commit bf9d9013a2a559858efb590bf922377be9d6d969
> 
> Replace the typeless b_fspriv2 and the ugly macros around it with a properly
> typed transaction pointer.  As a fallout the log buffer state debug checks
> are also removed.  We could have kept them using casts, but as they do
> not have a real purpose we can as well just remove them.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Alex Elder <aelder@sgi.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  libxfs/libxfs_io.h |  4 +---
>  libxfs/trans.c     | 44 ++++++++++++++++++++++----------------------
>  2 files changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 29fb1c7..5acd3df 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -69,7 +69,7 @@ typedef struct xfs_buf {
>  	pthread_t		b_holder;
>  	unsigned int		b_recur;
>  	void			*b_fspriv;
> -	void			*b_fsprivate2;
> +	void			*b_transp;
>  	void			*b_fsprivate3;
>  	void			*b_addr;
>  	int			b_error;
> @@ -111,8 +111,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
>  #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
>  #define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
>  
> -#define XFS_BUF_FSPRIVATE2(bp,type)	((type)(bp)->b_fsprivate2)
> -#define XFS_BUF_SET_FSPRIVATE2(bp,val)	(bp)->b_fsprivate2 = (void *)(val)
>  #define XFS_BUF_FSPRIVATE3(bp,type)	((type)(bp)->b_fsprivate3)
>  #define XFS_BUF_SET_FSPRIVATE3(bp,val)	(bp)->b_fsprivate3 = (void *)(val)
>  
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 6a1901b..57ff3ea 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -299,7 +299,7 @@ libxfs_trans_inode_alloc_buf(
>  {
>  	xfs_buf_log_item_t	*bip = bp->b_fspriv;;
>  
> -	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +	ASSERT(bp->bp_transp == tp);
>  	ASSERT(bip != NULL);
>  	bip->bli_flags |= XFS_BLI_INODE_ALLOC_BUF;
>  	xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
> @@ -365,7 +365,7 @@ libxfs_trans_dirty_buf(
>  {
>  	struct xfs_buf_log_item	*bip = bp->b_fspriv;
>  
> -	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +	ASSERT(bp->bp_transp == tp);
>  	ASSERT(bip != NULL);
>  
>  #ifdef XACT_DEBUG
> @@ -430,11 +430,11 @@ libxfs_trans_brelse(
>  #endif
>  
>  	if (tp == NULL) {
> -		ASSERT(XFS_BUF_FSPRIVATE2(bp, void *) == NULL);
> +		ASSERT(bp->bp_transp == NULL);
>  		libxfs_putbuf(bp);
>  		return;
>  	}
> -	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +	ASSERT(bp->bp_transp == tp);
>  	bip = bp->b_fspriv;
>  	ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
>  	if (bip->bli_recur > 0) {
> @@ -449,7 +449,7 @@ libxfs_trans_brelse(
>  	xfs_trans_del_item(&bip->bli_item);
>  	if (bip->bli_flags & XFS_BLI_HOLD)
>  		bip->bli_flags &= ~XFS_BLI_HOLD;
> -	XFS_BUF_SET_FSPRIVATE2(bp, NULL);
> +	bp->b_transp = NULL;
>  	libxfs_putbuf(bp);
>  }
>  
> @@ -463,7 +463,7 @@ libxfs_trans_binval(
>  	fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
>  #endif
>  
> -	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +	ASSERT(bp->bp_transp == tp);
>  	ASSERT(bip != NULL);
>  
>  	if (bip->bli_flags & XFS_BLI_STALE)
> @@ -485,7 +485,7 @@ libxfs_trans_bjoin(
>  {
>  	xfs_buf_log_item_t	*bip;
>  
> -	ASSERT(XFS_BUF_FSPRIVATE2(bp, void *) == NULL);
> +	ASSERT(bp->bp_transp == NULL);
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
>  #endif
> @@ -493,7 +493,7 @@ libxfs_trans_bjoin(
>  	xfs_buf_item_init(bp, tp->t_mountp);
>  	bip = bp->b_fspriv;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> -	XFS_BUF_SET_FSPRIVATE2(bp, tp);
> +	bp->b_transp = tp;
>  }
>  
>  void
> @@ -503,7 +503,7 @@ libxfs_trans_bhold(
>  {
>  	xfs_buf_log_item_t	*bip =bp->b_fspriv;
>  
> -	ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +	ASSERT(bp->bp_transp == tp);
>  	ASSERT(bp->b_fspriv != NULL);
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
> @@ -528,7 +528,7 @@ libxfs_trans_get_buf_map(
>  
>  	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
>  	if (bp != NULL) {
> -		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +		ASSERT(bp->bp_transp == tp);
>  		bip = bp->b_fspriv;
>  		ASSERT(bip != NULL);
>  		bip->bli_recur++;
> @@ -547,8 +547,8 @@ libxfs_trans_get_buf_map(
>  	bip->bli_recur = 0;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  
> -	/* initialize b_fsprivate2 so we can find it incore */
> -	XFS_BUF_SET_FSPRIVATE2(bp, tp);
> +	/* initialize b_transp so we can find it incore */
> +	bp->b_transp = tp;
>  	return bp;
>  }
>  
> @@ -568,7 +568,7 @@ libxfs_trans_getsb(
>  
>  	bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
>  	if (bp != NULL) {
> -		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +		ASSERT(bp->bp_transp == tp);
>  		bip = bp->b_fspriv;
>  		ASSERT(bip != NULL);
>  		bip->bli_recur++;
> @@ -585,8 +585,8 @@ libxfs_trans_getsb(
>  	bip->bli_recur = 0;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  
> -	/* initialize b_fsprivate2 so we can find it incore */
> -	XFS_BUF_SET_FSPRIVATE2(bp, tp);
> +	/* initialize b_transp so we can find it incore */
> +	bp->b_transp = tp;
>  	return bp;
>  }
>  
> @@ -619,7 +619,7 @@ libxfs_trans_read_buf_map(
>  
>  	bp = xfs_trans_buf_item_match(tp, btp, map, nmaps);
>  	if (bp != NULL) {
> -		ASSERT(XFS_BUF_FSPRIVATE2(bp, xfs_trans_t *) == tp);
> +		ASSERT(bp->bp_transp == tp);
>  		ASSERT(bp->b_fspriv != NULL);
>  		bip = bp->b_fspriv;
>  		bip->bli_recur++;
> @@ -642,8 +642,8 @@ libxfs_trans_read_buf_map(
>  	bip->bli_recur = 0;
>  	xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>  
> -	/* initialise b_fsprivate2 so we can find it incore */
> -	XFS_BUF_SET_FSPRIVATE2(bp, tp);
> +	/* initialise b_transp so we can find it incore */
> +	bp->b_transp = tp;
>  done:
>  	*bpp = bp;
>  	return 0;
> @@ -735,8 +735,8 @@ inode_item_done(
>  	}
>  
>  	ip->i_transp = NULL;	/* disassociate from transaction */
> -	bp->b_fspriv = NULL;			/* remove log item */
> -	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
> +	bp->b_fspriv = NULL;	/* remove log item */
> +	bp->b_transp = NULL;	/* remove xact ptr */
>  	libxfs_writebuf(bp, 0);
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
> @@ -755,7 +755,7 @@ buf_item_done(
>  	bp = bip->bli_buf;
>  	ASSERT(bp != NULL);
>  	bp->b_fspriv = NULL;			/* remove log item */
> -	XFS_BUF_SET_FSPRIVATE2(bp, NULL);	/* remove xact ptr */
> +	bp->b_transp = NULL;			/* remove xact ptr */
>  
>  	hold = (bip->bli_flags & XFS_BLI_HOLD);
>  	if (bip->bli_flags & XFS_BLI_DIRTY) {
> @@ -804,7 +804,7 @@ buf_item_unlock(
>  	uint			hold;
>  
>  	/* Clear the buffer's association with this transaction. */
> -	XFS_BUF_SET_FSPRIVATE2(bip->bli_buf, NULL);
> +	bip->bli_buf->b_transp = NULL;
>  
>  	hold = bip->bli_flags & XFS_BLI_HOLD;
>  	bip->bli_flags &= ~XFS_BLI_HOLD;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/8] xfs: remove unused buf_fsprivate3

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:33PM -0600, Eric Sandeen wrote:
> The buf_fsprivate3 field has no actual use, other than a pointless
> "if it's not set, set it" in xfs_buf_item_init; nobody cares after
> that.  Remove it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  libxfs/libxfs_io.h | 4 ----
>  libxfs/logitem.c   | 2 --
>  2 files changed, 6 deletions(-)
> 
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 5acd3df..2fce04d 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -70,7 +70,6 @@ typedef struct xfs_buf {
>  	unsigned int		b_recur;
>  	void			*b_fspriv;
>  	void			*b_transp;
> -	void			*b_fsprivate3;
>  	void			*b_addr;
>  	int			b_error;
>  	const struct xfs_buf_ops *b_ops;
> @@ -111,9 +110,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
>  #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
>  #define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
>  
> -#define XFS_BUF_FSPRIVATE3(bp,type)	((type)(bp)->b_fsprivate3)
> -#define XFS_BUF_SET_FSPRIVATE3(bp,val)	(bp)->b_fsprivate3 = (void *)(val)
> -
>  #define XFS_BUF_SET_PRIORITY(bp,pri)	cache_node_set_priority( \
>  						libxfs_bcache, \
>  						(struct cache_node *)(bp), \
> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
> index 0c183b5..0c50fcf 100644
> --- a/libxfs/logitem.c
> +++ b/libxfs/logitem.c
> @@ -96,8 +96,6 @@ xfs_buf_item_init(
>  	 * the first.  If we do already have one, there is
>  	 * nothing to do here so return.
>  	 */
> -	if (XFS_BUF_FSPRIVATE3(bp, xfs_mount_t *) != mp)
> -		XFS_BUF_SET_FSPRIVATE3(bp, mp);
>  	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
>  	if (bp->b_fspriv != NULL) {
>  		lip = bp->b_fspriv;
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] libxfs: use a memory zone for transactions

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:34PM -0600, Eric Sandeen wrote:
> In addition to more closely matching the kernel, this will
> help us catch any leaks from these allocations.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  libxfs/init.c  |  4 ++++
>  libxfs/trans.c | 29 ++++++++++++++++++-----------
>  2 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 302f088..7bde8b7 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -384,6 +384,7 @@ manage_zones(int release)
>  	extern kmem_zone_t	*xfs_da_state_zone;
>  	extern kmem_zone_t	*xfs_btree_cur_zone;
>  	extern kmem_zone_t	*xfs_bmap_free_item_zone;
> +	extern kmem_zone_t	*xfs_trans_zone;
>  	extern kmem_zone_t	*xfs_log_item_desc_zone;
>  	extern void		xfs_dir_startup();
>  
> @@ -395,6 +396,7 @@ manage_zones(int release)
>  		kmem_free(xfs_da_state_zone);
>  		kmem_free(xfs_btree_cur_zone);
>  		kmem_free(xfs_bmap_free_item_zone);
> +		kmem_free(xfs_trans_zone);
>  		kmem_free(xfs_log_item_desc_zone);
>  		return;
>  	}
> @@ -413,6 +415,8 @@ manage_zones(int release)
>  	xfs_bmap_free_item_zone = kmem_zone_init(
>  			sizeof(struct xfs_extent_free_item),
>  			"xfs_bmap_free_item");
> +	xfs_trans_zone = kmem_zone_init(
> +			sizeof(struct xfs_trans), "xfs_trans");
>  	xfs_log_item_desc_zone = kmem_zone_init(
>  			sizeof(struct xfs_log_item_desc), "xfs_log_item_desc");
>  	xfs_dir_startup();
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 57ff3ea..035cc22 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -36,6 +36,7 @@ static void xfs_trans_free_items(struct xfs_trans *tp);
>   * Simple transaction interface
>   */
>  
> +kmem_zone_t	*xfs_trans_zone;
>  kmem_zone_t	*xfs_log_item_desc_zone;
>  
>  /*
> @@ -143,6 +144,18 @@ libxfs_trans_roll(
>  	return 0;
>  }
>  
> +/*
> + * Free the transaction structure.  If there is more clean up
> + * to do when the structure is freed, add it here.
> + */
> +static void
> +xfs_trans_free(
> +	struct xfs_trans	*tp)
> +{
> +	kmem_zone_free(xfs_trans_zone, tp);
> +	tp = NULL;

I don't think we need to NULL the local variable and then return.

--D

> +}
> +
>  int
>  libxfs_trans_alloc(
>  	struct xfs_mount	*mp,
> @@ -166,11 +179,8 @@ libxfs_trans_alloc(
>  			return -ENOSPC;
>  	}
>  
> -	if ((ptr = calloc(sizeof(xfs_trans_t), 1)) == NULL) {
> -		fprintf(stderr, _("%s: xact calloc failed (%d bytes): %s\n"),
> -			progname, (int)sizeof(xfs_trans_t), strerror(errno));
> -		exit(1);
> -	}
> +	ptr = kmem_zone_zalloc(xfs_trans_zone,
> +		(flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
>  	ptr->t_mountp = mp;
>  	ptr->t_blk_res = blocks;
>  	INIT_LIST_HEAD(&ptr->t_items);
> @@ -212,8 +222,7 @@ libxfs_trans_cancel(
>  #endif
>  	if (tp != NULL) {
>  		xfs_trans_free_items(tp);
> -		free(tp);
> -		tp = NULL;
> +		xfs_trans_free(tp);
>  	}
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "## cancelled transaction %p\n", otp);
> @@ -867,8 +876,7 @@ libxfs_trans_commit(
>  		fprintf(stderr, "committed clean transaction %p\n", tp);
>  #endif
>  		xfs_trans_free_items(tp);
> -		free(tp);
> -		tp = NULL;
> +		xfs_trans_free(tp);
>  		return 0;
>  	}
>  
> @@ -891,7 +899,6 @@ libxfs_trans_commit(
>  	trans_committed(tp);
>  
>  	/* That's it for the transaction structure.  Free it. */
> -	free(tp);
> -	tp = NULL;
> +	xfs_trans_free(tp);
>  	return 0;
>  }
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] libxfs: use a memory zone for log items

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:35PM -0600, Eric Sandeen wrote:
> In addition to more closely matching the kernel, this will
> help us catch any leaks from these allocations.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
>  libxfs/trans.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 035cc22..f79fae9 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -65,13 +65,7 @@ libxfs_trans_add_item(
>  	ASSERT(lip->li_mountp == tp->t_mountp);
>  	ASSERT(lip->li_ailp == tp->t_mountp->m_ail);
>  
> -	lidp = calloc(sizeof(struct xfs_log_item_desc), 1);
> -	if (!lidp) {
> -		fprintf(stderr, _("%s: lidp calloc failed (%d bytes): %s\n"),
> -			progname, (int)sizeof(struct xfs_log_item_desc),
> -			strerror(errno));
> -		exit(1);
> -	}
> +	lidp = kmem_zone_zalloc(xfs_log_item_desc_zone, KM_SLEEP | KM_NOFS);
>  
>  	lidp->lid_item = lip;
>  	lidp->lid_flags = 0;
> @@ -80,6 +74,14 @@ libxfs_trans_add_item(
>  	lip->li_desc = lidp;
>  }
>  
> +static void
> +libxfs_trans_free_item_desc(
> +        struct xfs_log_item_desc *lidp)
> +{
> +	list_del_init(&lidp->lid_trans);
> +	kmem_zone_free(xfs_log_item_desc_zone, lidp);
> +}
> +
>  /*
>   * Unlink and free the given descriptor.
>   */
> @@ -87,8 +89,7 @@ void
>  libxfs_trans_del_item(
>  	struct xfs_log_item	*lip)
>  {
> -	list_del_init(&lip->li_desc->lid_trans);
> -	free(lip->li_desc);
> +	libxfs_trans_free_item_desc(lip->li_desc);
>  	lip->li_desc = NULL;
>  }
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] libxfs: add function to free all bufferse in bcache

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:37PM -0600, Eric Sandeen wrote:
> Subject: libxfs: add function to free all bufferse in bcache

s/bufferse/buffers/

> libxfs_bcache_purge simply moves all "free" buffers
> onto the xfs_buf_freelist mru list; add a new function to
> actually free them when we tear everything down, so leak
> checkers don't go nuts about lots of unfreed xfs_bufs
> at exit.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  libxfs/init.c      |  2 ++
>  libxfs/libxfs_io.h |  1 +
>  libxfs/rdwr.c      | 16 ++++++++++++++++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 7bde8b7..aea308b 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -889,6 +889,8 @@ void
>  libxfs_destroy(void)
>  {
>  	manage_zones(1);
> +	libxfs_bcache_purge();
> +	libxfs_bcache_free();
>  	cache_destroy(libxfs_bcache);
>  }
>  
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 2fce04d..81d2804 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -191,6 +191,7 @@ extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
>  			const struct xfs_buf_ops *ops);
>  extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
>  extern void	libxfs_bcache_purge(void);
> +extern void	libxfs_bcache_free(void);
>  extern void	libxfs_bcache_flush(void);
>  extern void	libxfs_purgebuf(xfs_buf_t *);
>  extern int	libxfs_bcache_overflowed(void);
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index c5ffd4d..1dcabdd 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1278,6 +1278,22 @@ libxfs_bulkrelse(
>  }
>  
>  /*
> + * Free everything from the xfs_buf_freelist MRU, used at final teardown
> + */
> +void
> +libxfs_bcache_free(void)
> +{
> +	struct list_head	*cm_list;
> +	xfs_buf_t		*bp, *next;
> +
> + 	cm_list = &xfs_buf_freelist.cm_list;
> +	list_for_each_entry_safe(bp, next, cm_list, b_node.cn_mru) {
> +		free(bp->b_addr);

Hm, do we need to free bp->b_maps here?  Just looking at
__libxfs_getbufr (aka the other function that can tear down a
bp->b_addr).

> +		free(bp);

kmem_zone_free, since we got the bp from kmem_zone_alloc.

Do we need to reinit xfs_buf_freelist.cm_list, since ->next now points
to freed list items?

--D

> +	}
> +}
> +
> +/*
>   * When a buffer is marked dirty, the error is cleared. Hence if we are trying
>   * to flush a buffer prior to cache reclaim that has an error on it it means
>   * we've already tried to flush it and it failed. Prevent repeated corruption
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] libxfs: add function to free all bufferse in bcache

[Eric Sandeen]

On 1/11/18 2:05 PM, Darrick J. Wong wrote:
> On Thu, Jan 11, 2018 at 01:37:37PM -0600, Eric Sandeen wrote:
>> Subject: libxfs: add function to free all bufferse in bcache
> 
> s/bufferse/buffers/

That's just my flair ;)

>> libxfs_bcache_purge simply moves all "free" buffers
>> onto the xfs_buf_freelist mru list; add a new function to
>> actually free them when we tear everything down, so leak
>> checkers don't go nuts about lots of unfreed xfs_bufs
>> at exit.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
>>  libxfs/init.c      |  2 ++
>>  libxfs/libxfs_io.h |  1 +
>>  libxfs/rdwr.c      | 16 ++++++++++++++++
>>  3 files changed, 19 insertions(+)
>>
>> diff --git a/libxfs/init.c b/libxfs/init.c
>> index 7bde8b7..aea308b 100644
>> --- a/libxfs/init.c
>> +++ b/libxfs/init.c
>> @@ -889,6 +889,8 @@ void
>>  libxfs_destroy(void)
>>  {
>>  	manage_zones(1);
>> +	libxfs_bcache_purge();
>> +	libxfs_bcache_free();
>>  	cache_destroy(libxfs_bcache);
>>  }
>>  
>> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
>> index 2fce04d..81d2804 100644
>> --- a/libxfs/libxfs_io.h
>> +++ b/libxfs/libxfs_io.h
>> @@ -191,6 +191,7 @@ extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
>>  			const struct xfs_buf_ops *ops);
>>  extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
>>  extern void	libxfs_bcache_purge(void);
>> +extern void	libxfs_bcache_free(void);
>>  extern void	libxfs_bcache_flush(void);
>>  extern void	libxfs_purgebuf(xfs_buf_t *);
>>  extern int	libxfs_bcache_overflowed(void);
>> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
>> index c5ffd4d..1dcabdd 100644
>> --- a/libxfs/rdwr.c
>> +++ b/libxfs/rdwr.c
>> @@ -1278,6 +1278,22 @@ libxfs_bulkrelse(
>>  }
>>  
>>  /*
>> + * Free everything from the xfs_buf_freelist MRU, used at final teardown
>> + */
>> +void
>> +libxfs_bcache_free(void)
>> +{
>> +	struct list_head	*cm_list;
>> +	xfs_buf_t		*bp, *next;
>> +
>> + 	cm_list = &xfs_buf_freelist.cm_list;
>> +	list_for_each_entry_safe(bp, next, cm_list, b_node.cn_mru) {
>> +		free(bp->b_addr);
> 
> Hm, do we need to free bp->b_maps here?  Just looking at
> __libxfs_getbufr (aka the other function that can tear down a
> bp->b_addr).

Oh yeah, probably so.
>> +		free(bp);
> 
> kmem_zone_free, since we got the bp from kmem_zone_alloc.

Well remember I faked up the zone allocation count for buffers in and out of
cache. If I zone_free here, count ll go way negative (but nobody will check at
that point).  I'm not sure if this is all too strange to live...?

> Do we need to reinit xfs_buf_freelist.cm_list, since ->next now points
> to freed list items?

Probably so.  Nobody uses it at this point anyway but still, best to leave it
in a sane state I guess.

-Eric

Re: [PATCH 6/8] libxfs: manage xfs_buf_zone count within cache

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:36PM -0600, Eric Sandeen wrote:
> Fake up the xfs_buf_zone allocated count a bit to account
> for buffers freed to and allocated from cache.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  libxfs/rdwr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 474e5eb..c5ffd4d 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -665,6 +665,7 @@ __libxfs_getbufr(int blen)
>  				free(bp->b_maps);
>  			bp->b_maps = NULL;
>  		}
> +		xfs_buf_zone->allocated++;

I'm confused.  kmem_zone_zalloc increments zone->allocated and
kmem_zone_free decrements it.  Now we're messing with ->allocated
ourselves, which means that buffers that have ended up on
xfs_buf_freelist are still allocated (according to the heap) but the
zone accounting thinks the buffer is freed?

<goes away, reviews the next patch, comes back>

The next patch has xfs_buf_freelist frees the buffer directly and
bypassing kmem_zone_free.  So I wonder, if you change the next patch to
call kmem_zone_free and kill this patch, won't the accounting work out?

(xfs buf zone allocated == 0)

bp = _zone_alloc()
xfs_buf_zone->allocated++

...do stuff with buffer...

xfs_buf_relse(bp)
list_add(bp, xfs_buf_freelist)

...buffer still allocated, xfs_buf_zone->allocated == 1...

libxfs_bcache_free()
_zone_free(bp)
xfs_buf_zone->allocated--

...exit program, and:
xfs_buf_zone->allocated == 0

I think killing this patch and teaching the next one to kmem_zone_free
the buffer works, right?  Or am I missing some subtlety here, in which
case said subtlety needs comments.

--D

>  	} else
>  		bp = kmem_zone_zalloc(xfs_buf_zone, 0);
>  	pthread_mutex_unlock(&xfs_buf_freelist.cm_mutex);
> @@ -1245,6 +1246,7 @@ libxfs_brelse(
>  			"releasing dirty buffer to free list!");
>  
>  	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
> +	xfs_buf_zone->allocated--;
>  	list_add(&bp->b_node.cn_mru, &xfs_buf_freelist.cm_list);
>  	pthread_mutex_unlock(&xfs_buf_freelist.cm_mutex);
>  }
> @@ -1268,6 +1270,7 @@ libxfs_bulkrelse(
>  	}
>  
>  	pthread_mutex_lock(&xfs_buf_freelist.cm_mutex);
> +	xfs_buf_zone->allocated -= count;
>  	list_splice(list, &xfs_buf_freelist.cm_list);
>  	pthread_mutex_unlock(&xfs_buf_freelist.cm_mutex);
>  
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] libxfs: add function to free all bufferse in bcache

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 02:11:09PM -0600, Eric Sandeen wrote:
> On 1/11/18 2:05 PM, Darrick J. Wong wrote:
> > On Thu, Jan 11, 2018 at 01:37:37PM -0600, Eric Sandeen wrote:
> >> Subject: libxfs: add function to free all bufferse in bcache
> > 
> > s/bufferse/buffers/
> 
> That's just my flair ;)
> 
> >> libxfs_bcache_purge simply moves all "free" buffers
> >> onto the xfs_buf_freelist mru list; add a new function to
> >> actually free them when we tear everything down, so leak
> >> checkers don't go nuts about lots of unfreed xfs_bufs
> >> at exit.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> >> ---
> >>  libxfs/init.c      |  2 ++
> >>  libxfs/libxfs_io.h |  1 +
> >>  libxfs/rdwr.c      | 16 ++++++++++++++++
> >>  3 files changed, 19 insertions(+)
> >>
> >> diff --git a/libxfs/init.c b/libxfs/init.c
> >> index 7bde8b7..aea308b 100644
> >> --- a/libxfs/init.c
> >> +++ b/libxfs/init.c
> >> @@ -889,6 +889,8 @@ void
> >>  libxfs_destroy(void)
> >>  {
> >>  	manage_zones(1);
> >> +	libxfs_bcache_purge();
> >> +	libxfs_bcache_free();
> >>  	cache_destroy(libxfs_bcache);
> >>  }
> >>  
> >> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> >> index 2fce04d..81d2804 100644
> >> --- a/libxfs/libxfs_io.h
> >> +++ b/libxfs/libxfs_io.h
> >> @@ -191,6 +191,7 @@ extern void	libxfs_readbuf_verify(struct xfs_buf *bp,
> >>  			const struct xfs_buf_ops *ops);
> >>  extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
> >>  extern void	libxfs_bcache_purge(void);
> >> +extern void	libxfs_bcache_free(void);
> >>  extern void	libxfs_bcache_flush(void);
> >>  extern void	libxfs_purgebuf(xfs_buf_t *);
> >>  extern int	libxfs_bcache_overflowed(void);
> >> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> >> index c5ffd4d..1dcabdd 100644
> >> --- a/libxfs/rdwr.c
> >> +++ b/libxfs/rdwr.c
> >> @@ -1278,6 +1278,22 @@ libxfs_bulkrelse(
> >>  }
> >>  
> >>  /*
> >> + * Free everything from the xfs_buf_freelist MRU, used at final teardown
> >> + */
> >> +void
> >> +libxfs_bcache_free(void)
> >> +{
> >> +	struct list_head	*cm_list;
> >> +	xfs_buf_t		*bp, *next;
> >> +
> >> + 	cm_list = &xfs_buf_freelist.cm_list;
> >> +	list_for_each_entry_safe(bp, next, cm_list, b_node.cn_mru) {
> >> +		free(bp->b_addr);
> > 
> > Hm, do we need to free bp->b_maps here?  Just looking at
> > __libxfs_getbufr (aka the other function that can tear down a
> > bp->b_addr).
> 
> Oh yeah, probably so.
> >> +		free(bp);
> > 
> > kmem_zone_free, since we got the bp from kmem_zone_alloc.
> 
> Well remember I faked up the zone allocation count for buffers in and out of
> cache. If I zone_free here, count ll go way negative (but nobody will check at
> that point).  I'm not sure if this is all too strange to live...?

Yep, and I (just replied to the other patch) think the allocation counts
faking should go away, unless there's some reason why we need to have
xfs_buf_zone->allocated == 0 when there are buffers still hanging out on
xfs_buf_freelist.

> > Do we need to reinit xfs_buf_freelist.cm_list, since ->next now points
> > to freed list items?
> 
> Probably so.  Nobody uses it at this point anyway but still, best to leave it
> in a sane state I guess.

<nod>

--D

> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/8] libxfs: Catch non-empty zones on destroy

[Darrick J. Wong]

On Thu, Jan 11, 2018 at 01:37:38PM -0600, Eric Sandeen wrote:
> Move xfs_inode_zone definition to its primary file, and add
> it to the list of zones to release; properly release
> xfs_ili_zone as well.
> 
> Create and use a kmem_zone_destroy which warns if we are
> releasing a non-empty zone when the LIBXFS_LEAK_CHECK
> environment variable is set, wire this into libxfs_destroy(),
> and call that when various tools exit.
> 
> The LIBXFS_LEAK_CHECK environment variable also causes
> the program to exit with failure when a leak is detected.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  copy/xfs_copy.c     |  1 +
>  db/init.c           |  2 ++
>  include/kmem.h      |  1 +
>  libxfs/init.c       | 38 +++++++++++++++++++++++---------------
>  libxfs/kmem.c       | 14 ++++++++++++++
>  libxfs/rdwr.c       |  2 +-
>  mkfs/xfs_mkfs.c     |  1 +
>  repair/xfs_repair.c |  1 +
>  8 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
> index fb37375..648db86 100644
> --- a/copy/xfs_copy.c
> +++ b/copy/xfs_copy.c
> @@ -1214,6 +1214,7 @@ main(int argc, char **argv)
>  
>  	check_errors();
>  	libxfs_umount(mp);
> +	libxfs_destroy();

These added libxfs_destroy calls should go in a separate patch, since
it's a different logical change from complaining about leaks.

>  
>  	return 0;
>  }
> diff --git a/db/init.c b/db/init.c
> index b108a06..29fc344 100644
> --- a/db/init.c
> +++ b/db/init.c
> @@ -236,5 +236,7 @@ close_devices:
>  		libxfs_device_close(x.logdev);
>  	if (x.rtdev)
>  		libxfs_device_close(x.rtdev);
> +	libxfs_destroy();
> +
>  	return exitcode;
>  }
> diff --git a/include/kmem.h b/include/kmem.h
> index 65f0ade..80ce25b 100644
> --- a/include/kmem.h
> +++ b/include/kmem.h
> @@ -31,6 +31,7 @@ typedef struct kmem_zone {
>  } kmem_zone_t;
>  
>  extern kmem_zone_t *kmem_zone_init(int, char *);
> +extern int	kmem_zone_destroy(kmem_zone_t *);
>  extern void	*kmem_zone_alloc(kmem_zone_t *, int);
>  extern void	*kmem_zone_zalloc(kmem_zone_t *, int);
>  
> diff --git a/libxfs/init.c b/libxfs/init.c
> index aea308b..e32f27c 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -43,9 +43,7 @@ int libxfs_bhash_size;		/* #buckets in bcache */
>  
>  int	use_xfs_buf_lock;	/* global flag: use xfs_buf_t locks for MT */
>  
> -static void manage_zones(int);	/* setup global zones */
> -
> -kmem_zone_t	*xfs_inode_zone;
> +static int manage_zones(int);	/* setup/teardown global zones */
>  
>  /*
>   * dev_map - map open devices to fd.
> @@ -374,11 +372,12 @@ done:
>  /*
>   * Initialize/destroy all of the zone allocators we use.
>   */
> -static void
> +static int 

Trailing space.  Also, you are being quite optimistic that we won't ever
leak more than 2^31 buff...OH, I see, this is the number of zones with
leaks, not the number of leaked zone items.

>  manage_zones(int release)
>  {
>  	extern kmem_zone_t	*xfs_buf_zone;
>  	extern kmem_zone_t	*xfs_ili_zone;
> +	extern kmem_zone_t	*xfs_inode_zone;
>  	extern kmem_zone_t	*xfs_ifork_zone;
>  	extern kmem_zone_t	*xfs_buf_item_zone;
>  	extern kmem_zone_t	*xfs_da_state_zone;
> @@ -389,16 +388,19 @@ manage_zones(int release)
>  	extern void		xfs_dir_startup();
>  
>  	if (release) {	/* free zone allocation */
> -		kmem_free(xfs_buf_zone);
> -		kmem_free(xfs_inode_zone);
> -		kmem_free(xfs_ifork_zone);
> -		kmem_free(xfs_buf_item_zone);
> -		kmem_free(xfs_da_state_zone);
> -		kmem_free(xfs_btree_cur_zone);
> -		kmem_free(xfs_bmap_free_item_zone);
> -		kmem_free(xfs_trans_zone);
> -		kmem_free(xfs_log_item_desc_zone);
> -		return;
> +		int	leaked = 0;
> +
> +		leaked += kmem_zone_destroy(xfs_buf_zone);
> +		leaked += kmem_zone_destroy(xfs_ili_zone);
> +		leaked += kmem_zone_destroy(xfs_inode_zone);
> +		leaked += kmem_zone_destroy(xfs_ifork_zone);
> +		leaked += kmem_zone_destroy(xfs_buf_item_zone);
> +		leaked += kmem_zone_destroy(xfs_da_state_zone);
> +		leaked += kmem_zone_destroy(xfs_btree_cur_zone);
> +		leaked += kmem_zone_destroy(xfs_bmap_free_item_zone);
> +		leaked += kmem_zone_destroy(xfs_trans_zone);
> +		leaked += kmem_zone_destroy(xfs_log_item_desc_zone);
> +		return leaked;
>  	}
>  	/* otherwise initialise zone allocation */
>  	xfs_buf_zone = kmem_zone_init(sizeof(xfs_buf_t), "xfs_buffer");
> @@ -420,6 +422,8 @@ manage_zones(int release)
>  	xfs_log_item_desc_zone = kmem_zone_init(
>  			sizeof(struct xfs_log_item_desc), "xfs_log_item_desc");
>  	xfs_dir_startup();
> +
> +	return 0;
>  }
>  
>  /*
> @@ -888,10 +892,14 @@ libxfs_umount(xfs_mount_t *mp)
>  void
>  libxfs_destroy(void)
>  {
> -	manage_zones(1);
> +	int	leaked;
> +
> +	leaked = manage_zones(1);
>  	libxfs_bcache_purge();
>  	libxfs_bcache_free();
>  	cache_destroy(libxfs_bcache);
> +	if (getenv("LIBXFS_LEAK_CHECK") && leaked)
> +		exit(1);
>  }
>  
>  int
> diff --git a/libxfs/kmem.c b/libxfs/kmem.c
> index c8bcb50..d599b3d 100644
> --- a/libxfs/kmem.c
> +++ b/libxfs/kmem.c
> @@ -23,6 +23,20 @@ kmem_zone_init(int size, char *name)
>  	return ptr;
>  }
>  
> +int
> +kmem_zone_destroy(kmem_zone_t *zone)

struct kmem_zone	*zone ?

> +{
> +	int	leaked = 0;
> +
> +	if (getenv("LIBXFS_LEAK_CHECK") && zone->allocated) {
> +		leaked = 1;
> +		printf("zone %s freed with %d items allocated\n",
> +					zone->zone_name, zone->allocated);
> +	}
> +	free(zone);
> +	return leaked;
> +}
> +
>  void *
>  kmem_zone_alloc(kmem_zone_t *zone, int flags)
>  {
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 1dcabdd..fdb2865 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1352,8 +1352,8 @@ struct cache_operations libxfs_bcache_operations = {
>   * Inode cache stubs.
>   */
>  
> +kmem_zone_t		*xfs_inode_zone;
>  extern kmem_zone_t	*xfs_ili_zone;
> -extern kmem_zone_t	*xfs_inode_zone;

/me wonders what's going on here?  Alphabetizing, I guess...

<shrug> meh, whatever :)

--D

>  
>  int
>  libxfs_iget(xfs_mount_t *mp, xfs_trans_t *tp, xfs_ino_t ino, uint lock_flags,
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 5f1ac9f..e08d1d1 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -4028,6 +4028,7 @@ main(
>  	if (xi.logdev && xi.logdev != xi.ddev)
>  		libxfs_device_close(xi.logdev);
>  	libxfs_device_close(xi.ddev);
> +	libxfs_destroy();
>  
>  	return 0;
>  }
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index b2dd91b..312a0d0 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -1082,6 +1082,7 @@ _("Note - stripe unit (%d) and width (%d) were copied from a backup superblock.\
>  	if (x.logdev && x.logdev != x.ddev)
>  		libxfs_device_close(x.logdev);
>  	libxfs_device_close(x.ddev);
> +	libxfs_destroy();
>  
>  	if (verbose)
>  		summary_report();
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/8] libxfs: manage xfs_buf_zone count within cache

[Eric Sandeen]

On 1/11/18 2:20 PM, Darrick J. Wong wrote:
> On Thu, Jan 11, 2018 at 01:37:36PM -0600, Eric Sandeen wrote:
>> Fake up the xfs_buf_zone allocated count a bit to account
>> for buffers freed to and allocated from cache.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
>>  libxfs/rdwr.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
>> index 474e5eb..c5ffd4d 100644
>> --- a/libxfs/rdwr.c
>> +++ b/libxfs/rdwr.c
>> @@ -665,6 +665,7 @@ __libxfs_getbufr(int blen)
>>  				free(bp->b_maps);
>>  			bp->b_maps = NULL;
>>  		}
>> +		xfs_buf_zone->allocated++;
> 
> I'm confused.  kmem_zone_zalloc increments zone->allocated and
> kmem_zone_free decrements it.  Now we're messing with ->allocated
> ourselves, which means that buffers that have ended up on
> xfs_buf_freelist are still allocated (according to the heap) but the
> zone accounting thinks the buffer is freed?
> 
> <goes away, reviews the next patch, comes back>
> 
> The next patch has xfs_buf_freelist frees the buffer directly and
> bypassing kmem_zone_free.  So I wonder, if you change the next patch to
> call kmem_zone_free and kill this patch, won't the accounting work out?
> 
> (xfs buf zone allocated == 0)
> 
> bp = _zone_alloc()
> xfs_buf_zone->allocated++
> 
> ...do stuff with buffer...
> 
> xfs_buf_relse(bp)
> list_add(bp, xfs_buf_freelist)
> 
> ...buffer still allocated, xfs_buf_zone->allocated == 1...
> 
> libxfs_bcache_free()
> _zone_free(bp)
> xfs_buf_zone->allocated--
> 
> ...exit program, and:
> xfs_buf_zone->allocated == 0
> 
> I think killing this patch and teaching the next one to kmem_zone_free
> the buffer works, right?  Or am I missing some subtlety here, in which
> case said subtlety needs comments.

Yes, it would work out.  This patch was trying to fake up actual use
by libxfs, because my main goal was to be able to find leaks in libxfs.
Counting it as still allocated when libxfs thinks it's put it away
seems to confuse that goal.

In the end, it's just where I want to draw the line for the accounting.

In this way I was hoping to keep the cache details out of that
accounting.  When libxfs says the buffer is done and puts the last ref,
it's considered to be freed by libxfs, whether or not the cache chooses
to hang on to it, discard it later, or whatever.

Maybe I could fake up a special libxfs_zone_free for the buffer zone which
hooks  into the cache mechanisms, so we do nothing but normal zone
alloc/free from the libxfs perspective?  <handwave>

-Eric

Re: [PATCH 6/8] libxfs: manage xfs_buf_zone count within cache

[Dave Chinner]

On Thu, Jan 11, 2018 at 02:48:33PM -0600, Eric Sandeen wrote:
> 
> 
> On 1/11/18 2:20 PM, Darrick J. Wong wrote:
> > On Thu, Jan 11, 2018 at 01:37:36PM -0600, Eric Sandeen wrote:
> >> Fake up the xfs_buf_zone allocated count a bit to account
> >> for buffers freed to and allocated from cache.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> >> ---
> >>  libxfs/rdwr.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> >> index 474e5eb..c5ffd4d 100644
> >> --- a/libxfs/rdwr.c
> >> +++ b/libxfs/rdwr.c
> >> @@ -665,6 +665,7 @@ __libxfs_getbufr(int blen)
> >>  				free(bp->b_maps);
> >>  			bp->b_maps = NULL;
> >>  		}
> >> +		xfs_buf_zone->allocated++;
> > 
> > I'm confused.  kmem_zone_zalloc increments zone->allocated and
> > kmem_zone_free decrements it.  Now we're messing with ->allocated
> > ourselves, which means that buffers that have ended up on
> > xfs_buf_freelist are still allocated (according to the heap) but the
> > zone accounting thinks the buffer is freed?
> > 
> > <goes away, reviews the next patch, comes back>
> > 
> > The next patch has xfs_buf_freelist frees the buffer directly and
> > bypassing kmem_zone_free.  So I wonder, if you change the next patch to
> > call kmem_zone_free and kill this patch, won't the accounting work out?
> > 
> > (xfs buf zone allocated == 0)
> > 
> > bp = _zone_alloc()
> > xfs_buf_zone->allocated++
> > 
> > ...do stuff with buffer...
> > 
> > xfs_buf_relse(bp)
> > list_add(bp, xfs_buf_freelist)
> > 
> > ...buffer still allocated, xfs_buf_zone->allocated == 1...
> > 
> > libxfs_bcache_free()
> > _zone_free(bp)
> > xfs_buf_zone->allocated--
> > 
> > ...exit program, and:
> > xfs_buf_zone->allocated == 0
> > 
> > I think killing this patch and teaching the next one to kmem_zone_free
> > the buffer works, right?  Or am I missing some subtlety here, in which
> > case said subtlety needs comments.
> 
> Yes, it would work out.  This patch was trying to fake up actual use
> by libxfs, because my main goal was to be able to find leaks in libxfs.
> Counting it as still allocated when libxfs thinks it's put it away
> seems to confuse that goal.
> 
> In the end, it's just where I want to draw the line for the accounting.
> 
> In this way I was hoping to keep the cache details out of that
> accounting.  When libxfs says the buffer is done and puts the last ref,
> it's considered to be freed by libxfs, whether or not the cache chooses
> to hang on to it, discard it later, or whatever.

It's not the job of the zone allocator to track whether something
is cached or not - it just tracks how many objects are allocated.

Indeed, the libxfs cache iitself keeps track of how many objects it
has remaining in it, and if you define CACHE_DEBUG it will issue a
warning if the cache is not empty after it has been purged and is
being torn down.  i.e. we already have debug mechanisms to tell if
we are leaking objects through the cache...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 8/8] libxfs: Catch non-empty zones on destroy

[Eric Sandeen]

On 1/11/18 2:29 PM, Darrick J. Wong wrote:
>> +kmem_zone_t		*xfs_inode_zone;
>>  extern kmem_zone_t	*xfs_ili_zone;
>> -extern kmem_zone_t	*xfs_inode_zone;
> /me wonders what's going on here?  Alphabetizing, I guess...
> 
> <shrug> meh, whatever :)

it's moving the definition of the inode zone variable into this
file which is directly related, rather than randomly plopped in init.c
and referring to it here with extern.

As for placement, I put the actual definition above an extern
reference to something else somewhere else, seemed to make sense
to "rank" them like that.

As for splitting the patch up, *shrug* ok.  Without
the libxfs_destroy() code none of this ever runs, but sure,
I can build it up then enable it separately.

-Eric

[PATCH 4/8 V2] libxfs: use a memory zone for transactions

[Eric Sandeen]

In addition to more closely matching the kernel, this will
help us catch any leaks from these allocations.

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

V2: remove pointless NULL assignments

diff --git a/libxfs/init.c b/libxfs/init.c
index 302f088..7bde8b7 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -384,6 +384,7 @@ manage_zones(int release)
 	extern kmem_zone_t	*xfs_da_state_zone;
 	extern kmem_zone_t	*xfs_btree_cur_zone;
 	extern kmem_zone_t	*xfs_bmap_free_item_zone;
+	extern kmem_zone_t	*xfs_trans_zone;
 	extern kmem_zone_t	*xfs_log_item_desc_zone;
 	extern void		xfs_dir_startup();
 
@@ -395,6 +396,7 @@ manage_zones(int release)
 		kmem_free(xfs_da_state_zone);
 		kmem_free(xfs_btree_cur_zone);
 		kmem_free(xfs_bmap_free_item_zone);
+		kmem_free(xfs_trans_zone);
 		kmem_free(xfs_log_item_desc_zone);
 		return;
 	}
@@ -413,6 +415,8 @@ manage_zones(int release)
 	xfs_bmap_free_item_zone = kmem_zone_init(
 			sizeof(struct xfs_extent_free_item),
 			"xfs_bmap_free_item");
+	xfs_trans_zone = kmem_zone_init(
+			sizeof(struct xfs_trans), "xfs_trans");
 	xfs_log_item_desc_zone = kmem_zone_init(
 			sizeof(struct xfs_log_item_desc), "xfs_log_item_desc");
 	xfs_dir_startup();
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 57ff3ea..a602eaa 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -36,6 +36,7 @@ static void xfs_trans_free_items(struct xfs_trans *tp);
  * Simple transaction interface
  */
 
+kmem_zone_t	*xfs_trans_zone;
 kmem_zone_t	*xfs_log_item_desc_zone;
 
 /*
@@ -143,6 +144,17 @@ libxfs_trans_roll(
 	return 0;
 }
 
+/*
+ * Free the transaction structure.  If there is more clean up
+ * to do when the structure is freed, add it here.
+ */
+static void
+xfs_trans_free(
+	struct xfs_trans	*tp)
+{
+	kmem_zone_free(xfs_trans_zone, tp);
+}
+
 int
 libxfs_trans_alloc(
 	struct xfs_mount	*mp,
@@ -166,11 +178,8 @@ libxfs_trans_alloc(
 			return -ENOSPC;
 	}
 
-	if ((ptr = calloc(sizeof(xfs_trans_t), 1)) == NULL) {
-		fprintf(stderr, _("%s: xact calloc failed (%d bytes): %s\n"),
-			progname, (int)sizeof(xfs_trans_t), strerror(errno));
-		exit(1);
-	}
+	ptr = kmem_zone_zalloc(xfs_trans_zone,
+		(flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
 	ptr->t_mountp = mp;
 	ptr->t_blk_res = blocks;
 	INIT_LIST_HEAD(&ptr->t_items);
@@ -212,8 +221,7 @@ libxfs_trans_cancel(
 #endif
 	if (tp != NULL) {
 		xfs_trans_free_items(tp);
-		free(tp);
-		tp = NULL;
+		xfs_trans_free(tp);
 	}
 #ifdef XACT_DEBUG
 	fprintf(stderr, "## cancelled transaction %p\n", otp);
@@ -867,8 +875,7 @@ libxfs_trans_commit(
 		fprintf(stderr, "committed clean transaction %p\n", tp);
 #endif
 		xfs_trans_free_items(tp);
-		free(tp);
-		tp = NULL;
+		xfs_trans_free(tp);
 		return 0;
 	}
 
@@ -891,7 +898,6 @@ libxfs_trans_commit(
 	trans_committed(tp);
 
 	/* That's it for the transaction structure.  Free it. */
-	free(tp);
-	tp = NULL;
+	xfs_trans_free(tp);
 	return 0;
 }

Re: [PATCH 4/8 V2] libxfs: use a memory zone for transactions

[Darrick J. Wong]

On Thu, Jan 25, 2018 at 01:01:19PM -0600, Eric Sandeen wrote:
> In addition to more closely matching the kernel, this will
> help us catch any leaks from these allocations.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

> ---
> 
> V2: remove pointless NULL assignments
> 
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 302f088..7bde8b7 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -384,6 +384,7 @@ manage_zones(int release)
>  	extern kmem_zone_t	*xfs_da_state_zone;
>  	extern kmem_zone_t	*xfs_btree_cur_zone;
>  	extern kmem_zone_t	*xfs_bmap_free_item_zone;
> +	extern kmem_zone_t	*xfs_trans_zone;
>  	extern kmem_zone_t	*xfs_log_item_desc_zone;
>  	extern void		xfs_dir_startup();
>  
> @@ -395,6 +396,7 @@ manage_zones(int release)
>  		kmem_free(xfs_da_state_zone);
>  		kmem_free(xfs_btree_cur_zone);
>  		kmem_free(xfs_bmap_free_item_zone);
> +		kmem_free(xfs_trans_zone);
>  		kmem_free(xfs_log_item_desc_zone);
>  		return;
>  	}
> @@ -413,6 +415,8 @@ manage_zones(int release)
>  	xfs_bmap_free_item_zone = kmem_zone_init(
>  			sizeof(struct xfs_extent_free_item),
>  			"xfs_bmap_free_item");
> +	xfs_trans_zone = kmem_zone_init(
> +			sizeof(struct xfs_trans), "xfs_trans");
>  	xfs_log_item_desc_zone = kmem_zone_init(
>  			sizeof(struct xfs_log_item_desc), "xfs_log_item_desc");
>  	xfs_dir_startup();
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 57ff3ea..a602eaa 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -36,6 +36,7 @@ static void xfs_trans_free_items(struct xfs_trans *tp);
>   * Simple transaction interface
>   */
>  
> +kmem_zone_t	*xfs_trans_zone;
>  kmem_zone_t	*xfs_log_item_desc_zone;
>  
>  /*
> @@ -143,6 +144,17 @@ libxfs_trans_roll(
>  	return 0;
>  }
>  
> +/*
> + * Free the transaction structure.  If there is more clean up
> + * to do when the structure is freed, add it here.
> + */
> +static void
> +xfs_trans_free(
> +	struct xfs_trans	*tp)
> +{
> +	kmem_zone_free(xfs_trans_zone, tp);
> +}
> +
>  int
>  libxfs_trans_alloc(
>  	struct xfs_mount	*mp,
> @@ -166,11 +178,8 @@ libxfs_trans_alloc(
>  			return -ENOSPC;
>  	}
>  
> -	if ((ptr = calloc(sizeof(xfs_trans_t), 1)) == NULL) {
> -		fprintf(stderr, _("%s: xact calloc failed (%d bytes): %s\n"),
> -			progname, (int)sizeof(xfs_trans_t), strerror(errno));
> -		exit(1);
> -	}
> +	ptr = kmem_zone_zalloc(xfs_trans_zone,
> +		(flags & XFS_TRANS_NOFS) ? KM_NOFS : KM_SLEEP);
>  	ptr->t_mountp = mp;
>  	ptr->t_blk_res = blocks;
>  	INIT_LIST_HEAD(&ptr->t_items);
> @@ -212,8 +221,7 @@ libxfs_trans_cancel(
>  #endif
>  	if (tp != NULL) {
>  		xfs_trans_free_items(tp);
> -		free(tp);
> -		tp = NULL;
> +		xfs_trans_free(tp);
>  	}
>  #ifdef XACT_DEBUG
>  	fprintf(stderr, "## cancelled transaction %p\n", otp);
> @@ -867,8 +875,7 @@ libxfs_trans_commit(
>  		fprintf(stderr, "committed clean transaction %p\n", tp);
>  #endif
>  		xfs_trans_free_items(tp);
> -		free(tp);
> -		tp = NULL;
> +		xfs_trans_free(tp);
>  		return 0;
>  	}
>  
> @@ -891,7 +898,6 @@ libxfs_trans_commit(
>  	trans_committed(tp);
>  
>  	/* That's it for the transaction structure.  Free it. */
> -	free(tp);
> -	tp = NULL;
> +	xfs_trans_free(tp);
>  	return 0;
>  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html