[PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START

[PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START

[Chandra Seetharaman]

Remove the definition and usage of the macro XFS_BUF_SET_START.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/xfs_buf_item.c      |    1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 536ede5..e256289 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -274,7 +274,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_SET_FSPRIVATE(bp, val)		((bp)->b_fspriv = (void*)(val))
 #define XFS_BUF_FSPRIVATE2(bp, type)		((type)(bp)->b_fspriv2)
 #define XFS_BUF_SET_FSPRIVATE2(bp, val)		((bp)->b_fspriv2 = (void*)(val))
-#define XFS_BUF_SET_START(bp)			do { } while (0)
 
 #define XFS_BUF_PTR(bp)			(xfs_caddr_t)((bp)->b_addr)
 #define XFS_BUF_SET_PTR(bp, val, cnt)	xfs_buf_associate_memory(bp, val, cnt)
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 299f614..dc7b096 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -1003,7 +1003,6 @@ xfs_buf_iodone_callbacks(
 		if (!XFS_BUF_ISSTALE(bp)) {
 			XFS_BUF_DELAYWRITE(bp);
 			XFS_BUF_DONE(bp);
-			XFS_BUF_SET_START(bp);
 		}
 		ASSERT(XFS_BUF_IODONE_FUNC(bp));
 		trace_xfs_buf_item_iodone_async(bp, _RET_IP_);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START

[Christoph Hellwig]

On Fri, Jul 15, 2011 at 06:21:41PM -0700, Chandra Seetharaman wrote:
> Remove the definition and usage of the macro XFS_BUF_SET_START.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 00/12] Remove number of macros from xfs_buf.h

[Chandra Seetharaman]

Hello All,

Here are few patchset to remove some of the macro definitions from 
fs/xfs/linux-2.6/xfs_buf.h.

Please review and comment.

Thanks & Regards,

chandra

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 01/12] xfs: Remove the macro XFS_BUF_BFLAGS

[Chandra Seetharaman]

Remove the definition of the macro XFS_BUF_BFLAGS and its usage.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.c |    2 +-
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/xfs_trans_buf.c     |    2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index b2b4119..969fd15 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -1094,7 +1094,7 @@ STATIC int
 xfs_bioerror_relse(
 	struct xfs_buf	*bp)
 {
-	int64_t		fl = XFS_BUF_BFLAGS(bp);
+	int64_t		fl = bp->b_flags;
 	/*
 	 * No need to wait until the buffer is unpinned.
 	 * We aren't flushing it.
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 6a83b46..6b6c25f 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -232,7 +232,6 @@ extern void xfs_buf_terminate(void);
 	({ char __b[BDEVNAME_SIZE]; bdevname((target)->bt_bdev, __b); __b; })
 
 
-#define XFS_BUF_BFLAGS(bp)	((bp)->b_flags)
 #define XFS_BUF_ZEROFLAGS(bp) \
 	((bp)->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC|XBF_DELWRI| \
 			    XBF_SYNCIO|XBF_FUA|XBF_FLUSH))
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index 15584fc..1bc04d4 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -430,7 +430,7 @@ shutdown_abort:
 	if (XFS_BUF_ISSTALE(bp) && XFS_BUF_ISDELAYWRITE(bp))
 		xfs_notice(mp, "about to pop assert, bp == 0x%p", bp);
 #endif
-	ASSERT((XFS_BUF_BFLAGS(bp) & (XBF_STALE|XBF_DELWRI)) !=
+	ASSERT((bp->b_flags & (XBF_STALE|XBF_DELWRI)) !=
 				     (XBF_STALE|XBF_DELWRI));
 
 	trace_xfs_trans_read_buf_shut(bp, _RET_IP_);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 02/12] xfs: Remove the macro XFS_BUF_ZEROFLAGS

[Chandra Seetharaman]

Remove the definition and usage of the macro XFS_BUF_ZEROFLAGS.

In the specific contexts, not all flags need to be cleared. Hence the
simplification.

Rationale for xfs_log.c:
from hch: XBF_READ and XBF_DELWRI will never be set here, XBF_WRITE
XBF_ASYNC, and XBF_SYNCIO are set just below.
Hence clearing only XBF_FUA and XBF_FLUSH would suffice.

Rationale for xfs_log_recover.c
from hch: XBF_READ, XBF_WRITE, XBF_ASYNC and XBF_DELWRI already get dealt with
in xfs_bwrite a few lines down.
Hence clearing only XBF_SYNCIO, XBF_FUA and XBF_FLUSH would suffice.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/linux-2.6/xfs_buf.h |    5 -----
 fs/xfs/xfs_log.c           |    4 ++--
 fs/xfs/xfs_log_recover.c   |    2 +-
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 6b6c25f..d7df7b6 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -231,11 +231,6 @@ extern void xfs_buf_terminate(void);
 #define xfs_buf_target_name(target)	\
 	({ char __b[BDEVNAME_SIZE]; bdevname((target)->bt_bdev, __b); __b; })
 
-
-#define XFS_BUF_ZEROFLAGS(bp) \
-	((bp)->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC|XBF_DELWRI| \
-			    XBF_SYNCIO|XBF_FUA|XBF_FLUSH))
-
 void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_STALE(bp)	xfs_buf_stale(bp);
 #define XFS_BUF_UNSTALE(bp)	((bp)->b_flags &= ~XBF_STALE)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 06ff843..5319a95 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1354,7 +1354,7 @@ xlog_sync(xlog_t		*log,
 	}
 	XFS_BUF_SET_COUNT(bp, count);
 	bp->b_fspriv = iclog;
-	XFS_BUF_ZEROFLAGS(bp);
+	bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
 	XFS_BUF_BUSY(bp);
 	XFS_BUF_ASYNC(bp);
 	bp->b_flags |= XBF_SYNCIO;
@@ -1401,7 +1401,7 @@ xlog_sync(xlog_t		*log,
 		XFS_BUF_SET_PTR(bp, (xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
 					    (__psint_t)count), split);
 		bp->b_fspriv = iclog;
-		XFS_BUF_ZEROFLAGS(bp);
+		bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
 		XFS_BUF_BUSY(bp);
 		XFS_BUF_ASYNC(bp);
 		bp->b_flags |= XBF_SYNCIO;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 8fe4206..3913b2f 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -265,7 +265,7 @@ xlog_bwrite(
 	ASSERT(BBTOB(nbblks) <= XFS_BUF_SIZE(bp));
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
-	XFS_BUF_ZEROFLAGS(bp);
+	bp->b_flags &= ~(XBF_SYNCIO|XBF_FUA|XBF_FLUSH);
 	XFS_BUF_BUSY(bp);
 	XFS_BUF_HOLD(bp);
 	xfs_buf_lock(bp);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family

[Chandra Seetharaman]

Remove the definitions and usage of the macros XFS_BUF_ERROR,
XFS_BUF_GETERROR and XFS_BUF_ISERROR.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/linux-2.6/xfs_buf.c |    6 +++---
 fs/xfs/linux-2.6/xfs_buf.h |    4 ----
 fs/xfs/quota/xfs_dquot.c   |    3 ++-
 fs/xfs/xfs_alloc.c         |    7 +++----
 fs/xfs/xfs_attr.c          |    3 +--
 fs/xfs/xfs_btree.c         |   17 ++++++-----------
 fs/xfs/xfs_buf_item.c      |    4 ++--
 fs/xfs/xfs_da_btree.c      |    2 +-
 fs/xfs/xfs_ialloc.c        |    5 ++---
 fs/xfs/xfs_inode.c         |    2 +-
 fs/xfs/xfs_log.c           |    4 ++--
 fs/xfs/xfs_log_recover.c   |   12 +++++-------
 fs/xfs/xfs_rtalloc.c       |    2 +-
 fs/xfs/xfs_rw.c            |    6 +++---
 fs/xfs/xfs_trans_buf.c     |   15 +++++++--------
 fs/xfs/xfs_vnodeops.c      |    4 ++--
 16 files changed, 41 insertions(+), 55 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 969fd15..704418a 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -596,7 +596,7 @@ _xfs_buf_read(
 	bp->b_flags |= flags & (XBF_READ | XBF_ASYNC | XBF_READ_AHEAD);
 
 	status = xfs_buf_iorequest(bp);
-	if (status || XFS_BUF_ISERROR(bp) || (flags & XBF_ASYNC))
+	if (status || bp->b_error || (flags & XBF_ASYNC))
 		return status;
 	return xfs_buf_iowait(bp);
 }
@@ -1069,7 +1069,7 @@ xfs_bioerror(
 	/*
 	 * No need to wait until the buffer is unpinned, we aren't flushing it.
 	 */
-	XFS_BUF_ERROR(bp, EIO);
+	xfs_buf_ioerror(bp, EIO);
 
 	/*
 	 * We're calling xfs_buf_ioend, so delete XBF_DONE flag.
@@ -1115,7 +1115,7 @@ xfs_bioerror_relse(
 		 * There's no reason to mark error for
 		 * ASYNC buffers.
 		 */
-		XFS_BUF_ERROR(bp, EIO);
+		xfs_buf_ioerror(bp, EIO);
 		XFS_BUF_FINISH_IOWAIT(bp);
 	} else {
 		xfs_buf_relse(bp);
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index d7df7b6..c14297c 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -245,10 +245,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNDELAYWRITE(bp)	xfs_buf_delwri_dequeue(bp)
 #define XFS_BUF_ISDELAYWRITE(bp)	((bp)->b_flags & XBF_DELWRI)
 
-#define XFS_BUF_ERROR(bp,no)	xfs_buf_ioerror(bp,no)
-#define XFS_BUF_GETERROR(bp)	xfs_buf_geterror(bp)
-#define XFS_BUF_ISERROR(bp)	(xfs_buf_geterror(bp) ? 1 : 0)
-
 #define XFS_BUF_DONE(bp)	((bp)->b_flags |= XBF_DONE)
 #define XFS_BUF_UNDONE(bp)	((bp)->b_flags &= ~XBF_DONE)
 #define XFS_BUF_ISDONE(bp)	((bp)->b_flags & XBF_DONE)
diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
index 837f311..e7e35fb 100644
--- a/fs/xfs/quota/xfs_dquot.c
+++ b/fs/xfs/quota/xfs_dquot.c
@@ -403,7 +403,8 @@ xfs_qm_dqalloc(
 			       dqp->q_blkno,
 			       mp->m_quotainfo->qi_dqchunklen,
 			       0);
-	if (!bp || (error = XFS_BUF_GETERROR(bp)))
+	error = xfs_buf_geterror(bp);
+	if (error)
 		goto error1;
 	/*
 	 * Make a chunk of dquots out of this buffer and log
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 1e00b3e..bdd9cb5 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -451,8 +451,7 @@ xfs_alloc_read_agfl(
 			XFS_FSS_TO_BB(mp, 1), 0, &bp);
 	if (error)
 		return error;
-	ASSERT(bp);
-	ASSERT(!XFS_BUF_GETERROR(bp));
+	ASSERT(!xfs_buf_geterror(bp));
 	XFS_BUF_SET_VTYPE_REF(bp, B_FS_AGFL, XFS_AGFL_REF);
 	*bpp = bp;
 	return 0;
@@ -2116,7 +2115,7 @@ xfs_read_agf(
 	if (!*bpp)
 		return 0;
 
-	ASSERT(!XFS_BUF_GETERROR(*bpp));
+	ASSERT(!(*bpp)->b_error);
 	agf = XFS_BUF_TO_AGF(*bpp);
 
 	/*
@@ -2168,7 +2167,7 @@ xfs_alloc_read_agf(
 		return error;
 	if (!*bpp)
 		return 0;
-	ASSERT(!XFS_BUF_GETERROR(*bpp));
+	ASSERT(!(*bpp)->b_error);
 
 	agf = XFS_BUF_TO_AGF(*bpp);
 	pag = xfs_perag_get(mp, agno);
diff --git a/fs/xfs/xfs_attr.c b/fs/xfs/xfs_attr.c
index cbae424..160bcdc 100644
--- a/fs/xfs/xfs_attr.c
+++ b/fs/xfs/xfs_attr.c
@@ -2121,8 +2121,7 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
 
 		bp = xfs_buf_get(mp->m_ddev_targp, dblkno, blkcnt,
 				 XBF_LOCK | XBF_DONT_BLOCK);
-		ASSERT(bp);
-		ASSERT(!XFS_BUF_GETERROR(bp));
+		ASSERT(!xfs_buf_geterror(bp));
 
 		tmp = (valuelen < XFS_BUF_SIZE(bp)) ? valuelen :
 							XFS_BUF_SIZE(bp);
diff --git a/fs/xfs/xfs_btree.c b/fs/xfs/xfs_btree.c
index cabf4b5..2b9fd38 100644
--- a/fs/xfs/xfs_btree.c
+++ b/fs/xfs/xfs_btree.c
@@ -275,8 +275,7 @@ xfs_btree_dup_cursor(
 				return error;
 			}
 			new->bc_bufs[i] = bp;
-			ASSERT(bp);
-			ASSERT(!XFS_BUF_GETERROR(bp));
+			ASSERT(!xfs_buf_geterror(bp));
 		} else
 			new->bc_bufs[i] = NULL;
 	}
@@ -467,8 +466,7 @@ xfs_btree_get_bufl(
 	ASSERT(fsbno != NULLFSBLOCK);
 	d = XFS_FSB_TO_DADDR(mp, fsbno);
 	bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, lock);
-	ASSERT(bp);
-	ASSERT(!XFS_BUF_GETERROR(bp));
+	ASSERT(!xfs_buf_geterror(bp));
 	return bp;
 }
 
@@ -491,8 +489,7 @@ xfs_btree_get_bufs(
 	ASSERT(agbno != NULLAGBLOCK);
 	d = XFS_AGB_TO_DADDR(mp, agno, agbno);
 	bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, lock);
-	ASSERT(bp);
-	ASSERT(!XFS_BUF_GETERROR(bp));
+	ASSERT(!xfs_buf_geterror(bp));
 	return bp;
 }
 
@@ -632,7 +629,7 @@ xfs_btree_read_bufl(
 			mp->m_bsize, lock, &bp))) {
 		return error;
 	}
-	ASSERT(!bp || !XFS_BUF_GETERROR(bp));
+	ASSERT(!xfs_buf_geterror(bp));
 	if (bp)
 		XFS_BUF_SET_VTYPE_REF(bp, B_FS_MAP, refval);
 	*bpp = bp;
@@ -973,8 +970,7 @@ xfs_btree_get_buf_block(
 	*bpp = xfs_trans_get_buf(cur->bc_tp, mp->m_ddev_targp, d,
 				 mp->m_bsize, flags);
 
-	ASSERT(*bpp);
-	ASSERT(!XFS_BUF_GETERROR(*bpp));
+	ASSERT(!xfs_buf_geterror(*bpp));
 
 	*block = XFS_BUF_TO_BLOCK(*bpp);
 	return 0;
@@ -1006,8 +1002,7 @@ xfs_btree_read_buf_block(
 	if (error)
 		return error;
 
-	ASSERT(*bpp != NULL);
-	ASSERT(!XFS_BUF_GETERROR(*bpp));
+	ASSERT(!xfs_buf_geterror(*bpp));
 
 	xfs_btree_set_refs(cur, *bpp);
 	*block = XFS_BUF_TO_BLOCK(*bpp);
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 8849291..38417ab 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -960,7 +960,7 @@ xfs_buf_iodone_callbacks(
 	static ulong		lasttime;
 	static xfs_buftarg_t	*lasttarg;
 
-	if (likely(!XFS_BUF_GETERROR(bp)))
+	if (likely(!xfs_buf_geterror(bp)))
 		goto do_callbacks;
 
 	/*
@@ -991,7 +991,7 @@ xfs_buf_iodone_callbacks(
 	 * around.
 	 */
 	if (XFS_BUF_ISASYNC(bp)) {
-		XFS_BUF_ERROR(bp, 0); /* errno of 0 unsets the flag */
+		xfs_buf_ioerror(bp, 0); /* errno of 0 unsets the flag */
 
 		if (!XFS_BUF_ISSTALE(bp)) {
 			XFS_BUF_DELAYWRITE(bp);
diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
index 2925726..5d9290d 100644
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -2040,7 +2040,7 @@ xfs_da_do_buf(
 		case 0:
 			bp = xfs_trans_get_buf(trans, mp->m_ddev_targp,
 				mappedbno, nmapped, 0);
-			error = bp ? XFS_BUF_GETERROR(bp) : XFS_ERROR(EIO);
+			error = bp ? bp->b_error : XFS_ERROR(EIO);
 			break;
 		case 1:
 		case 2:
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index dd5628b..9f24ec2 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -202,8 +202,7 @@ xfs_ialloc_inode_init(
 		fbuf = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
 					 mp->m_bsize * blks_per_cluster,
 					 XBF_LOCK);
-		ASSERT(fbuf);
-		ASSERT(!XFS_BUF_GETERROR(fbuf));
+		ASSERT(!xfs_buf_geterror(fbuf));
 
 		/*
 		 * Initialize all inodes in this buffer and then log them.
@@ -1486,7 +1485,7 @@ xfs_read_agi(
 	if (error)
 		return error;
 
-	ASSERT(*bpp && !XFS_BUF_GETERROR(*bpp));
+	ASSERT(!xfs_buf_geterror(*bpp));
 	agi = XFS_BUF_TO_AGI(*bpp);
 
 	/*
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 3cc21dd..bdb47b2 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2473,7 +2473,7 @@ cluster_corrupt_out:
 		if (bp->b_iodone) {
 			XFS_BUF_UNDONE(bp);
 			XFS_BUF_STALE(bp);
-			XFS_BUF_ERROR(bp,EIO);
+			xfs_buf_ioerror(bp, EIO);
 			xfs_buf_ioend(bp, 0);
 		} else {
 			XFS_BUF_STALE(bp);
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 5319a95..0a7930a 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -878,7 +878,7 @@ xlog_iodone(xfs_buf_t *bp)
 	/*
 	 * Race to shutdown the filesystem if we see an error.
 	 */
-	if (XFS_TEST_ERROR((XFS_BUF_GETERROR(bp)), l->l_mp,
+	if (XFS_TEST_ERROR((xfs_buf_geterror(bp)), l->l_mp,
 			XFS_ERRTAG_IODONE_IOERR, XFS_RANDOM_IODONE_IOERR)) {
 		xfs_ioerror_alert("xlog_iodone", l->l_mp, bp, XFS_BUF_ADDR(bp));
 		XFS_BUF_STALE(bp);
@@ -1248,7 +1248,7 @@ xlog_bdstrat(
 	struct xlog_in_core	*iclog = bp->b_fspriv;
 
 	if (iclog->ic_state & XLOG_STATE_IOERROR) {
-		XFS_BUF_ERROR(bp, EIO);
+		xfs_buf_ioerror(bp, EIO);
 		XFS_BUF_STALE(bp);
 		xfs_buf_ioend(bp, 0);
 		/*
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 3913b2f..2eee6e2 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -360,7 +360,7 @@ STATIC void
 xlog_recover_iodone(
 	struct xfs_buf	*bp)
 {
-	if (XFS_BUF_GETERROR(bp)) {
+	if (bp->b_error) {
 		/*
 		 * We're not going to bother about retrying
 		 * this during recovery. One strike!
@@ -2135,15 +2135,14 @@ xlog_recover_buffer_pass2(
 
 	bp = xfs_buf_read(mp->m_ddev_targp, buf_f->blf_blkno, buf_f->blf_len,
 			  buf_flags);
-	if (XFS_BUF_ISERROR(bp)) {
+	error = bp->b_error;
+	if (error) {
 		xfs_ioerror_alert("xlog_recover_do..(read#1)", mp,
 				  bp, buf_f->blf_blkno);
-		error = XFS_BUF_GETERROR(bp);
 		xfs_buf_relse(bp);
 		return error;
 	}
 
-	error = 0;
 	if (buf_f->blf_flags & XFS_BLF_INODE_BUF) {
 		error = xlog_recover_do_inode_buffer(mp, item, bp, buf_f);
 	} else if (buf_f->blf_flags &
@@ -2227,14 +2226,13 @@ xlog_recover_inode_pass2(
 
 	bp = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len,
 			  XBF_LOCK);
-	if (XFS_BUF_ISERROR(bp)) {
+	error = bp->b_error;
+	if (error) {
 		xfs_ioerror_alert("xlog_recover_do..(read#2)", mp,
 				  bp, in_f->ilf_blkno);
-		error = XFS_BUF_GETERROR(bp);
 		xfs_buf_relse(bp);
 		goto error;
 	}
-	error = 0;
 	ASSERT(in_f->ilf_fields & XFS_ILOG_CORE);
 	dip = (xfs_dinode_t *)xfs_buf_offset(bp, in_f->ilf_boffset);
 
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 8f76fdf..cb8132c 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -883,7 +883,7 @@ xfs_rtbuf_get(
 	if (error) {
 		return error;
 	}
-	ASSERT(bp && !XFS_BUF_GETERROR(bp));
+	ASSERT(!xfs_buf_geterror(bp));
 	*bpp = bp;
 	return 0;
 }
diff --git a/fs/xfs/xfs_rw.c b/fs/xfs/xfs_rw.c
index d6d6fdf..d1f76f8 100644
--- a/fs/xfs/xfs_rw.c
+++ b/fs/xfs/xfs_rw.c
@@ -106,7 +106,7 @@ xfs_ioerror_alert(
 		 "       (\"%s\") error %d buf count %zd",
 		XFS_BUFTARG_NAME(XFS_BUF_TARGET(bp)),
 		(__uint64_t)blkno, func,
-		XFS_BUF_GETERROR(bp), XFS_BUF_COUNT(bp));
+		bp->b_error, XFS_BUF_COUNT(bp));
 }
 
 /*
@@ -137,8 +137,8 @@ xfs_read_buf(
 	bp = xfs_buf_read(target, blkno, len, flags);
 	if (!bp)
 		return XFS_ERROR(EIO);
-	error = XFS_BUF_GETERROR(bp);
-	if (bp && !error && !XFS_FORCED_SHUTDOWN(mp)) {
+	error = bp->b_error;
+	if (!error && !XFS_FORCED_SHUTDOWN(mp)) {
 		*bpp = bp;
 	} else {
 		*bpp = NULL;
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index 1bc04d4..f9f1bf6 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -194,7 +194,7 @@ xfs_trans_get_buf(xfs_trans_t	*tp,
 		return NULL;
 	}
 
-	ASSERT(!XFS_BUF_GETERROR(bp));
+	ASSERT(!bp->b_error);
 
 	_xfs_trans_bjoin(tp, bp, 1);
 	trace_xfs_trans_get_buf(bp->b_fspriv);
@@ -293,10 +293,10 @@ xfs_trans_read_buf(
 			return (flags & XBF_TRYLOCK) ?
 					EAGAIN : XFS_ERROR(ENOMEM);
 
-		if (XFS_BUF_GETERROR(bp) != 0) {
+		if (bp->b_error) {
+			error = bp->b_error;
 			xfs_ioerror_alert("xfs_trans_read_buf", mp,
 					  bp, blkno);
-			error = XFS_BUF_GETERROR(bp);
 			xfs_buf_relse(bp);
 			return error;
 		}
@@ -330,7 +330,7 @@ xfs_trans_read_buf(
 		ASSERT(xfs_buf_islocked(bp));
 		ASSERT(bp->b_transp == tp);
 		ASSERT(bp->b_fspriv != NULL);
-		ASSERT((XFS_BUF_ISERROR(bp)) == 0);
+		ASSERT(!bp->b_error);
 		if (!(XFS_BUF_ISDONE(bp))) {
 			trace_xfs_trans_read_buf_io(bp, _RET_IP_);
 			ASSERT(!XFS_BUF_ISASYNC(bp));
@@ -386,10 +386,9 @@ xfs_trans_read_buf(
 		return (flags & XBF_TRYLOCK) ?
 					0 : XFS_ERROR(ENOMEM);
 	}
-	if (XFS_BUF_GETERROR(bp) != 0) {
-	    XFS_BUF_SUPER_STALE(bp);
-		error = XFS_BUF_GETERROR(bp);
-
+	if (bp->b_error) {
+		error = bp->b_error;
+		XFS_BUF_SUPER_STALE(bp);
 		xfs_ioerror_alert("xfs_trans_read_buf", mp,
 				  bp, blkno);
 		if (tp->t_flags & XFS_TRANS_DIRTY)
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 88d1214..97daa35 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -83,7 +83,7 @@ xfs_readlink_bmap(
 
 		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
 				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);
-		error = XFS_BUF_GETERROR(bp);
+		error = bp->b_error;
 		if (error) {
 			xfs_ioerror_alert("xfs_readlink",
 				  ip->i_mount, bp, XFS_BUF_ADDR(bp));
@@ -1648,7 +1648,7 @@ xfs_symlink(
 			byte_cnt = XFS_FSB_TO_B(mp, mval[n].br_blockcount);
 			bp = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
 					       BTOBB(byte_cnt), 0);
-			ASSERT(bp && !XFS_BUF_GETERROR(bp));
+			ASSERT(!xfs_buf_geterror(bp));
 			if (pathlen < byte_cnt) {
 				byte_cnt = pathlen;
 			}
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 04/12] xfs: Remove macro XFS_BUF_BUSY and family

[Chandra Seetharaman]

Remove the definitions and uses of the macros XFS_BUF_BUSY,
XFS_BUF_UNBUSY, and XFS_BUF_ISBUSY.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.c |    1 -
 fs/xfs/linux-2.6/xfs_buf.h |    4 ----
 fs/xfs/quota/xfs_dquot.c   |    4 ----
 fs/xfs/xfs_buf_item.c      |    2 --
 fs/xfs/xfs_log.c           |    4 ----
 fs/xfs/xfs_log_recover.c   |    2 --
 fs/xfs/xfs_trans_buf.c     |    9 ---------
 7 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 704418a..ae2c2e7 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -679,7 +679,6 @@ xfs_buf_read_uncached(
 	/* set up the buffer for a read IO */
 	XFS_BUF_SET_ADDR(bp, daddr);
 	XFS_BUF_READ(bp);
-	XFS_BUF_BUSY(bp);
 
 	xfsbdstrat(mp, bp);
 	error = xfs_buf_iowait(bp);
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index c14297c..ce5c7e2 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -249,10 +249,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNDONE(bp)	((bp)->b_flags &= ~XBF_DONE)
 #define XFS_BUF_ISDONE(bp)	((bp)->b_flags & XBF_DONE)
 
-#define XFS_BUF_BUSY(bp)	do { } while (0)
-#define XFS_BUF_UNBUSY(bp)	do { } while (0)
-#define XFS_BUF_ISBUSY(bp)	(1)
-
 #define XFS_BUF_ASYNC(bp)	((bp)->b_flags |= XBF_ASYNC)
 #define XFS_BUF_UNASYNC(bp)	((bp)->b_flags &= ~XBF_ASYNC)
 #define XFS_BUF_ISASYNC(bp)	((bp)->b_flags & XBF_ASYNC)
diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
index e7e35fb..772ae2a 100644
--- a/fs/xfs/quota/xfs_dquot.c
+++ b/fs/xfs/quota/xfs_dquot.c
@@ -318,7 +318,6 @@ xfs_qm_init_dquot_blk(
 	int		curid, i;
 
 	ASSERT(tp);
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(xfs_buf_islocked(bp));
 
 	d = (xfs_dqblk_t *)XFS_BUF_PTR(bp);
@@ -535,7 +534,6 @@ xfs_qm_dqtobp(
 			return XFS_ERROR(error);
 	}
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(xfs_buf_islocked(bp));
 
 	/*
@@ -554,7 +552,6 @@ xfs_qm_dqtobp(
 			xfs_trans_brelse(tp, bp);
 			return XFS_ERROR(EIO);
 		}
-		XFS_BUF_BUSY(bp); /* We dirtied this */
 	}
 
 	*O_bpp = bp;
@@ -623,7 +620,6 @@ xfs_qm_dqread(
 	 * this particular dquot was repaired. We still aren't afraid to
 	 * brelse it because we have the changes incore.
 	 */
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(xfs_buf_islocked(bp));
 	xfs_trans_brelse(tp, bp);
 
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 38417ab..9e9b4a7 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -371,7 +371,6 @@ xfs_buf_item_pin(
 {
 	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
 
-	ASSERT(XFS_BUF_ISBUSY(bip->bli_buf));
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
 	ASSERT((bip->bli_flags & XFS_BLI_LOGGED) ||
 	       (bip->bli_flags & XFS_BLI_STALE));
@@ -895,7 +894,6 @@ xfs_buf_attach_iodone(
 {
 	xfs_log_item_t	*head_lip;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(xfs_buf_islocked(bp));
 
 	lip->li_cb = cb;
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 0a7930a..5805a3a 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1051,7 +1051,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	if (!bp)
 		goto out_free_log;
 	bp->b_iodone = xlog_iodone;
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(xfs_buf_islocked(bp));
 	log->l_xbuf = bp;
 
@@ -1108,7 +1107,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 		iclog->ic_callback_tail = &(iclog->ic_callback);
 		iclog->ic_datap = (char *)iclog->ic_data + log->l_iclog_hsize;
 
-		ASSERT(XFS_BUF_ISBUSY(iclog->ic_bp));
 		ASSERT(xfs_buf_islocked(iclog->ic_bp));
 		init_waitqueue_head(&iclog->ic_force_wait);
 		init_waitqueue_head(&iclog->ic_write_wait);
@@ -1355,7 +1353,6 @@ xlog_sync(xlog_t		*log,
 	XFS_BUF_SET_COUNT(bp, count);
 	bp->b_fspriv = iclog;
 	bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
-	XFS_BUF_BUSY(bp);
 	XFS_BUF_ASYNC(bp);
 	bp->b_flags |= XBF_SYNCIO;
 
@@ -1402,7 +1399,6 @@ xlog_sync(xlog_t		*log,
 					    (__psint_t)count), split);
 		bp->b_fspriv = iclog;
 		bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
-		XFS_BUF_BUSY(bp);
 		XFS_BUF_ASYNC(bp);
 		bp->b_flags |= XBF_SYNCIO;
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 2eee6e2..f774713 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -178,7 +178,6 @@ xlog_bread_noalign(
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
 	XFS_BUF_READ(bp);
-	XFS_BUF_BUSY(bp);
 	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
 	XFS_BUF_SET_TARGET(bp, log->l_mp->m_logdev_targp);
 
@@ -266,7 +265,6 @@ xlog_bwrite(
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
 	bp->b_flags &= ~(XBF_SYNCIO|XBF_FUA|XBF_FLUSH);
-	XFS_BUF_BUSY(bp);
 	XFS_BUF_HOLD(bp);
 	xfs_buf_lock(bp);
 	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index f9f1bf6..7dd62e2 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -80,7 +80,6 @@ _xfs_trans_bjoin(
 {
 	struct xfs_buf_log_item	*bip;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == NULL);
 
 	/*
@@ -580,7 +579,6 @@ xfs_trans_bhold(xfs_trans_t	*tp,
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
@@ -601,7 +599,6 @@ xfs_trans_bhold_release(xfs_trans_t	*tp,
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
@@ -630,7 +627,6 @@ xfs_trans_log_buf(xfs_trans_t	*tp,
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT((first <= last) && (last < XFS_BUF_COUNT(bp)));
@@ -701,7 +697,6 @@ xfs_trans_binval(
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
@@ -773,7 +768,6 @@ xfs_trans_inode_buf(
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
@@ -797,7 +791,6 @@ xfs_trans_stale_inode_buf(
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
@@ -822,7 +815,6 @@ xfs_trans_inode_alloc_buf(
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
@@ -850,7 +842,6 @@ xfs_trans_dquot_buf(
 {
 	xfs_buf_log_item_t	*bip = bp->b_fspriv;
 
-	ASSERT(XFS_BUF_ISBUSY(bp));
 	ASSERT(bp->b_transp == tp);
 	ASSERT(bip != NULL);
 	ASSERT(type == XFS_BLF_UDQUOT_BUF ||
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 05/12] xfs: Remove macro XFS_BUF_HOLD

[Chandra Seetharaman]

Remove the definition and usage of the macro XFS_BUF_HOLD

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/xfs_buf_item.c      |    2 +-
 fs/xfs/xfs_log_recover.c   |    2 +-
 fs/xfs/xfs_mount.c         |    2 +-
 4 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index ce5c7e2..1828b29 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -253,7 +253,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNASYNC(bp)	((bp)->b_flags &= ~XBF_ASYNC)
 #define XFS_BUF_ISASYNC(bp)	((bp)->b_flags & XBF_ASYNC)
 
-#define XFS_BUF_HOLD(bp)	xfs_buf_hold(bp)
 #define XFS_BUF_READ(bp)	((bp)->b_flags |= XBF_READ)
 #define XFS_BUF_UNREAD(bp)	((bp)->b_flags &= ~XBF_READ)
 #define XFS_BUF_ISREAD(bp)	((bp)->b_flags & XBF_READ)
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 9e9b4a7..a6dd497 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -484,7 +484,7 @@ xfs_buf_item_trylock(
 		return XFS_ITEM_LOCKED;
 
 	/* take a reference to the buffer.  */
-	XFS_BUF_HOLD(bp);
+	xfs_buf_hold(bp);
 
 	ASSERT(!(bip->bli_flags & XFS_BLI_STALE));
 	trace_xfs_buf_item_trylock(bip);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index f774713..8b02dca 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -265,7 +265,7 @@ xlog_bwrite(
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
 	bp->b_flags &= ~(XBF_SYNCIO|XBF_FUA|XBF_FLUSH);
-	XFS_BUF_HOLD(bp);
+	xfs_buf_hold(bp);
 	xfs_buf_lock(bp);
 	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
 	XFS_BUF_SET_TARGET(bp, log->l_mp->m_logdev_targp);
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 7f25245..b00c808 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1938,7 +1938,7 @@ xfs_getsb(
 		xfs_buf_lock(bp);
 	}
 
-	XFS_BUF_HOLD(bp);
+	xfs_buf_hold(bp);
 	ASSERT(XFS_BUF_ISDONE(bp));
 	return bp;
 }
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START

[Chandra Seetharaman]

Remove the definition and usage of the macro XFS_BUF_SET_START.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.h |    2 --
 fs/xfs/xfs_buf_item.c      |    1 -
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 1828b29..6758697 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -261,8 +261,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNWRITE(bp)	((bp)->b_flags &= ~XBF_WRITE)
 #define XFS_BUF_ISWRITE(bp)	((bp)->b_flags & XBF_WRITE)
 
-#define XFS_BUF_SET_START(bp)			do { } while (0)
-
 #define XFS_BUF_PTR(bp)			(xfs_caddr_t)((bp)->b_addr)
 #define XFS_BUF_SET_PTR(bp, val, cnt)	xfs_buf_associate_memory(bp, val, cnt)
 #define XFS_BUF_ADDR(bp)		((bp)->b_bn)
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index a6dd497..bd4c62b 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -994,7 +994,6 @@ xfs_buf_iodone_callbacks(
 		if (!XFS_BUF_ISSTALE(bp)) {
 			XFS_BUF_DELAYWRITE(bp);
 			XFS_BUF_DONE(bp);
-			XFS_BUF_SET_START(bp);
 		}
 		ASSERT(bp->b_iodone != NULL);
 		trace_xfs_buf_item_iodone_async(bp, _RET_IP_);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR

[Chandra Seetharaman]

Remove the definition and usages of the macro XFS_BUF_PTR.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.c |    2 +-
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/quota/xfs_dquot.c   |    6 +++---
 fs/xfs/quota/xfs_qm.c      |    2 +-
 fs/xfs/xfs_ag.h            |    6 +++---
 fs/xfs/xfs_bmap.c          |    3 +--
 fs/xfs/xfs_btree.h         |    2 +-
 fs/xfs/xfs_buf_item.c      |    6 +++---
 fs/xfs/xfs_da_btree.c      |   10 +++++-----
 fs/xfs/xfs_dinode.h        |    2 +-
 fs/xfs/xfs_log.c           |    2 +-
 fs/xfs/xfs_log_recover.c   |   10 +++++-----
 fs/xfs/xfs_rtalloc.c       |   30 +++++++++++++++---------------
 fs/xfs/xfs_rtalloc.h       |    2 +-
 fs/xfs/xfs_sb.h            |    2 +-
 fs/xfs/xfs_vnodeops.c      |    6 +++---
 16 files changed, 45 insertions(+), 47 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index ae2c2e7..6a42f71 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -1320,7 +1320,7 @@ xfs_buf_offset(
 	struct page		*page;
 
 	if (bp->b_flags & XBF_MAPPED)
-		return XFS_BUF_PTR(bp) + offset;
+		return bp->b_addr + offset;
 
 	offset += bp->b_offset;
 	page = bp->b_pages[offset >> PAGE_SHIFT];
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 6758697..6ae7bde 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -261,7 +261,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNWRITE(bp)	((bp)->b_flags &= ~XBF_WRITE)
 #define XFS_BUF_ISWRITE(bp)	((bp)->b_flags & XBF_WRITE)
 
-#define XFS_BUF_PTR(bp)			(xfs_caddr_t)((bp)->b_addr)
 #define XFS_BUF_SET_PTR(bp, val, cnt)	xfs_buf_associate_memory(bp, val, cnt)
 #define XFS_BUF_ADDR(bp)		((bp)->b_bn)
 #define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_bn = (xfs_daddr_t)(bno))
diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
index 772ae2a..fccde4a 100644
--- a/fs/xfs/quota/xfs_dquot.c
+++ b/fs/xfs/quota/xfs_dquot.c
@@ -320,7 +320,7 @@ xfs_qm_init_dquot_blk(
 	ASSERT(tp);
 	ASSERT(xfs_buf_islocked(bp));
 
-	d = (xfs_dqblk_t *)XFS_BUF_PTR(bp);
+	d = bp->b_addr;
 
 	/*
 	 * ID of the first dquot in the block - id's are zero based.
@@ -539,7 +539,7 @@ xfs_qm_dqtobp(
 	/*
 	 * calculate the location of the dquot inside the buffer.
 	 */
-	ddq = (struct xfs_disk_dquot *)(XFS_BUF_PTR(bp) + dqp->q_bufoffset);
+	ddq = bp->b_addr + dqp->q_bufoffset;
 
 	/*
 	 * A simple sanity check in case we got a corrupted dquot...
@@ -1201,7 +1201,7 @@ xfs_qm_dqflush(
 	/*
 	 * Calculate the location of the dquot inside the buffer.
 	 */
-	ddqp = (struct xfs_disk_dquot *)(XFS_BUF_PTR(bp) + dqp->q_bufoffset);
+	ddqp = bp->b_addr + dqp->q_bufoffset;
 
 	/*
 	 * A simple sanity check in case we got a corrupted dquot..
diff --git a/fs/xfs/quota/xfs_qm.c b/fs/xfs/quota/xfs_qm.c
index 46e54ad..9a0aa76 100644
--- a/fs/xfs/quota/xfs_qm.c
+++ b/fs/xfs/quota/xfs_qm.c
@@ -1240,7 +1240,7 @@ xfs_qm_reset_dqcounts(
 	do_div(j, sizeof(xfs_dqblk_t));
 	ASSERT(mp->m_quotainfo->qi_dqperchunk == j);
 #endif
-	ddq = (xfs_disk_dquot_t *)XFS_BUF_PTR(bp);
+	ddq = bp->b_addr;
 	for (j = 0; j < mp->m_quotainfo->qi_dqperchunk; j++) {
 		/*
 		 * Do a sanity check, and if needed, repair the dqblk. Don't
diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
index 6530769..4805f00 100644
--- a/fs/xfs/xfs_ag.h
+++ b/fs/xfs/xfs_ag.h
@@ -103,7 +103,7 @@ typedef struct xfs_agf {
 /* disk block (xfs_daddr_t) in the AG */
 #define XFS_AGF_DADDR(mp)	((xfs_daddr_t)(1 << (mp)->m_sectbb_log))
 #define	XFS_AGF_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGF_DADDR(mp))
-#define	XFS_BUF_TO_AGF(bp)	((xfs_agf_t *)XFS_BUF_PTR(bp))
+#define	XFS_BUF_TO_AGF(bp)	((xfs_agf_t *)((bp)->b_addr))
 
 extern int xfs_read_agf(struct xfs_mount *mp, struct xfs_trans *tp,
 			xfs_agnumber_t agno, int flags, struct xfs_buf **bpp);
@@ -156,7 +156,7 @@ typedef struct xfs_agi {
 /* disk block (xfs_daddr_t) in the AG */
 #define XFS_AGI_DADDR(mp)	((xfs_daddr_t)(2 << (mp)->m_sectbb_log))
 #define	XFS_AGI_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGI_DADDR(mp))
-#define	XFS_BUF_TO_AGI(bp)	((xfs_agi_t *)XFS_BUF_PTR(bp))
+#define	XFS_BUF_TO_AGI(bp)	((xfs_agi_t *)((bp)->b_addr))
 
 extern int xfs_read_agi(struct xfs_mount *mp, struct xfs_trans *tp,
 				xfs_agnumber_t agno, struct xfs_buf **bpp);
@@ -168,7 +168,7 @@ extern int xfs_read_agi(struct xfs_mount *mp, struct xfs_trans *tp,
 #define XFS_AGFL_DADDR(mp)	((xfs_daddr_t)(3 << (mp)->m_sectbb_log))
 #define	XFS_AGFL_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_AGFL_DADDR(mp))
 #define XFS_AGFL_SIZE(mp)	((mp)->m_sb.sb_sectsize / sizeof(xfs_agblock_t))
-#define	XFS_BUF_TO_AGFL(bp)	((xfs_agfl_t *)XFS_BUF_PTR(bp))
+#define	XFS_BUF_TO_AGFL(bp)	((xfs_agfl_t *)((bp)->b_addr))
 
 typedef struct xfs_agfl {
 	__be32		agfl_bno[1];	/* actually XFS_AGFL_SIZE(mp) */
diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index c51a3f9..25cb2b2 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -3384,8 +3384,7 @@ xfs_bmap_local_to_extents(
 		ASSERT(args.len == 1);
 		*firstblock = args.fsbno;
 		bp = xfs_btree_get_bufl(args.mp, tp, args.fsbno, 0);
-		memcpy((char *)XFS_BUF_PTR(bp), ifp->if_u1.if_data,
-			ifp->if_bytes);
+		memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
 		xfs_trans_log_buf(tp, bp, 0, ifp->if_bytes - 1);
 		xfs_bmap_forkoff_reset(args.mp, ip, whichfork);
 		xfs_idata_realloc(ip, -ifp->if_bytes, whichfork);
diff --git a/fs/xfs/xfs_btree.h b/fs/xfs/xfs_btree.h
index 8d05a6a..5b240de 100644
--- a/fs/xfs/xfs_btree.h
+++ b/fs/xfs/xfs_btree.h
@@ -262,7 +262,7 @@ typedef struct xfs_btree_cur
 /*
  * Convert from buffer to btree block header.
  */
-#define	XFS_BUF_TO_BLOCK(bp)	((struct xfs_btree_block *)XFS_BUF_PTR(bp))
+#define	XFS_BUF_TO_BLOCK(bp)	((struct xfs_btree_block *)((bp)->b_addr))
 
 
 /*
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index bd4c62b..a16c24c 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -124,9 +124,9 @@ xfs_buf_item_log_check(
 
 	bp = bip->bli_buf;
 	ASSERT(XFS_BUF_COUNT(bp) > 0);
-	ASSERT(XFS_BUF_PTR(bp) != NULL);
+	ASSERT(bp->b_addr != NULL);
 	orig = bip->bli_orig;
-	buffer = XFS_BUF_PTR(bp);
+	buffer = bp->b_addr;
 	for (x = 0; x < XFS_BUF_COUNT(bp); x++) {
 		if (orig[x] != buffer[x] && !btst(bip->bli_logged, x)) {
 			xfs_emerg(bp->b_mount,
@@ -725,7 +725,7 @@ xfs_buf_item_init(
 	 * to have logged.
 	 */
 	bip->bli_orig = (char *)kmem_alloc(XFS_BUF_COUNT(bp), KM_SLEEP);
-	memcpy(bip->bli_orig, XFS_BUF_PTR(bp), XFS_BUF_COUNT(bp));
+	memcpy(bip->bli_orig, bp->b_addr, XFS_BUF_COUNT(bp));
 	bip->bli_logged = (char *)kmem_zalloc(XFS_BUF_COUNT(bp) / NBBY, KM_SLEEP);
 #endif
 
diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
index 5d9290d..d56ccb7 100644
--- a/fs/xfs/xfs_da_btree.c
+++ b/fs/xfs/xfs_da_btree.c
@@ -2258,7 +2258,7 @@ xfs_da_buf_make(int nbuf, xfs_buf_t **bps)
 		dabuf->nbuf = 1;
 		bp = bps[0];
 		dabuf->bbcount = (short)BTOBB(XFS_BUF_COUNT(bp));
-		dabuf->data = XFS_BUF_PTR(bp);
+		dabuf->data = bp->b_addr;
 		dabuf->bps[0] = bp;
 	} else {
 		dabuf->nbuf = nbuf;
@@ -2269,7 +2269,7 @@ xfs_da_buf_make(int nbuf, xfs_buf_t **bps)
 		dabuf->data = kmem_alloc(BBTOB(dabuf->bbcount), KM_SLEEP);
 		for (i = off = 0; i < nbuf; i++, off += XFS_BUF_COUNT(bp)) {
 			bp = bps[i];
-			memcpy((char *)dabuf->data + off, XFS_BUF_PTR(bp),
+			memcpy((char *)dabuf->data + off, bp->b_addr,
 				XFS_BUF_COUNT(bp));
 		}
 	}
@@ -2292,8 +2292,8 @@ xfs_da_buf_clean(xfs_dabuf_t *dabuf)
 		for (i = off = 0; i < dabuf->nbuf;
 				i++, off += XFS_BUF_COUNT(bp)) {
 			bp = dabuf->bps[i];
-			memcpy(XFS_BUF_PTR(bp), (char *)dabuf->data + off,
-				XFS_BUF_COUNT(bp));
+			memcpy(bp->b_addr, dabuf->data + off,
+						XFS_BUF_COUNT(bp));
 		}
 	}
 }
@@ -2330,7 +2330,7 @@ xfs_da_log_buf(xfs_trans_t *tp, xfs_dabuf_t *dabuf, uint first, uint last)
 
 	ASSERT(dabuf->nbuf && dabuf->data && dabuf->bbcount && dabuf->bps[0]);
 	if (dabuf->nbuf == 1) {
-		ASSERT(dabuf->data == (void *)XFS_BUF_PTR(dabuf->bps[0]));
+		ASSERT(dabuf->data == dabuf->bps[0]->b_addr);
 		xfs_trans_log_buf(tp, dabuf->bps[0], first, last);
 		return;
 	}
diff --git a/fs/xfs/xfs_dinode.h b/fs/xfs/xfs_dinode.h
index dffba9b..a372163 100644
--- a/fs/xfs/xfs_dinode.h
+++ b/fs/xfs/xfs_dinode.h
@@ -148,7 +148,7 @@ typedef enum xfs_dinode_fmt {
 		be32_to_cpu((dip)->di_nextents) : \
 		be16_to_cpu((dip)->di_anextents))
 
-#define	XFS_BUF_TO_DINODE(bp)	((xfs_dinode_t *)XFS_BUF_PTR(bp))
+#define	XFS_BUF_TO_DINODE(bp)	((xfs_dinode_t *)((bp)->b_addr))
 
 /*
  * For block and character special files the 32bit dev_t is stored at the
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 5805a3a..4255a1c 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1403,7 +1403,7 @@ xlog_sync(xlog_t		*log,
 		bp->b_flags |= XBF_SYNCIO;
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 			bp->b_flags |= XBF_FUA;
-		dptr = XFS_BUF_PTR(bp);
+		dptr = bp->b_addr;
 		/*
 		 * Bump the cycle numbers at the start of each block
 		 * since this part of the buffer is at the start of
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 8b02dca..11ff19c 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -147,7 +147,7 @@ xlog_align(
 	xfs_daddr_t	offset = blk_no & ((xfs_daddr_t)log->l_sectBBsize - 1);
 
 	ASSERT(BBTOB(offset + nbblks) <= XFS_BUF_SIZE(bp));
-	return XFS_BUF_PTR(bp) + BBTOB(offset);
+	return bp->b_addr + BBTOB(offset);
 }
 
 
@@ -219,7 +219,7 @@ xlog_bread_offset(
 	xfs_buf_t	*bp,
 	xfs_caddr_t	offset)
 {
-	xfs_caddr_t	orig_offset = XFS_BUF_PTR(bp);
+	xfs_caddr_t	orig_offset = bp->b_addr;
 	int		orig_len = bp->b_buffer_length;
 	int		error, error2;
 
@@ -1260,7 +1260,7 @@ xlog_write_log_records(
 		 */
 		ealign = round_down(end_block, sectbb);
 		if (j == 0 && (start_block + endcount > ealign)) {
-			offset = XFS_BUF_PTR(bp) + BBTOB(ealign - start_block);
+			offset = bp->b_addr + BBTOB(ealign - start_block);
 			error = xlog_bread_offset(log, ealign, sectbb,
 							bp, offset);
 			if (error)
@@ -3433,7 +3433,7 @@ xlog_do_recovery_pass(
 			/*
 			 * Check for header wrapping around physical end-of-log
 			 */
-			offset = XFS_BUF_PTR(hbp);
+			offset = hbp->b_addr;
 			split_hblks = 0;
 			wrapped_hblks = 0;
 			if (blk_no + hblks <= log->l_logBBsize) {
@@ -3493,7 +3493,7 @@ xlog_do_recovery_pass(
 			} else {
 				/* This log record is split across the
 				 * physical end of log */
-				offset = XFS_BUF_PTR(dbp);
+				offset = dbp->b_addr;
 				split_bblks = 0;
 				if (blk_no != log->l_logBBsize) {
 					/* some data is before the physical
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index cb8132c..35561a5 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -168,7 +168,7 @@ error_cancel:
 				xfs_trans_cancel(tp, cancelflags);
 				goto error;
 			}
-			memset(XFS_BUF_PTR(bp), 0, mp->m_sb.sb_blocksize);
+			memset(bp->b_addr, 0, mp->m_sb.sb_blocksize);
 			xfs_trans_log_buf(tp, bp, 0, mp->m_sb.sb_blocksize - 1);
 			/*
 			 * Commit the transaction.
@@ -943,7 +943,7 @@ xfs_rtcheck_range(
 	if (error) {
 		return error;
 	}
-	bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+	bufp = bp->b_addr;
 	/*
 	 * Compute the starting word's address, and starting bit.
 	 */
@@ -994,7 +994,7 @@ xfs_rtcheck_range(
 			if (error) {
 				return error;
 			}
-			b = bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			b = bufp = bp->b_addr;
 			word = 0;
 		} else {
 			/*
@@ -1040,7 +1040,7 @@ xfs_rtcheck_range(
 			if (error) {
 				return error;
 			}
-			b = bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			b = bufp = bp->b_addr;
 			word = 0;
 		} else {
 			/*
@@ -1158,7 +1158,7 @@ xfs_rtfind_back(
 	if (error) {
 		return error;
 	}
-	bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+	bufp = bp->b_addr;
 	/*
 	 * Get the first word's index & point to it.
 	 */
@@ -1210,7 +1210,7 @@ xfs_rtfind_back(
 			if (error) {
 				return error;
 			}
-			bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			bufp = bp->b_addr;
 			word = XFS_BLOCKWMASK(mp);
 			b = &bufp[word];
 		} else {
@@ -1256,7 +1256,7 @@ xfs_rtfind_back(
 			if (error) {
 				return error;
 			}
-			bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			bufp = bp->b_addr;
 			word = XFS_BLOCKWMASK(mp);
 			b = &bufp[word];
 		} else {
@@ -1333,7 +1333,7 @@ xfs_rtfind_forw(
 	if (error) {
 		return error;
 	}
-	bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+	bufp = bp->b_addr;
 	/*
 	 * Get the first word's index & point to it.
 	 */
@@ -1384,7 +1384,7 @@ xfs_rtfind_forw(
 			if (error) {
 				return error;
 			}
-			b = bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			b = bufp = bp->b_addr;
 			word = 0;
 		} else {
 			/*
@@ -1429,7 +1429,7 @@ xfs_rtfind_forw(
 			if (error) {
 				return error;
 			}
-			b = bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			b = bufp = bp->b_addr;
 			word = 0;
 		} else {
 			/*
@@ -1649,7 +1649,7 @@ xfs_rtmodify_range(
 	if (error) {
 		return error;
 	}
-	bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+	bufp = bp->b_addr;
 	/*
 	 * Compute the starting word's address, and starting bit.
 	 */
@@ -1694,7 +1694,7 @@ xfs_rtmodify_range(
 			if (error) {
 				return error;
 			}
-			first = b = bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			first = b = bufp = bp->b_addr;
 			word = 0;
 		} else {
 			/*
@@ -1734,7 +1734,7 @@ xfs_rtmodify_range(
 			if (error) {
 				return error;
 			}
-			first = b = bufp = (xfs_rtword_t *)XFS_BUF_PTR(bp);
+			first = b = bufp = bp->b_addr;
 			word = 0;
 		} else {
 			/*
@@ -1832,8 +1832,8 @@ xfs_rtmodify_summary(
 	 */
 	sp = XFS_SUMPTR(mp, bp, so);
 	*sp += delta;
-	xfs_trans_log_buf(tp, bp, (uint)((char *)sp - (char *)XFS_BUF_PTR(bp)),
-		(uint)((char *)sp - (char *)XFS_BUF_PTR(bp) + sizeof(*sp) - 1));
+	xfs_trans_log_buf(tp, bp, (uint)((char *)sp - (char *)bp->b_addr),
+		(uint)((char *)sp - (char *)bp->b_addr + sizeof(*sp) - 1));
 	return 0;
 }
 
diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h
index 09e1f4f..f7f3a35 100644
--- a/fs/xfs/xfs_rtalloc.h
+++ b/fs/xfs/xfs_rtalloc.h
@@ -47,7 +47,7 @@ struct xfs_trans;
 #define	XFS_SUMOFFSTOBLOCK(mp,s)	\
 	(((s) * (uint)sizeof(xfs_suminfo_t)) >> (mp)->m_sb.sb_blocklog)
 #define	XFS_SUMPTR(mp,bp,so)	\
-	((xfs_suminfo_t *)((char *)XFS_BUF_PTR(bp) + \
+	((xfs_suminfo_t *)((bp)->b_addr + \
 		(((so) * (uint)sizeof(xfs_suminfo_t)) & XFS_BLOCKMASK(mp))))
 
 #define	XFS_BITTOBLOCK(mp,bi)	((bi) >> (mp)->m_blkbit_log)
diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
index 1eb2ba5..cb6ae71 100644
--- a/fs/xfs/xfs_sb.h
+++ b/fs/xfs/xfs_sb.h
@@ -509,7 +509,7 @@ static inline int xfs_sb_version_hasprojid32bit(xfs_sb_t *sbp)
 
 #define XFS_SB_DADDR		((xfs_daddr_t)0) /* daddr in filesystem/ag */
 #define	XFS_SB_BLOCK(mp)	XFS_HDR_BLOCK(mp, XFS_SB_DADDR)
-#define XFS_BUF_TO_SBP(bp)	((xfs_dsb_t *)XFS_BUF_PTR(bp))
+#define XFS_BUF_TO_SBP(bp)	((xfs_dsb_t *)((bp)->b_addr))
 
 #define	XFS_HDR_BLOCK(mp,d)	((xfs_agblock_t)XFS_BB_TO_FSBT(mp,d))
 #define	XFS_DADDR_TO_FSB(mp,d)	XFS_AGB_TO_FSB(mp, \
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 97daa35..07f480a 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -94,7 +94,7 @@ xfs_readlink_bmap(
 			byte_cnt = pathlen;
 		pathlen -= byte_cnt;
 
-		memcpy(link, XFS_BUF_PTR(bp), byte_cnt);
+		memcpy(link, bp->b_addr, byte_cnt);
 		xfs_buf_relse(bp);
 	}
 
@@ -1654,7 +1654,7 @@ xfs_symlink(
 			}
 			pathlen -= byte_cnt;
 
-			memcpy(XFS_BUF_PTR(bp), cur_chunk, byte_cnt);
+			memcpy(bp->b_addr, cur_chunk, byte_cnt);
 			cur_chunk += byte_cnt;
 
 			xfs_trans_log_buf(tp, bp, 0, byte_cnt - 1);
@@ -1999,7 +1999,7 @@ xfs_zero_remaining_bytes(
 					  mp, bp, XFS_BUF_ADDR(bp));
 			break;
 		}
-		memset(XFS_BUF_PTR(bp) +
+		memset(bp->b_addr +
 			(offset - XFS_FSB_TO_B(mp, imap.br_startoff)),
 		      0, lastoffset - offset + 1);
 		XFS_BUF_UNDONE(bp);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Chandra Seetharaman]

Remove the definition and usages of the macro XFS_BUF_SET_PTR.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/xfs_log.c           |    5 +++--
 fs/xfs/xfs_log_recover.c   |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 6ae7bde..7b1f484 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -261,7 +261,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNWRITE(bp)	((bp)->b_flags &= ~XBF_WRITE)
 #define XFS_BUF_ISWRITE(bp)	((bp)->b_flags & XBF_WRITE)
 
-#define XFS_BUF_SET_PTR(bp, val, cnt)	xfs_buf_associate_memory(bp, val, cnt)
 #define XFS_BUF_ADDR(bp)		((bp)->b_bn)
 #define XFS_BUF_SET_ADDR(bp, bno)	((bp)->b_bn = (xfs_daddr_t)(bno))
 #define XFS_BUF_OFFSET(bp)		((bp)->b_file_offset)
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 4255a1c..21e770f 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1395,8 +1395,9 @@ xlog_sync(xlog_t		*log,
 	if (split) {
 		bp = iclog->ic_log->l_xbuf;
 		XFS_BUF_SET_ADDR(bp, 0);	     /* logical 0 */
-		XFS_BUF_SET_PTR(bp, (xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
-					    (__psint_t)count), split);
+		xfs_buf_associate_memory(bp,
+				(xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
+				    (__psint_t)count), split);
 		bp->b_fspriv = iclog;
 		bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
 		XFS_BUF_ASYNC(bp);
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 11ff19c..3f77077 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -223,14 +223,14 @@ xlog_bread_offset(
 	int		orig_len = bp->b_buffer_length;
 	int		error, error2;
 
-	error = XFS_BUF_SET_PTR(bp, offset, BBTOB(nbblks));
+	error = xfs_buf_associate_memory(bp, offset, BBTOB(nbblks));
 	if (error)
 		return error;
 
 	error = xlog_bread_noalign(log, blk_no, nbblks, bp);
 
 	/* must reset buffer pointer even on error */
-	error2 = XFS_BUF_SET_PTR(bp, orig_offset, orig_len);
+	error2 = xfs_buf_associate_memory(bp, orig_offset, orig_len);
 	if (error)
 		return error;
 	return error2;
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned

[Chandra Seetharaman]

Replace the macro XFS_BUF_ISPINNED with an inline helper function
xfs_buf_ispinned, and change all its usages.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.c  |    2 +-
 fs/xfs/linux-2.6/xfs_buf.h  |    5 ++++-
 fs/xfs/linux-2.6/xfs_sync.c |    2 +-
 fs/xfs/quota/xfs_dquot.c    |    4 ++--
 fs/xfs/xfs_buf_item.c       |    2 +-
 fs/xfs/xfs_inode.c          |    2 +-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 6a42f71..5e929f0 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -1677,7 +1677,7 @@ xfs_buf_delwri_split(
 	list_for_each_entry_safe(bp, n, dwq, b_list) {
 		ASSERT(bp->b_flags & XBF_DELWRI);
 
-		if (!XFS_BUF_ISPINNED(bp) && xfs_buf_trylock(bp)) {
+		if (!xfs_buf_ispinned(bp) && xfs_buf_trylock(bp)) {
 			if (!force &&
 			    time_before(jiffies, bp->b_queuetime + age)) {
 				xfs_buf_unlock(bp);
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 7b1f484..71e1d6f 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -280,7 +280,10 @@ xfs_buf_set_ref(
 #define XFS_BUF_SET_VTYPE_REF(bp, type, ref)	xfs_buf_set_ref(bp, ref)
 #define XFS_BUF_SET_VTYPE(bp, type)		do { } while (0)
 
-#define XFS_BUF_ISPINNED(bp)	atomic_read(&((bp)->b_pin_count))
+static inline int xfs_buf_ispinned(struct xfs_buf *bp)
+{
+	return atomic_read(&(bp->b_pin_count));
+}
 
 #define XFS_BUF_FINISH_IOWAIT(bp)	complete(&bp->b_iowait);
 
diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index 5cc158e..a8500e9 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -330,7 +330,7 @@ xfs_sync_fsdata(
 	 * between there and here.
 	 */
 	bp = xfs_getsb(mp, 0);
-	if (XFS_BUF_ISPINNED(bp))
+	if (xfs_buf_ispinned(bp))
 		xfs_log_force(mp, 0);
 
 	return xfs_bwrite(mp, bp);
diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
index fccde4a..4bebb29 100644
--- a/fs/xfs/quota/xfs_dquot.c
+++ b/fs/xfs/quota/xfs_dquot.c
@@ -1237,7 +1237,7 @@ xfs_qm_dqflush(
 	 * If the buffer is pinned then push on the log so we won't
 	 * get stuck waiting in the write for too long.
 	 */
-	if (XFS_BUF_ISPINNED(bp)) {
+	if (xfs_buf_ispinned(bp)) {
 		trace_xfs_dqflush_force(dqp);
 		xfs_log_force(mp, 0);
 	}
@@ -1444,7 +1444,7 @@ xfs_qm_dqflock_pushbuf_wait(
 		goto out_lock;
 
 	if (XFS_BUF_ISDELAYWRITE(bp)) {
-		if (XFS_BUF_ISPINNED(bp))
+		if (xfs_buf_ispinned(bp))
 			xfs_log_force(mp, 0);
 		xfs_buf_delwri_promote(bp);
 		wake_up_process(bp->b_target->bt_task);
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index a16c24c..a3d2bbc 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -478,7 +478,7 @@ xfs_buf_item_trylock(
 	struct xfs_buf_log_item	*bip = BUF_ITEM(lip);
 	struct xfs_buf		*bp = bip->bli_buf;
 
-	if (XFS_BUF_ISPINNED(bp))
+	if (xfs_buf_ispinned(bp))
 		return XFS_ITEM_PINNED;
 	if (!xfs_buf_trylock(bp))
 		return XFS_ITEM_LOCKED;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index bdb47b2..76ee2c5 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2585,7 +2585,7 @@ xfs_iflush(
 	 * If the buffer is pinned then push on the log now so we won't
 	 * get stuck waiting in the write for too long.
 	 */
-	if (XFS_BUF_ISPINNED(bp))
+	if (xfs_buf_ispinned(bp))
 		xfs_log_force(mp, 0);
 
 	/*
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 10/12] xfs: Remove the macro XFS_BUF_SET_TARGET

[Chandra Seetharaman]

Remove the macro XFS_BUF_SET_TARGET.

hch: As all the buffer allocator already set ->b_target it should be safe
to simply remove these calls.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/xfs_log_recover.c   |    2 --
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 71e1d6f..31495cf 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -287,7 +287,6 @@ static inline int xfs_buf_ispinned(struct xfs_buf *bp)
 
 #define XFS_BUF_FINISH_IOWAIT(bp)	complete(&bp->b_iowait);
 
-#define XFS_BUF_SET_TARGET(bp, target)	((bp)->b_target = (target))
 #define XFS_BUF_TARGET(bp)		((bp)->b_target)
 #define XFS_BUFTARG_NAME(target)	xfs_buf_target_name(target)
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 3f77077..713ba9c 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -179,7 +179,6 @@ xlog_bread_noalign(
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
 	XFS_BUF_READ(bp);
 	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
-	XFS_BUF_SET_TARGET(bp, log->l_mp->m_logdev_targp);
 
 	xfsbdstrat(log->l_mp, bp);
 	error = xfs_buf_iowait(bp);
@@ -268,7 +267,6 @@ xlog_bwrite(
 	xfs_buf_hold(bp);
 	xfs_buf_lock(bp);
 	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
-	XFS_BUF_SET_TARGET(bp, log->l_mp->m_logdev_targp);
 
 	if ((error = xfs_bwrite(log->l_mp, bp)))
 		xfs_ioerror_alert("xlog_bwrite", log->l_mp,
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 11/12] xfs: Remove the macro XFS_BUF_TARGET

[Chandra Seetharaman]

Remove the definition and usages of the macro XFS_BUF_TARGET

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.h |    1 -
 fs/xfs/xfs_buf_item.c      |    6 +++---
 fs/xfs/xfs_mount.c         |    2 +-
 fs/xfs/xfs_rw.c            |    2 +-
 fs/xfs/xfs_trans_buf.c     |    2 +-
 5 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 31495cf..075f567 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -287,7 +287,6 @@ static inline int xfs_buf_ispinned(struct xfs_buf *bp)
 
 #define XFS_BUF_FINISH_IOWAIT(bp)	complete(&bp->b_iowait);
 
-#define XFS_BUF_TARGET(bp)		((bp)->b_target)
 #define XFS_BUFTARG_NAME(target)	xfs_buf_target_name(target)
 
 static inline void xfs_buf_relse(xfs_buf_t *bp)
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index a3d2bbc..5c2b554 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -971,14 +971,14 @@ xfs_buf_iodone_callbacks(
 		goto do_callbacks;
 	}
 
-	if (XFS_BUF_TARGET(bp) != lasttarg ||
+	if (bp->b_target != lasttarg ||
 	    time_after(jiffies, (lasttime + 5*HZ))) {
 		lasttime = jiffies;
 		xfs_alert(mp, "Device %s: metadata write error block 0x%llx",
-			XFS_BUFTARG_NAME(XFS_BUF_TARGET(bp)),
+			XFS_BUFTARG_NAME(bp->b_target),
 		      (__uint64_t)XFS_BUF_ADDR(bp));
 	}
-	lasttarg = XFS_BUF_TARGET(bp);
+	lasttarg = bp->b_target;
 
 	/*
 	 * If the write was asynchronous then no one will be looking for the
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index b00c808..49ecc17 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1615,7 +1615,7 @@ xfs_unmountfs_writesb(xfs_mount_t *mp)
 		XFS_BUF_UNDELAYWRITE(sbp);
 		XFS_BUF_WRITE(sbp);
 		XFS_BUF_UNASYNC(sbp);
-		ASSERT(XFS_BUF_TARGET(sbp) == mp->m_ddev_targp);
+		ASSERT(sbp->b_target == mp->m_ddev_targp);
 		xfsbdstrat(mp, sbp);
 		error = xfs_buf_iowait(sbp);
 		if (error)
diff --git a/fs/xfs/xfs_rw.c b/fs/xfs/xfs_rw.c
index d1f76f8..7382bfe 100644
--- a/fs/xfs/xfs_rw.c
+++ b/fs/xfs/xfs_rw.c
@@ -104,7 +104,7 @@ xfs_ioerror_alert(
 	xfs_alert(mp,
 		 "I/O error occurred: meta-data dev %s block 0x%llx"
 		 "       (\"%s\") error %d buf count %zd",
-		XFS_BUFTARG_NAME(XFS_BUF_TARGET(bp)),
+		XFS_BUFTARG_NAME(bp->b_target),
 		(__uint64_t)blkno, func,
 		bp->b_error, XFS_BUF_COUNT(bp));
 }
diff --git a/fs/xfs/xfs_trans_buf.c b/fs/xfs/xfs_trans_buf.c
index 7dd62e2..137e2b9 100644
--- a/fs/xfs/xfs_trans_buf.c
+++ b/fs/xfs/xfs_trans_buf.c
@@ -54,7 +54,7 @@ xfs_trans_buf_item_match(
 	list_for_each_entry(lidp, &tp->t_items, lid_trans) {
 		blip = (struct xfs_buf_log_item *)lidp->lid_item;
 		if (blip->bli_item.li_type == XFS_LI_BUF &&
-		    XFS_BUF_TARGET(blip->bli_buf) == target &&
+		    blip->bli_buf->b_target == target &&
 		    XFS_BUF_ADDR(blip->bli_buf) == blkno &&
 		    XFS_BUF_COUNT(blip->bli_buf) == len)
 			return blip->bli_buf;
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Chandra Seetharaman]

Remove the definition and usages of the macro XFS_BUFTARG_NAME.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/linux-2.6/xfs_buf.c |    2 +-
 fs/xfs/linux-2.6/xfs_buf.h |    2 --
 fs/xfs/xfs_buf_item.c      |    2 +-
 fs/xfs/xfs_rw.c            |    2 +-
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 5e929f0..6bddce4 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -1480,7 +1480,7 @@ xfs_setsize_buftarg_flags(
 	if (set_blocksize(btp->bt_bdev, sectorsize)) {
 		xfs_warn(btp->bt_mount,
 			"Cannot set_blocksize to %u on device %s\n",
-			sectorsize, XFS_BUFTARG_NAME(btp));
+			sectorsize, xfs_buf_target_name(btp));
 		return EINVAL;
 	}
 
diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
index 075f567..3a421da 100644
--- a/fs/xfs/linux-2.6/xfs_buf.h
+++ b/fs/xfs/linux-2.6/xfs_buf.h
@@ -287,8 +287,6 @@ static inline int xfs_buf_ispinned(struct xfs_buf *bp)
 
 #define XFS_BUF_FINISH_IOWAIT(bp)	complete(&bp->b_iowait);
 
-#define XFS_BUFTARG_NAME(target)	xfs_buf_target_name(target)
-
 static inline void xfs_buf_relse(xfs_buf_t *bp)
 {
 	xfs_buf_unlock(bp);
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 5c2b554..0402173 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -975,7 +975,7 @@ xfs_buf_iodone_callbacks(
 	    time_after(jiffies, (lasttime + 5*HZ))) {
 		lasttime = jiffies;
 		xfs_alert(mp, "Device %s: metadata write error block 0x%llx",
-			XFS_BUFTARG_NAME(bp->b_target),
+			xfs_buf_target_name(bp->b_target),
 		      (__uint64_t)XFS_BUF_ADDR(bp));
 	}
 	lasttarg = bp->b_target;
diff --git a/fs/xfs/xfs_rw.c b/fs/xfs/xfs_rw.c
index 7382bfe..c96a8a0 100644
--- a/fs/xfs/xfs_rw.c
+++ b/fs/xfs/xfs_rw.c
@@ -104,7 +104,7 @@ xfs_ioerror_alert(
 	xfs_alert(mp,
 		 "I/O error occurred: meta-data dev %s block 0x%llx"
 		 "       (\"%s\") error %d buf count %zd",
-		XFS_BUFTARG_NAME(bp->b_target),
+		xfs_buf_target_name(bp->b_target),
 		(__uint64_t)blkno, func,
 		bp->b_error, XFS_BUF_COUNT(bp));
 }
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 01/12] xfs: Remove the macro XFS_BUF_BFLAGS

[Alex Elder]

On Thu, 2011-07-21 at 17:32 -0700, Chandra Seetharaman wrote:
> Remove the definition of the macro XFS_BUF_BFLAGS and its usage.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family

[Alex Elder]

On Thu, 2011-07-21 at 17:32 -0700, Chandra Seetharaman wrote:
> Remove the definitions and usage of the macros XFS_BUF_ERROR,
> XFS_BUF_GETERROR and XFS_BUF_ISERROR.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Nice work on this.  It is clear it was thoughtfully
done.

I have two things that need to be fixed.  If you do that
you can consider this signed off by me.

Reviewed-by: Alex Elder <aelder@sgi.com>

. . .

> diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
> index 837f311..e7e35fb 100644
> --- a/fs/xfs/quota/xfs_dquot.c
> +++ b/fs/xfs/quota/xfs_dquot.c
> @@ -403,7 +403,8 @@ xfs_qm_dqalloc(
>  			       dqp->q_blkno,
>  			       mp->m_quotainfo->qi_dqchunklen,
>  			       0);
> -	if (!bp || (error = XFS_BUF_GETERROR(bp)))
> +	error = xfs_buf_geterror(bp);
> +	if (error)
>  		goto error1;
>  	/*
>  	 * Make a chunk of dquots out of this buffer and log

This results in behavior that differs from before.
Previously, error would have value 0 following
the call to xfs_trans_get_buf() here, meaning that
(at error1:) xfs_qm_dqalloc() would return 0 in
this case.  Now it will return ENOMEM.

I think what you have done may be correct, but
since the change does more than the simple
macro transformation you intend, this change
should be done in a separate commit.

So either:
- post a new patch (preferably before this
  whole series) that makes this code return
  ENOMEM if xfs_trans_get_buf() returns a
  null pointer, then update this patch accordingly;
- or just change this patch to return 0 instead
  of  ENOMEM if xfs_trans_get_buf() returns a
  null pointer.

. . .

> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 88d1214..97daa35 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -83,7 +83,7 @@ xfs_readlink_bmap(
>  
>  		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
>  				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);

xfs_buf_read() can return NULL here, so to match
the existing behavior you should call xfs_buf_geterror()
here.

> -		error = XFS_BUF_GETERROR(bp);
> +		error = bp->b_error;
>  		if (error) {
>  			xfs_ioerror_alert("xfs_readlink",
>  				  ip->i_mount, bp, XFS_BUF_ADDR(bp));


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 04/12] xfs: Remove macro XFS_BUF_BUSY and family

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Remove the definitions and uses of the macros XFS_BUF_BUSY,
> XFS_BUF_UNBUSY, and XFS_BUF_ISBUSY.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 05/12] xfs: Remove macro XFS_BUF_HOLD

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Remove the definition and usage of the macro XFS_BUF_HOLD
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Remove the definition and usage of the macro XFS_BUF_SET_START.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Remove the definition and usages of the macro XFS_BUF_PTR.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

This looks fine.  One little comment below but it's
really more like a rhetorical question (or statement).
Well done.

Reviewed-by: Alex Elder <aelder@sgi.com>

> diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
> index ae2c2e7..6a42f71 100644
> --- a/fs/xfs/linux-2.6/xfs_buf.c
> +++ b/fs/xfs/linux-2.6/xfs_buf.c
> @@ -1320,7 +1320,7 @@ xfs_buf_offset(
>  	struct page		*page;
>  
>  	if (bp->b_flags & XBF_MAPPED)
> -		return XFS_BUF_PTR(bp) + offset;
> +		return bp->b_addr + offset;

I guess we're using GCC-isms elsewhere (including
possibly this one) so I suppose using arithmetic
on a void pointer is OK.  In any case it looks
nicer this way...

>  
>  	offset += bp->b_offset;
>  	page = bp->b_pages[offset >> PAGE_SHIFT];
> diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
> index 6758697..6ae7bde 100644
> --- a/fs/xfs/linux-2.6/xfs_buf.h
> +++ b/fs/xfs/linux-2.6/xfs_buf.h

. . .

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Remove the definition and usages of the macro XFS_BUF_SET_PTR.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

One suggestion.  Otherwise:

Reviewed-by: Alex Elder <aelder@sgi.com>

> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 4255a1c..21e770f 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -1395,8 +1395,9 @@ xlog_sync(xlog_t		*log,
>  	if (split) {
>  		bp = iclog->ic_log->l_xbuf;
>  		XFS_BUF_SET_ADDR(bp, 0);	     /* logical 0 */
> -		XFS_BUF_SET_PTR(bp, (xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> -					    (__psint_t)count), split);
> +		xfs_buf_associate_memory(bp,
> +				(xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> +				    (__psint_t)count), split);

If you just cast the address appropriately you can make this
look a lot nicer:

		xfs_buf_associate_memory(bp,
			(void *) &iclog->ic_header + count, split);

>  		bp->b_fspriv = iclog;
>  		bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
>  		XFS_BUF_ASYNC(bp);

. . .

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Replace the macro XFS_BUF_ISPINNED with an inline helper function
> xfs_buf_ispinned, and change all its usages.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

One simple suggestion below.  Otherwise:

Reviewed-by: Alex Elder <aelder@sgi.com>

> diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
> index 7b1f484..71e1d6f 100644
> --- a/fs/xfs/linux-2.6/xfs_buf.h
> +++ b/fs/xfs/linux-2.6/xfs_buf.h
> @@ -280,7 +280,10 @@ xfs_buf_set_ref(
>  #define XFS_BUF_SET_VTYPE_REF(bp, type, ref)	xfs_buf_set_ref(bp, ref)
>  #define XFS_BUF_SET_VTYPE(bp, type)		do { } while (0)
>  
> -#define XFS_BUF_ISPINNED(bp)	atomic_read(&((bp)->b_pin_count))
> +static inline int xfs_buf_ispinned(struct xfs_buf *bp)
> +{
> +	return atomic_read(&(bp->b_pin_count));

Good idea.  But drop the extra parentheses:

	return atomic_read(&bp->b_pin_count);

> +}
>  
>  #define XFS_BUF_FINISH_IOWAIT(bp)	complete(&bp->b_iowait);
>  


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 10/12] xfs: Remove the macro XFS_BUF_SET_TARGET

[Alex Elder]

On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> Remove the macro XFS_BUF_SET_TARGET.
> 
> hch: As all the buffer allocator already set ->b_target it should be safe
> to simply remove these calls.

Yes, in _xfs_buf_initialize().

> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 11/12] xfs: Remove the macro XFS_BUF_TARGET

[Alex Elder]

On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> Remove the definition and usages of the macro XFS_BUF_TARGET
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 11/12] xfs: Remove the macro XFS_BUF_TARGET

[Alex Elder]

On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> Remove the definition and usages of the macro XFS_BUF_TARGET
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Alex Elder]

On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> 
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>


Wow, I hadn't looked at the definition of
xfs_buf_target_name() before.  It's not safe
(using a pointer to since-released stack space),
though in practice it's going to be fine.

Defining it as an inline function with a static
buffer would at least avoid that, though it
means it's not reentrant either.

I would personally prefer doing it that way though.

/* NB: returns pointer to buffer reused on each call */
static inline char *
xfs_buf_target_name(struct xfs_buftarg *target)
{
        static char __b[BDEVNAME_SIZE];

        return bdevname(target->bt_bdev, __b);
}


Anyway, you didn't change this, but you're touching
the code that uses it.  So unless others object I
would like to see this changed along with the
rest of what you do here (which is all good, by
the way).

Either way:

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family

[Chandra Seetharaman]

Thanks for the review Alex.

See below for comments.

On Fri, 2011-07-22 at 14:38 -0500, Alex Elder wrote:
> On Thu, 2011-07-21 at 17:32 -0700, Chandra Seetharaman wrote:
> > Remove the definitions and usage of the macros XFS_BUF_ERROR,
> > XFS_BUF_GETERROR and XFS_BUF_ISERROR.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> 
> Nice work on this.  It is clear it was thoughtfully
> done.
> 
> I have two things that need to be fixed.  If you do that
> you can consider this signed off by me.xfs_buf_geterror
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> . . .
> 
> > diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
> > index 837f311..e7e35fb 100644
> > --- a/fs/xfs/quota/xfs_dquot.c
> > +++ b/fs/xfs/quota/xfs_dquot.c
> > @@ -403,7 +403,8 @@ xfs_qm_dqalloc(
> >  			       dqp->q_blkno,
> >  			       mp->m_quotainfo->qi_dqchunklen,
> >  			       0);
> > -	if (!bp || (error = XFS_BUF_GETERROR(bp)))
> > +	error = xfs_buf_geterror(bp);
> > +	if (error)
> >  		goto error1;
> >  	/*
> >  	 * Make a chunk of dquots out of this buffer and log
> 
> This results in behavior that differs from before.
> Previously, error would have value 0 following
> the call to xfs_trans_get_buf() here, meaning that
> (at error1:) xfs_qm_dqalloc() would return 0 in
> this case.  Now it will return ENOMEM.
> 
> I think what you have done may be correct, but
> since the change does more than the simple
> macro transformation you intend, this change
> should be done in a separate commit.
> 
> So either:
> - post a new patch (preferably before this
>   whole series) that makes this code return
>   ENOMEM if xfs_trans_get_buf() returns a
>   null pointer, then update this patch accordingly;

Will it this way and resent the patch
xfs_buf_geterror
> - or just change this patch to return 0 instead
>   of  ENOMEM if xfs_trans_get_buf() returns a
>   null pointer.
> 
> . . .
> 
> > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> > index 88d1214..97daa35 100644
> > --- a/fs/xfs/xfs_vnodeops.c
> > +++ b/fs/xfs/xfs_vnodeops.c
> > @@ -83,7 +83,7 @@ xfs_readlink_bmap(
> >  
> >  		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
> >  				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);
> 
> xfs_buf_read() can return NULL here, so to match
> the existing behavior you should call xfs_buf_geterror()
> here.
> 
> > -		error = XFS_BUF_GETERROR(bp);
> > +		error = bp->b_error;
> >  		if (error) {
> >  			xfs_ioerror_alert("xfs_readlink",
> >  				  ip->i_mount, bp, XFS_BUF_ADDR(bp));

I did the change consciously. If bp were NULL, error would have been set
to ENOMEM, and xfs_ioerror_alert() and xfs_buf_relse(), would have
accessed bp and tripped anyways. So, I felt using the indirection
(xfs_buf_geterror()) is not adding any value, hence set error by
directly accessing b_error.

There are more place like these.

What do you think of this option

Leave this as is (with b_error), and send another patch to check for bp
after xfs_buf_read() in all places (if you want this option, what do you
think error should be set to, I see both EIO and ENOMEM used. I think it
should be the same always).

If you don't like that option I can revert to xfs_buf_geterror() too.

> 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Chandra Seetharaman]

On Fri, 2011-07-22 at 14:38 -0500, Alex Elder wrote:
> On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> > Remove the definition and usages of the macro XFS_BUF_SET_PTR.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> One suggestion.  Otherwise:
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> > diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> > index 4255a1c..21e770f 100644
> > --- a/fs/xfs/xfs_log.c
> > +++ b/fs/xfs/xfs_log.c
> > @@ -1395,8 +1395,9 @@ xlog_sync(xlog_t		*log,
> >  	if (split) {
> >  		bp = iclog->ic_log->l_xbuf;
> >  		XFS_BUF_SET_ADDR(bp, 0);	     /* logical 0 */
> > -		XFS_BUF_SET_PTR(bp, (xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> > -					    (__psint_t)count), split);
> > +		xfs_buf_associate_memory(bp,
> > +				(xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> > +				    (__psint_t)count), split);
> 
> If you just cast the address appropriately you can make this
> look a lot nicer:
> 
> 		xfs_buf_associate_memory(bp,
> 			(void *) &iclog->ic_header + count, split);
> 

will do
> >  		bp->b_fspriv = iclog;
> >  		bp->b_flags &= ~(XBF_FUA|XBF_FLUSH);
> >  		XFS_BUF_ASYNC(bp);
> 
> . . .
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned

[Chandra Seetharaman]

On Fri, 2011-07-22 at 14:38 -0500, Alex Elder wrote:
> On Thu, 2011-07-21 at 17:33 -0700, Chandra Seetharaman wrote:
> > Replace the macro XFS_BUF_ISPINNED with an inline helper function
> > xfs_buf_ispinned, and change all its usages.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> One simple suggestion below.  Otherwise:
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> > diff --git a/fs/xfs/linux-2.6/xfs_buf.h b/fs/xfs/linux-2.6/xfs_buf.h
> > index 7b1f484..71e1d6f 100644
> > --- a/fs/xfs/linux-2.6/xfs_buf.h
> > +++ b/fs/xfs/linux-2.6/xfs_buf.h
> > @@ -280,7 +280,10 @@ xfs_buf_set_ref(
> >  #define XFS_BUF_SET_VTYPE_REF(bp, type, ref)	xfs_buf_set_ref(bp, ref)
> >  #define XFS_BUF_SET_VTYPE(bp, type)		do { } while (0)
> >  
> > -#define XFS_BUF_ISPINNED(bp)	atomic_read(&((bp)->b_pin_count))
> > +static inline int xfs_buf_ispinned(struct xfs_buf *bp)
> > +{
> > +	return atomic_read(&(bp->b_pin_count));
> 
> Good idea.  But drop the extra parentheses:
> 
> 	return atomic_read(&bp->b_pin_count);

will do
> 
> > +}
> >  
> >  #define XFS_BUF_FINISH_IOWAIT(bp)	complete(&bp->b_iowait);
> >  
> 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family

[Alex Elder]

On Fri, 2011-07-22 at 13:49 -0700, Chandra Seetharaman wrote:
> Thanks for the review Alex.
> 
> See below for comments.
> 
> On Fri, 2011-07-22 at 14:38 -0500, Alex Elder wrote:
> > On Thu, 2011-07-21 at 17:32 -0700, Chandra Seetharaman wrote:
> > > Remove the definitions and usage of the macros XFS_BUF_ERROR,

. . .

> > > diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
> > > index 837f311..e7e35fb 100644
> > > --- a/fs/xfs/quota/xfs_dquot.c
> > > +++ b/fs/xfs/quota/xfs_dquot.c
> > > @@ -403,7 +403,8 @@ xfs_qm_dqalloc(
> > >  			       dqp->q_blkno,
> > >  			       mp->m_quotainfo->qi_dqchunklen,
> > >  			       0);
> > > -	if (!bp || (error = XFS_BUF_GETERROR(bp)))
> > > +	error = xfs_buf_geterror(bp);
> > > +	if (error)
> > >  		goto error1;
> > >  	/*
> > >  	 * Make a chunk of dquots out of this buffer and log
> > 
> > This results in behavior that differs from before.
> > Previously, error would have value 0 following
> > the call to xfs_trans_get_buf() here, meaning that
> > (at error1:) xfs_qm_dqalloc() would return 0 in
> > this case.  Now it will return ENOMEM.
> > 
> > I think what you have done may be correct, but
> > since the change does more than the simple
> > macro transformation you intend, this change
> > should be done in a separate commit.
> > 
> > So either:
> > - post a new patch (preferably before this
> >   whole series) that makes this code return
> >   ENOMEM if xfs_trans_get_buf() returns a
> >   null pointer, then update this patch accordingly;
> 
> Will it this way and resent the patch
> xfs_buf_geterror

I don't grok that "sentence" and I'm not sure whether
you are referring to the one above or below.

> > - or just change this patch to return 0 instead
> >   of  ENOMEM if xfs_trans_get_buf() returns a
> >   null pointer.
> > 
> > . . .
> > 
> > > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> > > index 88d1214..97daa35 100644
> > > --- a/fs/xfs/xfs_vnodeops.c
> > > +++ b/fs/xfs/xfs_vnodeops.c
> > > @@ -83,7 +83,7 @@ xfs_readlink_bmap(
> > >  
> > >  		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
> > >  				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);
> > 
> > xfs_buf_read() can return NULL here, so to match
> > the existing behavior you should call xfs_buf_geterror()
> > here.
> > 
> > > -		error = XFS_BUF_GETERROR(bp);
> > > +		error = bp->b_error;
> > >  		if (error) {
> > >  			xfs_ioerror_alert("xfs_readlink",
> > >  				  ip->i_mount, bp, XFS_BUF_ADDR(bp));
> 
> I did the change consciously. If bp were NULL, error would have been set
> to ENOMEM, and xfs_ioerror_alert() and xfs_buf_relse(), would have
> accessed bp and tripped anyways. So, I felt using the indirection
> (xfs_buf_geterror()) is not adding any value, hence set error by
> directly accessing b_error.

But you are dereferencing a possibly null pointer in the
code you added.  Yes, the code that was already there
should not dereference it either, but that's no excuse
for you to do it.  (And fix the other code while you're
there, or make a note to get it fixed later.)

The reason it's important here is that the value of error
gets passed back to the caller, and although I didn't
go very far back to see what effect it has, a quick look
showed that it might lead to different behavior.  As I
said, it might be *correct* behavior, but in any case it's
different, so it belongs in its own commit.

> There are more place like these.

I noticed you doing this sort of thing in a bunch of other
spots in your patch, and in all of them they seemed to
follow a test that ensured the buffer pointer was non-null
(or it was implicit, because some *prior* dereference of
the pointer would have been a problem) therefore simply
checking bp->b_error was a fine replacement.

But in this one spot, it's a bit different, so I called
attention to it.

If you are convinced I'm mistaken and this will produce
results identical to before, say so and I'll take a
closer look. 

> What do you think of this option
> 
> Leave this as is (with b_error), and send another patch to check for bp
> after xfs_buf_read() in all places (if you want this option, what do you
> think error should be set to, I see both EIO and ENOMEM used. I think it
> should be the same always).
> 
> If you don't like that option I can revert to xfs_buf_geterror() too.

I think using xfs_buf_geterror() is the easiest thing
to do right now.  Changing it such that xfs_readlink_bmap()
returns ENOMEM in the event xfs_buf_read() here returns a null
pointer sounds like a reasonable thing to do, but do it in
a separate patch that focuses on that change and why it's
correct.  And (despite what I said earlier) I guess do it
*after* we've got this series in.  I'm about ready to get
it committed once you get it updated.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Chandra Seetharaman]

On Fri, 2011-07-22 at 14:49 -0500, Alex Elder wrote:
> On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> > Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> 
> Wow, I hadn't looked at the definition of
> xfs_buf_target_name() before.  It's not safe
> (using a pointer to since-released stack space),
> though in practice it's going to be fine.
> 
> Defining it as an inline function with a static
> buffer would at least avoid that, though it
> means it's not reentrant either.
> 
> I would personally prefer doing it that way though.
> 
> /* NB: returns pointer to buffer reused on each call */
> static inline char *
> xfs_buf_target_name(struct xfs_buftarg *target)
> {
>         static char __b[BDEVNAME_SIZE];
> 
>         return bdevname(target->bt_bdev, __b);
> }
> 
> 
> Anyway, you didn't change this, but you're touching
> the code that uses it.  So unless others object I
> would like to see this changed along with the
> rest of what you do here (which is all good, by
> the way).
> 
> Either way:
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Chandra Seetharaman]

On Fri, 2011-07-22 at 14:23 -0700, Chandra Seetharaman wrote:
> On Fri, 2011-07-22 at 14:49 -0500, Alex Elder wrote:
> > On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> > > Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> > > 
> > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > 
> > Wow, I hadn't looked at the definition of
> > xfs_buf_target_name() before.  It's not safe
> > (using a pointer to since-released stack space),
> > though in practice it's going to be fine.
> > 
> > Defining it as an inline function with a static
> > buffer would at least avoid that, though it
> > means it's not reentrant either.
> > 
> > I would personally prefer doing it that way though.
> > 
> > /* NB: returns pointer to buffer reused on each call */
> > static inline char *
> > xfs_buf_target_name(struct xfs_buftarg *target)
> > {
> >         static char __b[BDEVNAME_SIZE];
> > 
> >         return bdevname(target->bt_bdev, __b);
> > }
> > 
> > 
> > Anyway, you didn't change this, but you're touching
> > the code that uses it.  So unless others object I
> > would like to see this changed along with the
> > rest of what you do here (which is all good, by
> > the way).

Will do it.
> > 
> > Either way:
> > 
> > Reviewed-by: Alex Elder <aelder@sgi.com>
> > 
> > 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family

[Alex Elder]

On Fri, 2011-07-22 at 16:10 -0500, Alex Elder wrote:
> On Fri, 2011-07-22 at 13:49 -0700, Chandra Seetharaman wrote:
> > Thanks for the review Alex.
> > 
> > See below for comments.
> > 
> > On Fri, 2011-07-22 at 14:38 -0500, Alex Elder wrote:
> > > On Thu, 2011-07-21 at 17:32 -0700, Chandra Seetharaman wrote:
> > > > Remove the definitions and usage of the macros XFS_BUF_ERROR,
> 
> . . .

Looking my message again, I think I may have gotten confused
along the way.

. . .

> > > > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> > > > index 88d1214..97daa35 100644
> > > > --- a/fs/xfs/xfs_vnodeops.c
> > > > +++ b/fs/xfs/xfs_vnodeops.c
> > > > @@ -83,7 +83,7 @@ xfs_readlink_bmap(
> > > >  
> > > >  		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
> > > >  				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);
> > > 
> > > xfs_buf_read() can return NULL here, so to match
> > > the existing behavior you should call xfs_buf_geterror()
> > > here.
> > > 
> > > > -		error = XFS_BUF_GETERROR(bp);
> > > > +		error = bp->b_error;
> > > >  		if (error) {
> > > >  			xfs_ioerror_alert("xfs_readlink",
> > > >  				  ip->i_mount, bp, XFS_BUF_ADDR(bp));
> > 
> > I did the change consciously. If bp were NULL, error would have been set
> > to ENOMEM, and xfs_ioerror_alert() and xfs_buf_relse(), would have
> > accessed bp and tripped anyways. So, I felt using the indirection
> > (xfs_buf_geterror()) is not adding any value, hence set error by
> > directly accessing b_error.
> 
> But you are dereferencing a possibly null pointer in the
> code you added.  Yes, the code that was already there
> should not dereference it either, but that's no excuse
> for you to do it.  (And fix the other code while you're
> there, or make a note to get it fixed later.)

The comment above I stand by.  But the next one I think
applies to another hunk of code.

In any case, hopefully you understand what my point
is and you'll be able to update your patch accordingly.

Sorry for the confusion.

					-Alex

> The reason it's important here is that the value of error
> gets passed back to the caller, and although I didn't
> go very far back to see what effect it has, a quick look
> showed that it might lead to different behavior.  As I
> said, it might be *correct* behavior, but in any case it's
> different, so it belongs in its own commit.
> 
> > There are more place like these.
> 
> I noticed you doing this sort of thing in a bunch of other
> spots in your patch, and in all of them they seemed to
> follow a test that ensured the buffer pointer was non-null
> (or it was implicit, because some *prior* dereference of
> the pointer would have been a problem) therefore simply
> checking bp->b_error was a fine replacement.
> 
> But in this one spot, it's a bit different, so I called
> attention to it.
> 
> If you are convinced I'm mistaken and this will produce
> results identical to before, say so and I'll take a
> closer look. 
> 
> > What do you think of this option
> > 
> > Leave this as is (with b_error), and send another patch to check for bp
> > after xfs_buf_read() in all places (if you want this option, what do you
> > think error should be set to, I see both EIO and ENOMEM used. I think it
> > should be the same always).
> > 
> > If you don't like that option I can revert to xfs_buf_geterror() too.
> 
> I think using xfs_buf_geterror() is the easiest thing
> to do right now.  Changing it such that xfs_readlink_bmap()
> returns ENOMEM in the event xfs_buf_read() here returns a null
> pointer sounds like a reasonable thing to do, but do it in
> a separate patch that focuses on that change and why it's
> correct.  And (despite what I said earlier) I guess do it
> *after* we've got this series in.  I'm about ready to get
> it committed once you get it updated.


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family

[Chandra Seetharaman]

On Fri, 2011-07-22 at 16:10 -0500, Alex Elder wrote:
> On Fri, 2011-07-22 at 13:49 -0700, Chandra Seetharaman wrote:
> > Thanks for the review Alex.
> > 
> > See below for comments.
> > 
> > On Fri, 2011-07-22 at 14:38 -0500, Alex Elder wrote:
> > > On Thu, 2011-07-21 at 17:32 -0700, Chandra Seetharaman wrote:
> > > > Remove the definitions and usage of the macros XFS_BUF_ERROR,
> 
> . . .
> 
> > > > diff --git a/fs/xfs/quota/xfs_dquot.c b/fs/xfs/quota/xfs_dquot.c
> > > > index 837f311..e7e35fb 100644
> > > > --- a/fs/xfs/quota/xfs_dquot.c
> > > > +++ b/fs/xfs/quota/xfs_dquot.c
> > > > @@ -403,7 +403,8 @@ xfs_qm_dqalloc(
> > > >  			       dqp->q_blkno,
> > > >  			       mp->m_quotainfo->qi_dqchunklen,
> > > >  			       0);
> > > > -	if (!bp || (error = XFS_BUF_GETERROR(bp)))
> > > > +	error = xfs_buf_geterror(bp);
> > > > +	if (error)
> > > >  		goto error1;
> > > >  	/*
> > > >  	 * Make a chunk of dquots out of this buffer and log
> > > 
> > > This results in behavior that differs from before.
> > > Previously, error would have value 0 following
> > > the call to xfs_trans_get_buf() here, meaning that
> > > (at error1:) xfs_qm_dqalloc() would return 0 in
> > > this case.  Now it will return ENOMEM.
> > > 
> > > I think what you have done may be correct, but
> > > since the change does more than the simple
> > > macro transformation you intend, this change
> > > should be done in a separate commit.
> > > 
> > > So either:
> > > - post a new patch (preferably before this
> > >   whole series) that makes this code return
> > >   ENOMEM if xfs_trans_get_buf() returns a
> > >   null pointer, then update this patch accordingly;
> > 
> > Will it this way and resent the patch
> > xfs_buf_geterror
> 
> I don't grok that "sentence" and I'm not sure whether
> you are referring to the one above or below.
> 

Sorry, something got eaten up...

I meant to say, "will do it this way and resend the patch".

> > > - or just change this patch to return 0 instead
> > >   of  ENOMEM if xfs_trans_get_buf() returns a
> > >   null pointer.
> > > 
> > > . . .
> > > 
> > > > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> > > > index 88d1214..97daa35 100644
> > > > --- a/fs/xfs/xfs_vnodeops.c
> > > > +++ b/fs/xfs/xfs_vnodeops.c
> > > > @@ -83,7 +83,7 @@ xfs_readlink_bmap(
> > > >  
> > > >  		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
> > > >  				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);
> > > 
> > > xfs_buf_read() can return NULL here, so to match
> > > the existing behavior you should call xfs_buf_geterror()
> > > here.
> > > 
> > > > -		error = XFS_BUF_GETERROR(bp);
> > > > +		error = bp->b_error;
> > > >  		if (error) {
> > > >  			xfs_ioerror_alert("xfs_readlink",
> > > >  				  ip->i_mount, bp, XFS_BUF_ADDR(bp));
> > 
> > I did the change consciously. If bp were NULL, error would have been set
> > to ENOMEM, and xfs_ioerror_alert() and xfs_buf_relse(), would have
> > accessed bp and tripped anyways. So, I felt using the indirection
> > (xfs_buf_geterror()) is not adding any value, hence set error by
> > directly accessing b_error.
> 
> But you are dereferencing a possibly null pointer in the
> code you added.  Yes, the code that was already there
> should not dereference it either, but that's no excuse
> for you to do it.  (And fix the other code while you're
> there, or make a note to get it fixed later.)
> 
> The reason it's important here is that the value of error
> gets passed back to the caller, and although I didn't
> go very far back to see what effect it has, a quick look
> showed that it might lead to different behavior.  As I
> said, it might be *correct* behavior, but in any case it's
> different, so it belongs in its own commit.
> 
> > There are more place like these.
> 
> I noticed you doing this sort of thing in a bunch of other
> spots in your patch, and in all of them they seemed to
> follow a test that ensured the buffer pointer was non-null
> (or it was implicit, because some *prior* dereference of
> the pointer would have been a problem) therefore simply
> checking bp->b_error was a fine replacement.
> 
> But in this one spot, it's a bit different, so I called
> attention to it.
> 
> If you are convinced I'm mistaken and this will produce
> results identical to before, say so and I'll take a
> closer look. 
> 
> > What do you think of this option
> > 
> > Leave this as is (with b_error), and send another patch to check for bp
> > after xfs_buf_read() in all places (if you want this option, what do you
> > think error should be set to, I see both EIO and ENOMEM used. I think it
> > should be the same always).
> > 
> > If you don't like that option I can revert to xfs_buf_geterror() too.
> 
> I think using xfs_buf_geterror() is the easiest thing
> to do right now.  Changing it such that xfs_readlink_bmap()
> returns ENOMEM in the event xfs_buf_read() here returns a null
> pointer sounds like a reasonable thing to do, but do it in
> a separate patch that focuses on that change and why it's
> correct.  And (despite what I said earlier) I guess do it
> *after* we've got this series in.  I'm about ready to get
> it committed once you get it updated.

I will do it the way you suggested and send a separate patch fixing the
incorrect dereferences.

Thanks

chandra
> 
> 					-Alex
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Christoph Hellwig]

On Fri, Jul 22, 2011 at 02:38:30PM -0500, Alex Elder wrote:
> > -					    (__psint_t)count), split);
> > +		xfs_buf_associate_memory(bp,
> > +				(xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> > +				    (__psint_t)count), split);
> 
> If you just cast the address appropriately you can make this
> look a lot nicer:
> 
> 		xfs_buf_associate_memory(bp,
> 			(void *) &iclog->ic_header + count, split);

If we have to cast anyway I'd suggest casting to char * at least, as
that is standard behaviour. I'm okay with using the gcc void pointer
arithmetics extension when it saves us ugliness, but in this case it
doesn't really buy us anything.

Btw, nice follow-on cleanups would be to kill off the xfs_caddr_t and
__psint_t/__psunsigned_t types entirely.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Christoph Hellwig]

On Fri, Jul 22, 2011 at 02:49:41PM -0500, Alex Elder wrote:
> On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> > Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> > 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> 
> Wow, I hadn't looked at the definition of
> xfs_buf_target_name() before.  It's not safe
> (using a pointer to since-released stack space),
> though in practice it's going to be fine.
> 
> Defining it as an inline function with a static
> buffer would at least avoid that, though it
> means it's not reentrant either.

IMHO the right fix is to just kill it off entirely.  All XFS messages
now have the filesystem name prefixed to them, and while we can have
up to three devices, all these error messages can only hit either
the main or the log device, and it's obvious from the context which
one we did hit.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Alex Elder]

On Sun, 2011-07-24 at 07:35 -0400, Christoph Hellwig wrote:
> On Fri, Jul 22, 2011 at 02:38:30PM -0500, Alex Elder wrote:
> > > -					    (__psint_t)count), split);
> > > +		xfs_buf_associate_memory(bp,
> > > +				(xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> > > +				    (__psint_t)count), split);
> > 
> > If you just cast the address appropriately you can make this
> > look a lot nicer:
> > 
> > 		xfs_buf_associate_memory(bp,
> > 			(void *) &iclog->ic_header + count, split);
> 
> If we have to cast anyway I'd suggest casting to char * at least, as
> that is standard behaviour. I'm okay with using the gcc void pointer
> arithmetics extension when it saves us ugliness, but in this case it
> doesn't really buy us anything.

Yes, I agree.  I guess I had the "void *" addition on the brain
when I did this.  Chandra, I would like to make this small change
before I commit this.  OK with you?

> Btw, nice follow-on cleanups would be to kill off the xfs_caddr_t and
> __psint_t/__psunsigned_t types entirely.

Yes.  What do you suggest to use for a pointer-sized
type?  Just cast to (long) and rely on the usual arithmetic
conversions to do the right thing?

					-Alex


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Alex Elder]

On Sun, 2011-07-24 at 07:37 -0400, Christoph Hellwig wrote:
> On Fri, Jul 22, 2011 at 02:49:41PM -0500, Alex Elder wrote:
> > On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> > > Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> > > 
> > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > 
> > Wow, I hadn't looked at the definition of
> > xfs_buf_target_name() before.  It's not safe
> > (using a pointer to since-released stack space),
> > though in practice it's going to be fine.
> > 
> > Defining it as an inline function with a static
> > buffer would at least avoid that, though it
> > means it's not reentrant either.
> 
> IMHO the right fix is to just kill it off entirely.  All XFS messages
> now have the filesystem name prefixed to them, and while we can have
> up to three devices, all these error messages can only hit either
> the main or the log device, and it's obvious from the context which
> one we did hit.

That's an even better idea.  I was only reacting to the
code in front of me, but yes, removing it entirely
would be good.

For now though, I intend to commit this (in its now updated
form).  It can be removed as a separate patch.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Christoph Hellwig]

On Mon, Jul 25, 2011 at 10:57:36AM -0500, Alex Elder wrote:
> > Btw, nice follow-on cleanups would be to kill off the xfs_caddr_t and
> > __psint_t/__psunsigned_t types entirely.
> 
> Yes.  What do you suggest to use for a pointer-sized
> type?  Just cast to (long) and rely on the usual arithmetic
> conversions to do the right thing?

The correct C99 type is (u)intptr_t.  In the kernel we only have
uintptr_t, with ACPI defining a local version of intptr_t.  I'd suggest
trying to stick to uintptr_t if we can, and if we really need a signed
version add intptr_t to the common headers - it can unconditionally be
typedef to long in Linux anyway.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Christoph Hellwig]

On Mon, Jul 25, 2011 at 10:57:40AM -0500, Alex Elder wrote:
> That's an even better idea.  I was only reacting to the
> code in front of me, but yes, removing it entirely
> would be good.
> 
> For now though, I intend to commit this (in its now updated
> form).  It can be removed as a separate patch.

Agreed.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Alex Elder]

On Mon, 2011-07-25 at 12:25 -0400, Christoph Hellwig wrote:
> On Mon, Jul 25, 2011 at 10:57:36AM -0500, Alex Elder wrote:
> > > Btw, nice follow-on cleanups would be to kill off the xfs_caddr_t and
> > > __psint_t/__psunsigned_t types entirely.
> > 
> > Yes.  What do you suggest to use for a pointer-sized
> > type?  Just cast to (long) and rely on the usual arithmetic
> > conversions to do the right thing?
> 
> The correct C99 type is (u)intptr_t.  In the kernel we only have
> uintptr_t, with ACPI defining a local version of intptr_t.  I'd suggest
> trying to stick to uintptr_t if we can, and if we really need a signed
> version add intptr_t to the common headers - it can unconditionally be
> typedef to long in Linux anyway.

I *knew* there was a standard type but I couldn't
remember what it was called.  I even scanned through
my C spec but gave up looking pretty quickly.  I
think I had "size" somehow stuck in my mind and that
didn't help.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR

[Chandra Seetharaman]

On Mon, 2011-07-25 at 10:57 -0500, Alex Elder wrote:
> On Sun, 2011-07-24 at 07:35 -0400, Christoph Hellwig wrote:
> > On Fri, Jul 22, 2011 at 02:38:30PM -0500, Alex Elder wrote:
> > > > -					    (__psint_t)count), split);
> > > > +		xfs_buf_associate_memory(bp,
> > > > +				(xfs_caddr_t)((__psint_t)&(iclog->ic_header)+
> > > > +				    (__psint_t)count), split);
> > > 
> > > If you just cast the address appropriately you can make this
> > > look a lot nicer:
> > > 
> > > 		xfs_buf_associate_memory(bp,
> > > 			(void *) &iclog->ic_header + count, split);
> > 
> > If we have to cast anyway I'd suggest casting to char * at least, as
> > that is standard behaviour. I'm okay with using the gcc void pointer
> > arithmetics extension when it saves us ugliness, but in this case it
> > doesn't really buy us anything.
> 
> Yes, I agree.  I guess I had the "void *" addition on the brain
> when I did this.  Chandra, I would like to make this small change
> before I commit this.  OK with you?
> 

Yes.
> > Btw, nice follow-on cleanups would be to kill off the xfs_caddr_t and
> > __psint_t/__psunsigned_t types entirely.
> 
> Yes.  What do you suggest to use for a pointer-sized
> type?  Just cast to (long) and rely on the usual arithmetic
> conversions to do the right thing?
> 
> 					-Alex
> 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME

[Chandra Seetharaman]

On Mon, 2011-07-25 at 10:57 -0500, Alex Elder wrote:
> On Sun, 2011-07-24 at 07:37 -0400, Christoph Hellwig wrote:
> > On Fri, Jul 22, 2011 at 02:49:41PM -0500, Alex Elder wrote:
> > > On Thu, 2011-07-21 at 17:34 -0700, Chandra Seetharaman wrote:
> > > > Remove the definition and usages of the macro XFS_BUFTARG_NAME.
> > > > 
> > > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > 
> > > Wow, I hadn't looked at the definition of
> > > xfs_buf_target_name() before.  It's not safe
> > > (using a pointer to since-released stack space),
> > > though in practice it's going to be fine.
> > > 
> > > Defining it as an inline function with a static
> > > buffer would at least avoid that, though it
> > > means it's not reentrant either.
> > 
> > IMHO the right fix is to just kill it off entirely.  All XFS messages
> > now have the filesystem name prefixed to them, and while we can have
> > up to three devices, all these error messages can only hit either
> > the main or the log device, and it's obvious from the context which
> > one we did hit.
> 
> That's an even better idea.  I was only reacting to the
> code in front of me, but yes, removing it entirely
> would be good.
> 
> For now though, I intend to commit this (in its now updated
> form).  It can be removed as a separate patch.

I will do it.

chandra
> 
> 					-Alex
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs