* [PATCH 01/12] xfs: Remove the macro XFS_BUF_BFLAGS
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
@ 2011-07-22 0:32 ` Chandra Seetharaman
2011-07-22 19:37 ` Alex Elder
2011-07-22 0:32 ` [PATCH 02/12] xfs: Remove the macro XFS_BUF_ZEROFLAGS Chandra Seetharaman
` (10 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:32 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 02/12] xfs: Remove the macro XFS_BUF_ZEROFLAGS
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
2011-07-22 0:32 ` [PATCH 01/12] xfs: Remove the macro XFS_BUF_BFLAGS Chandra Seetharaman
@ 2011-07-22 0:32 ` Chandra Seetharaman
2011-07-22 0:32 ` [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family Chandra Seetharaman
` (9 subsequent siblings)
11 siblings, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:32 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
2011-07-22 0:32 ` [PATCH 01/12] xfs: Remove the macro XFS_BUF_BFLAGS Chandra Seetharaman
2011-07-22 0:32 ` [PATCH 02/12] xfs: Remove the macro XFS_BUF_ZEROFLAGS Chandra Seetharaman
@ 2011-07-22 0:32 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 04/12] xfs: Remove macro XFS_BUF_BUSY " Chandra Seetharaman
` (8 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:32 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family
2011-07-22 0:32 ` [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family Chandra Seetharaman
@ 2011-07-22 19:38 ` Alex Elder
2011-07-22 20:49 ` Chandra Seetharaman
0 siblings, 1 reply; 44+ messages in thread
From: Alex Elder @ 2011-07-22 19:38 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family
2011-07-22 19:38 ` Alex Elder
@ 2011-07-22 20:49 ` Chandra Seetharaman
2011-07-22 21:10 ` Alex Elder
0 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 20:49 UTC (permalink / raw)
To: aelder; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family
2011-07-22 20:49 ` Chandra Seetharaman
@ 2011-07-22 21:10 ` Alex Elder
2011-07-22 21:30 ` Alex Elder
2011-07-22 21:31 ` Chandra Seetharaman
0 siblings, 2 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-22 21:10 UTC (permalink / raw)
To: sekharan; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family
2011-07-22 21:10 ` Alex Elder
@ 2011-07-22 21:30 ` Alex Elder
2011-07-22 21:31 ` Chandra Seetharaman
1 sibling, 0 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-22 21:30 UTC (permalink / raw)
To: sekharan; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family
2011-07-22 21:10 ` Alex Elder
2011-07-22 21:30 ` Alex Elder
@ 2011-07-22 21:31 ` Chandra Seetharaman
1 sibling, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 21:31 UTC (permalink / raw)
To: aelder; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 04/12] xfs: Remove macro XFS_BUF_BUSY and family
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (2 preceding siblings ...)
2011-07-22 0:32 ` [PATCH 03/12] xfs: Remove the macro XFS_BUF_ERROR and family Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 05/12] xfs: Remove macro XFS_BUF_HOLD Chandra Seetharaman
` (7 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 05/12] xfs: Remove macro XFS_BUF_HOLD
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (3 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 04/12] xfs: Remove macro XFS_BUF_BUSY " Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START Chandra Seetharaman
` (6 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (4 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 05/12] xfs: Remove macro XFS_BUF_HOLD Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR Chandra Seetharaman
` (5 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (5 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 06/12] xfs: Remove macro XFS_BUF_SET_START Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR Chandra Seetharaman
` (4 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR
2011-07-22 0:33 ` [PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR Chandra Seetharaman
@ 2011-07-22 19:38 ` Alex Elder
0 siblings, 0 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-22 19:38 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (6 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 07/12] xfs: Remove the macro XFS_BUF_PTR Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned Chandra Seetharaman
` (3 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-22 0:33 ` [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR Chandra Seetharaman
@ 2011-07-22 19:38 ` Alex Elder
2011-07-22 20:50 ` Chandra Seetharaman
2011-07-24 11:35 ` Christoph Hellwig
0 siblings, 2 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-22 19:38 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-22 19:38 ` Alex Elder
@ 2011-07-22 20:50 ` Chandra Seetharaman
2011-07-24 11:35 ` Christoph Hellwig
1 sibling, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 20:50 UTC (permalink / raw)
To: aelder; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-22 19:38 ` Alex Elder
2011-07-22 20:50 ` Chandra Seetharaman
@ 2011-07-24 11:35 ` Christoph Hellwig
2011-07-25 15:57 ` Alex Elder
1 sibling, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2011-07-24 11:35 UTC (permalink / raw)
To: Alex Elder; +Cc: Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-24 11:35 ` Christoph Hellwig
@ 2011-07-25 15:57 ` Alex Elder
2011-07-25 16:25 ` Christoph Hellwig
2011-07-25 22:18 ` Chandra Seetharaman
0 siblings, 2 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-25 15:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-25 15:57 ` Alex Elder
@ 2011-07-25 16:25 ` Christoph Hellwig
2011-07-25 16:58 ` Alex Elder
2011-07-25 22:18 ` Chandra Seetharaman
1 sibling, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2011-07-25 16:25 UTC (permalink / raw)
To: Alex Elder; +Cc: Christoph Hellwig, Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-25 16:25 ` Christoph Hellwig
@ 2011-07-25 16:58 ` Alex Elder
0 siblings, 0 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-25 16:58 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR
2011-07-25 15:57 ` Alex Elder
2011-07-25 16:25 ` Christoph Hellwig
@ 2011-07-25 22:18 ` Chandra Seetharaman
1 sibling, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-25 22:18 UTC (permalink / raw)
To: aelder; +Cc: Christoph Hellwig, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (7 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 08/12] xfs: Remove the macro XFS_BUF_SET_PTR Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:33 ` [PATCH 10/12] xfs: Remove the macro XFS_BUF_SET_TARGET Chandra Seetharaman
` (2 subsequent siblings)
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned
2011-07-22 0:33 ` [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned Chandra Seetharaman
@ 2011-07-22 19:38 ` Alex Elder
2011-07-22 20:51 ` Chandra Seetharaman
0 siblings, 1 reply; 44+ messages in thread
From: Alex Elder @ 2011-07-22 19:38 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned
2011-07-22 19:38 ` Alex Elder
@ 2011-07-22 20:51 ` Chandra Seetharaman
0 siblings, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 20:51 UTC (permalink / raw)
To: aelder; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 10/12] xfs: Remove the macro XFS_BUF_SET_TARGET
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (8 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 09/12] Replace the macro XFS_BUF_ISPINNED with helper xfs_buf_ispinned Chandra Seetharaman
@ 2011-07-22 0:33 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 0:34 ` [PATCH 11/12] xfs: Remove the macro XFS_BUF_TARGET Chandra Seetharaman
2011-07-22 0:34 ` [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME Chandra Seetharaman
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:33 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 11/12] xfs: Remove the macro XFS_BUF_TARGET
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (9 preceding siblings ...)
2011-07-22 0:33 ` [PATCH 10/12] xfs: Remove the macro XFS_BUF_SET_TARGET Chandra Seetharaman
@ 2011-07-22 0:34 ` Chandra Seetharaman
2011-07-22 19:38 ` Alex Elder
2011-07-22 19:46 ` Alex Elder
2011-07-22 0:34 ` [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME Chandra Seetharaman
11 siblings, 2 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:34 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-22 0:32 [PATCH 00/12] Remove number of macros from xfs_buf.h Chandra Seetharaman
` (10 preceding siblings ...)
2011-07-22 0:34 ` [PATCH 11/12] xfs: Remove the macro XFS_BUF_TARGET Chandra Seetharaman
@ 2011-07-22 0:34 ` Chandra Seetharaman
2011-07-22 19:49 ` Alex Elder
11 siblings, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 0:34 UTC (permalink / raw)
To: xfs; +Cc: 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-22 0:34 ` [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME Chandra Seetharaman
@ 2011-07-22 19:49 ` Alex Elder
2011-07-22 21:23 ` Chandra Seetharaman
2011-07-24 11:37 ` Christoph Hellwig
0 siblings, 2 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-22 19:49 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-22 19:49 ` Alex Elder
@ 2011-07-22 21:23 ` Chandra Seetharaman
2011-07-22 21:26 ` Chandra Seetharaman
2011-07-24 11:37 ` Christoph Hellwig
1 sibling, 1 reply; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 21:23 UTC (permalink / raw)
To: aelder; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-22 21:23 ` Chandra Seetharaman
@ 2011-07-22 21:26 ` Chandra Seetharaman
0 siblings, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-22 21:26 UTC (permalink / raw)
To: aelder; +Cc: xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-22 19:49 ` Alex Elder
2011-07-22 21:23 ` Chandra Seetharaman
@ 2011-07-24 11:37 ` Christoph Hellwig
2011-07-25 15:57 ` Alex Elder
1 sibling, 1 reply; 44+ messages in thread
From: Christoph Hellwig @ 2011-07-24 11:37 UTC (permalink / raw)
To: Alex Elder; +Cc: Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-24 11:37 ` Christoph Hellwig
@ 2011-07-25 15:57 ` Alex Elder
2011-07-25 16:26 ` Christoph Hellwig
2011-07-25 22:18 ` Chandra Seetharaman
0 siblings, 2 replies; 44+ messages in thread
From: Alex Elder @ 2011-07-25 15:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-25 15:57 ` Alex Elder
@ 2011-07-25 16:26 ` Christoph Hellwig
2011-07-25 22:18 ` Chandra Seetharaman
1 sibling, 0 replies; 44+ messages in thread
From: Christoph Hellwig @ 2011-07-25 16:26 UTC (permalink / raw)
To: Alex Elder; +Cc: Christoph Hellwig, Chandra Seetharaman, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 12/12] xfs: Remove the macro XFS_BUFTARG_NAME
2011-07-25 15:57 ` Alex Elder
2011-07-25 16:26 ` Christoph Hellwig
@ 2011-07-25 22:18 ` Chandra Seetharaman
1 sibling, 0 replies; 44+ messages in thread
From: Chandra Seetharaman @ 2011-07-25 22:18 UTC (permalink / raw)
To: aelder; +Cc: Christoph Hellwig, xfs
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
^ permalink raw reply [flat|nested] 44+ messages in thread