Skip to contentSkip to conversation

xfs: clean up buffer flag macros

PATCH In review Dave Chinner 2 participants 19 messages Feb 5, 2016 – Feb 9, 2016 XFS filesystem

lore
Share & tools
lore

Dave Chinner

author
#0 lore raw

cover xfs: clean up buffer flag macros

Hi folks,

This series gets rid of the remaining XFS_BUF flag macros. We've removed most of them over the past few years as we've made changes, with only a few now remaining. May as well get rid of them for good...

-Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#1 lore raw

PATCH 1/6 xfs: remove XBF_DONE flag wrapper macros

From: Dave Chinner <[email protected]>

They only set/clear/check a flag, no need for obfuscating this with a macro.

Signed-off-by Dave Chinner <[email protected]>
---
 fs/xfs/xfs_buf.c         | 2 +-
 fs/xfs/xfs_buf.h         | 4 ----
 fs/xfs/xfs_buf_item.c    | 6 +++---
 fs/xfs/xfs_inode.c       | 2 +-
 fs/xfs/xfs_log.c         | 4 ++--
 fs/xfs/xfs_log_recover.c | 2 +-
 fs/xfs/xfs_mount.c       | 2 +-
 fs/xfs/xfs_trans_buf.c   | 4 ++--
 8 files changed, 11 insertions(+), 15 deletions(-)
fs/xfs/xfs_buf.c +1 -1
index 3c61547..5f7ff50 100644
@@ -650,7 +650,7 @@ xfs_buf_read_map(
 	if (bp) {
 		trace_xfs_buf_read(bp, flags, _RET_IP_);
 
-		if (!XFS_BUF_ISDONE(bp)) {
+		if (!(bp->b_flags & XBF_DONE)) {
 			XFS_STATS_INC(target->bt_mount, xb_get_read);
 			bp->b_ops = ops;
 			_xfs_buf_read(bp, flags);
fs/xfs/xfs_buf.h +0 -4
index c75721a..03b5d3a 100644
@@ -321,10 +321,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNSTALE(bp)	((bp)->b_flags &= ~XBF_STALE)
 #define XFS_BUF_ISSTALE(bp)	((bp)->b_flags & XBF_STALE)
 
-#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)
-
 #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)
fs/xfs/xfs_buf_item.c +3 -3
index 82f676f..2d2029d 100644
@@ -470,7 +470,7 @@ xfs_buf_item_unpin(
 		xfs_buf_hold(bp);
 		bp->b_flags |= XBF_ASYNC;
 		xfs_buf_ioerror(bp, -EIO);
-		XFS_BUF_UNDONE(bp);
+		bp->b_flags &= ~XBF_DONE;
 		xfs_buf_stale(bp);
 		xfs_buf_ioend(bp);
 	}

@@ -951,7 +951,7 @@ xfs_buf_iodone_callbacks(
 	 */
 	if (XFS_FORCED_SHUTDOWN(mp)) {
 		xfs_buf_stale(bp);
-		XFS_BUF_DONE(bp);
+		bp->b_flags |= XBF_DONE;
 		trace_xfs_buf_item_iodone(bp, _RET_IP_);
 		goto do_callbacks;
 	}

@@ -997,7 +997,7 @@ xfs_buf_iodone_callbacks(
 	 * sure to return the error to the caller of xfs_bwrite().
 	 */
 	xfs_buf_stale(bp);
-	XFS_BUF_DONE(bp);
+	bp->b_flags |= XBF_DONE;
 
 	trace_xfs_buf_error_relse(bp, _RET_IP_);
fs/xfs/xfs_inode.c +1 -1
index 205796f..415c70a 100644
@@ -3297,7 +3297,7 @@ cluster_corrupt_out:
 		 * mark it as stale and brelse.
 		 */
 		if (bp->b_iodone) {
-			XFS_BUF_UNDONE(bp);
+			bp->b_flags &= ~XBF_DONE;
 			xfs_buf_stale(bp);
 			xfs_buf_ioerror(bp, -EIO);
 			xfs_buf_ioend(bp);
fs/xfs/xfs_log.c +2 -2
index 9c9a1c9..aa8c9bf 100644
@@ -3979,7 +3979,7 @@ xfs_log_force_umount(
 	    log->l_flags & XLOG_ACTIVE_RECOVERY) {
 		mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
 		if (mp->m_sb_bp)
-			XFS_BUF_DONE(mp->m_sb_bp);
+			mp->m_sb_bp->b_flags |= XBF_DONE;
 		return 0;
 	}
 

@@ -4009,7 +4009,7 @@ xfs_log_force_umount(
 	spin_lock(&log->l_icloglock);
 	mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
 	if (mp->m_sb_bp)
-		XFS_BUF_DONE(mp->m_sb_bp);
+		mp->m_sb_bp->b_flags |= XBF_DONE;
 
 	/*
 	 * Mark the log and the iclogs with IO error flags to prevent any
fs/xfs/xfs_log_recover.c +1 -1
index bd6f23b..97cfeab 100644
@@ -4926,7 +4926,7 @@ xlog_do_recover(
 	 * updates, re-read in the superblock and reverify it.
 	 */
 	bp = xfs_getsb(log->l_mp, 0);
-	XFS_BUF_UNDONE(bp);
+	bp->b_flags &= ~XBF_DONE;
 	ASSERT(!(XFS_BUF_ISWRITE(bp)));
 	XFS_BUF_READ(bp);
 	XFS_BUF_UNASYNC(bp);
fs/xfs/xfs_mount.c +1 -1
index d306105..986290c 100644
@@ -1284,7 +1284,7 @@ xfs_getsb(
 	}
 
 	xfs_buf_hold(bp);
-	ASSERT(XFS_BUF_ISDONE(bp));
+	ASSERT(bp->b_flags & XBF_DONE);
 	return bp;
 }
fs/xfs/xfs_trans_buf.c +2 -2
index 7579841..ed6f61f 100644
@@ -155,7 +155,7 @@ xfs_trans_get_buf_map(
 		ASSERT(xfs_buf_islocked(bp));
 		if (XFS_FORCED_SHUTDOWN(tp->t_mountp)) {
 			xfs_buf_stale(bp);
-			XFS_BUF_DONE(bp);
+			bp->b_flags |= XBF_DONE;
 		}
 
 		ASSERT(bp->b_transp == tp);

@@ -518,7 +518,7 @@ xfs_trans_log_buf(xfs_trans_t	*tp,
 	 * inside the b_bdstrat callback so that this won't get written to
 	 * disk.
 	 */
-	XFS_BUF_DONE(bp);
+	bp->b_flags |= XBF_DONE;
 
 	ASSERT(atomic_read(&bip->bli_refcount) > 0);
 	bp->b_iodone = xfs_buf_iodone_callbacks;
Signature · 6 lines
2.5.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#3 lore raw

PATCH 2/6 xfs: remove XBF_ASYNC flag wrapper macros

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

They only set/clear/check a flag, no need for obfuscating this with a macro.

Signed-off-by Dave Chinner <[email protected]>
---
 fs/xfs/xfs_buf.h         | 4 ----
 fs/xfs/xfs_buf_item.c    | 2 +-
 fs/xfs/xfs_log.c         | 8 +++-----
 fs/xfs/xfs_log_recover.c | 3 +--
 4 files changed, 5 insertions(+), 12 deletions(-)
fs/xfs/xfs_buf.h +0 -4
index 03b5d3a..2a28d1c 100644
@@ -321,10 +321,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNSTALE(bp)	((bp)->b_flags &= ~XBF_STALE)
 #define XFS_BUF_ISSTALE(bp)	((bp)->b_flags & XBF_STALE)
 
-#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)
-
 #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)
fs/xfs/xfs_buf_item.c +1 -1
index 2d2029d..344ab44 100644
@@ -974,7 +974,7 @@ xfs_buf_iodone_callbacks(
 	 * errors tend to affect the whole device and a failing log write
 	 * will make us give up.  But we really ought to do better here.
 	 */
-	if (XFS_BUF_ISASYNC(bp)) {
+	if (bp->b_flags & XBF_ASYNC) {
 		ASSERT(bp->b_iodone != NULL);
 
 		trace_xfs_buf_item_iodone_async(bp, _RET_IP_);
fs/xfs/xfs_log.c +3 -5
index aa8c9bf..19db3da 100644
@@ -1212,7 +1212,7 @@ xlog_iodone(xfs_buf_t *bp)
 	}
 
 	/* log I/O is always issued ASYNC */
-	ASSERT(XFS_BUF_ISASYNC(bp));
+	ASSERT(bp->b_flags & XBF_ASYNC);
 	xlog_state_done_syncing(iclog, aborted);
 
 	/*

@@ -1865,8 +1865,7 @@ xlog_sync(
 	bp->b_io_length = BTOBB(count);
 	bp->b_fspriv = iclog;
 	XFS_BUF_ZEROFLAGS(bp);
-	XFS_BUF_ASYNC(bp);
-	bp->b_flags |= XBF_SYNCIO;
+	bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO);
 
 	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) {
 		bp->b_flags |= XBF_FUA;

@@ -1911,8 +1910,7 @@ xlog_sync(
 				(char *)&iclog->ic_header + count, split);
 		bp->b_fspriv = iclog;
 		XFS_BUF_ZEROFLAGS(bp);
-		XFS_BUF_ASYNC(bp);
-		bp->b_flags |= XBF_SYNCIO;
+		bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO);
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 			bp->b_flags |= XBF_FUA;
fs/xfs/xfs_log_recover.c +1 -2
index 97cfeab..b34337d 100644
@@ -4926,10 +4926,9 @@ xlog_do_recover(
 	 * updates, re-read in the superblock and reverify it.
 	 */
 	bp = xfs_getsb(log->l_mp, 0);
-	bp->b_flags &= ~XBF_DONE;
+	bp->b_flags &= ~(XBF_DONE | XBF_ASYNC);
 	ASSERT(!(XFS_BUF_ISWRITE(bp)));
 	XFS_BUF_READ(bp);
-	XFS_BUF_UNASYNC(bp);
 	bp->b_ops = &xfs_sb_buf_ops;
 
 	error = xfs_buf_submit_wait(bp);
Signature · 6 lines
2.5.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#5 lore raw

PATCH 3/6 xfs: remove XBF_READ flag wrapper macros

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

They only set/clear/check a flag, no need for obfuscating this with a macro.

Signed-off-by Dave Chinner <[email protected]>
---
 fs/xfs/xfs_buf.h         | 4 ----
 fs/xfs/xfs_log_recover.c | 4 ++--
 2 files changed, 2 insertions(+), 6 deletions(-)
fs/xfs/xfs_buf.h +0 -4
index 2a28d1c..329e612 100644
@@ -321,10 +321,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNSTALE(bp)	((bp)->b_flags &= ~XBF_STALE)
 #define XFS_BUF_ISSTALE(bp)	((bp)->b_flags & XBF_STALE)
 
-#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)
-
 #define XFS_BUF_WRITE(bp)	((bp)->b_flags |= XBF_WRITE)
 #define XFS_BUF_UNWRITE(bp)	((bp)->b_flags &= ~XBF_WRITE)
 #define XFS_BUF_ISWRITE(bp)	((bp)->b_flags & XBF_WRITE)
fs/xfs/xfs_log_recover.c +2 -2
index b34337d..81c525a 100644
@@ -190,7 +190,7 @@ xlog_bread_noalign(
 	ASSERT(nbblks <= bp->b_length);
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
-	XFS_BUF_READ(bp);
+	bp->b_flags |= XBF_READ;
 	bp->b_io_length = nbblks;
 	bp->b_error = 0;
 

@@ -4928,7 +4928,7 @@ xlog_do_recover(
 	bp = xfs_getsb(log->l_mp, 0);
 	bp->b_flags &= ~(XBF_DONE | XBF_ASYNC);
 	ASSERT(!(XFS_BUF_ISWRITE(bp)));
-	XFS_BUF_READ(bp);
+	bp->b_flags |= XBF_READ;
 	bp->b_ops = &xfs_sb_buf_ops;
 
 	error = xfs_buf_submit_wait(bp);
Signature · 6 lines
2.5.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#7 lore raw

PATCH 4/6 xfs: remove XBF_WRITE flag wrapper macros

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

They only set/clear/check a flag, no need for obfuscating this with a macro.

Signed-off-by Dave Chinner <[email protected]>
---
 fs/xfs/xfs_buf.h         | 4 ----
 fs/xfs/xfs_log.c         | 8 +++-----
 fs/xfs/xfs_log_recover.c | 2 +-
 3 files changed, 4 insertions(+), 10 deletions(-)
fs/xfs/xfs_buf.h +0 -4
index 329e612..a118962 100644
@@ -321,10 +321,6 @@ void xfs_buf_stale(struct xfs_buf *bp);
 #define XFS_BUF_UNSTALE(bp)	((bp)->b_flags &= ~XBF_STALE)
 #define XFS_BUF_ISSTALE(bp)	((bp)->b_flags & XBF_STALE)
 
-#define XFS_BUF_WRITE(bp)	((bp)->b_flags |= XBF_WRITE)
-#define XFS_BUF_UNWRITE(bp)	((bp)->b_flags &= ~XBF_WRITE)
-#define XFS_BUF_ISWRITE(bp)	((bp)->b_flags & XBF_WRITE)
-
 /*
  * These macros use the IO block map rather than b_bn. b_bn is now really
  * just for the buffer cache index for cached buffers. As IO does not use b_bn
fs/xfs/xfs_log.c +3 -5
index 19db3da..edf20b2 100644
@@ -1865,7 +1865,7 @@ xlog_sync(
 	bp->b_io_length = BTOBB(count);
 	bp->b_fspriv = iclog;
 	XFS_BUF_ZEROFLAGS(bp);
-	bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO);
+	bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 
 	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) {
 		bp->b_flags |= XBF_FUA;

@@ -1892,12 +1892,11 @@ xlog_sync(
 
 	/* account for log which doesn't start at block #0 */
 	XFS_BUF_SET_ADDR(bp, XFS_BUF_ADDR(bp) + log->l_logBBstart);
+
 	/*
 	 * Don't call xfs_bwrite here. We do log-syncs even when the filesystem
 	 * is shutting down.
 	 */
-	XFS_BUF_WRITE(bp);
-
 	error = xlog_bdstrat(bp);
 	if (error) {
 		xfs_buf_ioerror_alert(bp, "xlog_sync");

@@ -1910,7 +1909,7 @@ xlog_sync(
 				(char *)&iclog->ic_header + count, split);
 		bp->b_fspriv = iclog;
 		XFS_BUF_ZEROFLAGS(bp);
-		bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO);
+		bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 			bp->b_flags |= XBF_FUA;
 

@@ -1919,7 +1918,6 @@ xlog_sync(
 
 		/* account for internal log which doesn't start at block #0 */
 		XFS_BUF_SET_ADDR(bp, XFS_BUF_ADDR(bp) + log->l_logBBstart);
-		XFS_BUF_WRITE(bp);
 		error = xlog_bdstrat(bp);
 		if (error) {
 			xfs_buf_ioerror_alert(bp, "xlog_sync (split)");
fs/xfs/xfs_log_recover.c +1 -1
index 81c525a..68406fd 100644
@@ -4927,7 +4927,7 @@ xlog_do_recover(
 	 */
 	bp = xfs_getsb(log->l_mp, 0);
 	bp->b_flags &= ~(XBF_DONE | XBF_ASYNC);
-	ASSERT(!(XFS_BUF_ISWRITE(bp)));
+	ASSERT(!(bp->b_flags & XBF_WRITE));
 	bp->b_flags |= XBF_READ;
 	bp->b_ops = &xfs_sb_buf_ops;
Signature · 6 lines
2.5.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#9 lore raw

PATCH 5/6 xfs: remove XBF_STALE flag wrapper macros

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

They only set/clear/check a flag, no need for obfuscating this with a macro.

Signed-off-by Dave Chinner <[email protected]>
---
 fs/xfs/xfs_buf.h       | 4 +---
 fs/xfs/xfs_buf_item.c  | 2 +-
 fs/xfs/xfs_trans_buf.c | 6 +++---
 3 files changed, 5 insertions(+), 7 deletions(-)
fs/xfs/xfs_buf.h +1 -3
index a118962..98b7ee9 100644
@@ -302,6 +302,7 @@ extern void xfs_buf_iomove(xfs_buf_t *, size_t, size_t, void *,
 
 /* Buffer Utility Routines */
 extern void *xfs_buf_offset(struct xfs_buf *, size_t);
+extern void xfs_buf_stale(struct xfs_buf *bp);
 
 /* Delayed Write Buffer Routines */
 extern bool xfs_buf_delwri_queue(struct xfs_buf *, struct list_head *);

@@ -317,9 +318,6 @@ extern void xfs_buf_terminate(void);
 			    XBF_SYNCIO|XBF_FUA|XBF_FLUSH| \
 			    XBF_WRITE_FAIL))
 
-void xfs_buf_stale(struct xfs_buf *bp);
-#define XFS_BUF_UNSTALE(bp)	((bp)->b_flags &= ~XBF_STALE)
-#define XFS_BUF_ISSTALE(bp)	((bp)->b_flags & XBF_STALE)
 
 /*
  * These macros use the IO block map rather than b_bn. b_bn is now really
fs/xfs/xfs_buf_item.c +1 -1
index 344ab44..2891134 100644
@@ -408,7 +408,7 @@ xfs_buf_item_unpin(
 	if (freed && stale) {
 		ASSERT(bip->bli_flags & XFS_BLI_STALE);
 		ASSERT(xfs_buf_islocked(bp));
-		ASSERT(XFS_BUF_ISSTALE(bp));
+		ASSERT(bp->b_flags & XBF_STALE);
 		ASSERT(bip->__bli_format.blf_flags & XFS_BLF_CANCEL);
 
 		trace_xfs_buf_item_unpin_stale(bip);
fs/xfs/xfs_trans_buf.c +3 -3
index ed6f61f..8ee29ca 100644
@@ -534,8 +534,8 @@ xfs_trans_log_buf(xfs_trans_t	*tp,
 	 */
 	if (bip->bli_flags & XFS_BLI_STALE) {
 		bip->bli_flags &= ~XFS_BLI_STALE;
-		ASSERT(XFS_BUF_ISSTALE(bp));
-		XFS_BUF_UNSTALE(bp);
+		ASSERT(bp->b_flags & XBF_STALE);
+		bp->b_flags &= ~XBF_STALE;
 		bip->__bli_format.blf_flags &= ~XFS_BLF_CANCEL;
 	}
 

@@ -600,7 +600,7 @@ xfs_trans_binval(
 		 * If the buffer is already invalidated, then
 		 * just return.
 		 */
-		ASSERT(XFS_BUF_ISSTALE(bp));
+		ASSERT(bp->b_flags & XBF_STALE);
 		ASSERT(!(bip->bli_flags & (XFS_BLI_LOGGED | XFS_BLI_DIRTY)));
 		ASSERT(!(bip->__bli_format.blf_flags & XFS_BLF_INODE_BUF));
 		ASSERT(!(bip->__bli_format.blf_flags & XFS_BLFT_MASK));
Signature · 6 lines
2.5.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#11 lore raw

PATCH 6/6 xfs: rename XFS_BUF_ZEROFLAGS macro

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

This macro is really clearing the IO flags from the buffer. Rename it appropriately and turn it into a static inline to get rid of the shoutiness.

Signed-off-by Dave Chinner <[email protected]>
---
 fs/xfs/xfs_buf.h         | 10 ++++++----
 fs/xfs/xfs_log.c         |  4 ++--
 fs/xfs/xfs_log_recover.c |  2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)
fs/xfs/xfs_buf.h +6 -4
index 98b7ee9..0735096 100644
@@ -313,10 +313,12 @@ extern int xfs_buf_delwri_submit_nowait(struct list_head *);
 extern int xfs_buf_init(void);
 extern void xfs_buf_terminate(void);
 
-#define XFS_BUF_ZEROFLAGS(bp) \
-	((bp)->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC| \
-			    XBF_SYNCIO|XBF_FUA|XBF_FLUSH| \
-			    XBF_WRITE_FAIL))
+static inline void xfs_buf_clear_ioflags(struct xfs_buf *bp)
+{
+	bp->b_flags &= ~(XBF_READ | XBF_WRITE | XBF_ASYNC |
+			 XBF_SYNCIO | XBF_FUA | XBF_FLUSH |
+			 XBF_WRITE_FAIL);
+}
 
 
 /*
fs/xfs/xfs_log.c +2 -2
index edf20b2..185d1b0 100644
@@ -1864,7 +1864,7 @@ xlog_sync(
 
 	bp->b_io_length = BTOBB(count);
 	bp->b_fspriv = iclog;
-	XFS_BUF_ZEROFLAGS(bp);
+	xfs_buf_clear_ioflags(bp);
 	bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 
 	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) {

@@ -1908,7 +1908,7 @@ xlog_sync(
 		xfs_buf_associate_memory(bp,
 				(char *)&iclog->ic_header + count, split);
 		bp->b_fspriv = iclog;
-		XFS_BUF_ZEROFLAGS(bp);
+		xfs_buf_clear_ioflags(bp);
 		bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 			bp->b_flags |= XBF_FUA;
fs/xfs/xfs_log_recover.c +1 -1
index 68406fd..b05d07b 100644
@@ -275,9 +275,9 @@ xlog_bwrite(
 	ASSERT(nbblks <= bp->b_length);
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
-	XFS_BUF_ZEROFLAGS(bp);
 	xfs_buf_hold(bp);
 	xfs_buf_lock(bp);
+	xfs_buf_clear_ioflags(bp);
 	bp->b_io_length = nbblks;
 	bp->b_error = 0;
Signature · 6 lines
2.5.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#12 lore raw

Re: [PATCH 6/6] xfs: rename XFS_BUF_ZEROFLAGS macro

Instead of just renaming it I'd rather dig deeper.

In xlog_bwrite we call xfs_bwrite, which already handles all flag clearing, so the call to XFS_BUF_ZEROFLAGS can just
be removed.

xlog_sync already sets a lot of these flags again, and is called on an iclog buffer which never has the READ or XBF_WRITE_FAIL set, so just replacing it with an opencoded

	bp->b_flags &= (XBF_FUA | XBF_FLUSH);

for both instances should be enough, and even that could be moved into and else clause of the

	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)

conditional.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#13 lore raw

Re: [PATCH 6/6] xfs: rename XFS_BUF_ZEROFLAGS macro

Quoted 5 lines · Christoph Hellwig

Instead of just renaming it I'd rather dig deeper.

In xlog_bwrite we call xfs_bwrite, which already handles all flag clearing, so the call to XFS_BUF_ZEROFLAGS can just
be removed.

Done.

Quoted 5 lines

xlog_sync already sets a lot of these flags again, and is called on an iclog buffer which never has the READ or XBF_WRITE_FAIL set, so just replacing it with an opencoded

	bp->b_flags &= (XBF_FUA | XBF_FLUSH);

We can't do that because there are internal flags like _XBF_PAGES that are set on log buffers. Clearing such flags will cause problems when the iclog buffer is finally released on unmount. Hence I'd prefer to keep the code as it stands.

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#14 lore raw

PATCH v2 6/6 xfs: rename XFS_BUF_ZEROFLAGS macro

From: Dave Chinner <[email protected]>

This macro is really clearing the IO flags from the buffer. Rename it appropriately and turn it into a static inline to get rid of the shoutiness.

Signed-off-by Dave Chinner <[email protected]>

---
V2 - kill call from xlog_bwrite completely as xfs_bwrite() takes care of clearing the flags appropriately.

 fs/xfs/xfs_buf.h         | 10 ++++++----
 fs/xfs/xfs_log.c         |  4 ++--
 fs/xfs/xfs_log_recover.c |  1 -
 3 files changed, 8 insertions(+), 7 deletions(-)
fs/xfs/xfs_buf.h +6 -4
index 98b7ee9..0735096 100644
@@ -313,10 +313,12 @@ extern int xfs_buf_delwri_submit_nowait(struct list_head *);
 extern int xfs_buf_init(void);
 extern void xfs_buf_terminate(void);
 
-#define XFS_BUF_ZEROFLAGS(bp) \
-	((bp)->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC| \
-			    XBF_SYNCIO|XBF_FUA|XBF_FLUSH| \
-			    XBF_WRITE_FAIL))
+static inline void xfs_buf_clear_ioflags(struct xfs_buf *bp)
+{
+	bp->b_flags &= ~(XBF_READ | XBF_WRITE | XBF_ASYNC |
+			 XBF_SYNCIO | XBF_FUA | XBF_FLUSH |
+			 XBF_WRITE_FAIL);
+}
 
 
 /*
fs/xfs/xfs_log.c +2 -2
index edf20b2..185d1b0 100644
@@ -1864,7 +1864,7 @@ xlog_sync(
 
 	bp->b_io_length = BTOBB(count);
 	bp->b_fspriv = iclog;
-	XFS_BUF_ZEROFLAGS(bp);
+	xfs_buf_clear_ioflags(bp);
 	bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 
 	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) {

@@ -1908,7 +1908,7 @@ xlog_sync(
 		xfs_buf_associate_memory(bp,
 				(char *)&iclog->ic_header + count, split);
 		bp->b_fspriv = iclog;
-		XFS_BUF_ZEROFLAGS(bp);
+		xfs_buf_clear_ioflags(bp);
 		bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 			bp->b_flags |= XBF_FUA;
fs/xfs/xfs_log_recover.c +0 -1
index bee2257..23ad143 100644
@@ -275,7 +275,6 @@ xlog_bwrite(
 	ASSERT(nbblks <= bp->b_length);
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
-	XFS_BUF_ZEROFLAGS(bp);
 	xfs_buf_hold(bp);
 	xfs_buf_lock(bp);
 	bp->b_io_length = nbblks;
Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#15 lore raw

Re: [PATCH 6/6] xfs: rename XFS_BUF_ZEROFLAGS macro

In reply to Dave Chinner
Quoted 10 lines · Dave Chinner
Quoted 5 lines

xlog_sync already sets a lot of these flags again, and is called on an iclog buffer which never has the READ or XBF_WRITE_FAIL set, so just replacing it with an opencoded

	bp->b_flags &= (XBF_FUA | XBF_FLUSH);

We can't do that because there are internal flags like _XBF_PAGES that are set on log buffers. Clearing such flags will cause problems when the iclog buffer is finally released on unmount. Hence I'd prefer to keep the code as it stands.

Sorry - meabt to write:

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

which is all we'll need. All other flags cleared by XFS_BUF_ZEROFLAGS are either never set for log buffers (XBF_READ, XBF_WRITE_FAIL), or always set for log buffers (XBF_WRITE, XBF_ASYNC, XBF_SYNCIO).

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#16 lore raw

Re: [PATCH 6/6] xfs: rename XFS_BUF_ZEROFLAGS macro

Quoted 12 lines · Christoph Hellwig
Quoted 10 lines · Dave Chinner
Quoted 5 lines

xlog_sync already sets a lot of these flags again, and is called on an iclog buffer which never has the READ or XBF_WRITE_FAIL set, so just replacing it with an opencoded

	bp->b_flags &= (XBF_FUA | XBF_FLUSH);

We can't do that because there are internal flags like _XBF_PAGES that are set on log buffers. Clearing such flags will cause problems when the iclog buffer is finally released on unmount. Hence I'd prefer to keep the code as it stands.

Quoted 7 lines

Sorry - meabt to write:

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

which is all we'll need. All other flags cleared by XFS_BUF_ZEROFLAGS are either never set for log buffers (XBF_READ, XBF_WRITE_FAIL), or always set for log buffers (XBF_WRITE, XBF_ASYNC, XBF_SYNCIO).

Ah, that makes more sense. I should have realised that, but there are places where we do clear buffer flags like your original suggestion so it didn't occur to me it was just a typo. I'll fix it up.

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#17 lore raw

PATCH v3 6/6 xfs: remove XFS_BUF_ZEROFLAGS macro

xfs: remove XFS_BUF_ZEROFLAGS macro

From: Dave Chinner <[email protected]>

The places where we use this macro already clear unnecessary IO flags (e.g. through xfs_bwrite()) or never have unexpected IO flags set on them in the first place (e.g. iclog buffers). Remove the macro from these locations, and where necessary clear only the specific flags that are conditional in the current buffer use context.

Signed-off-by Dave Chinner <[email protected]>

---
Version 3 - completely remove the macro as Christoph originally intended.

 fs/xfs/xfs_buf.h         | 6 ------
 fs/xfs/xfs_log.c         | 4 ++--
 fs/xfs/xfs_log_recover.c | 1 -
 3 files changed, 2 insertions(+), 9 deletions(-)
fs/xfs/xfs_buf.h +0 -6
index 98b7ee9..4eb89bd 100644
@@ -313,12 +313,6 @@ extern int xfs_buf_delwri_submit_nowait(struct list_head *);
 extern int xfs_buf_init(void);
 extern void xfs_buf_terminate(void);
 
-#define XFS_BUF_ZEROFLAGS(bp) \
-	((bp)->b_flags &= ~(XBF_READ|XBF_WRITE|XBF_ASYNC| \
-			    XBF_SYNCIO|XBF_FUA|XBF_FLUSH| \
-			    XBF_WRITE_FAIL))
-
-
 /*
  * These macros use the IO block map rather than b_bn. b_bn is now really
  * just for the buffer cache index for cached buffers. As IO does not use b_bn
fs/xfs/xfs_log.c +2 -2
index edf20b2..40b700d 100644
@@ -1864,7 +1864,7 @@ xlog_sync(
 
 	bp->b_io_length = BTOBB(count);
 	bp->b_fspriv = iclog;
-	XFS_BUF_ZEROFLAGS(bp);
+	bp->b_flags &= ~(XBF_FUA | XBF_FLUSH);
 	bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 
 	if (log->l_mp->m_flags & XFS_MOUNT_BARRIER) {

@@ -1908,7 +1908,7 @@ xlog_sync(
 		xfs_buf_associate_memory(bp,
 				(char *)&iclog->ic_header + count, split);
 		bp->b_fspriv = iclog;
-		XFS_BUF_ZEROFLAGS(bp);
+		bp->b_flags &= ~(XBF_FUA | XBF_FLUSH);
 		bp->b_flags |= (XBF_ASYNC | XBF_SYNCIO | XBF_WRITE);
 		if (log->l_mp->m_flags & XFS_MOUNT_BARRIER)
 			bp->b_flags |= XBF_FUA;
fs/xfs/xfs_log_recover.c +0 -1
index 70e4c1e..1dc0e14 100644
@@ -275,7 +275,6 @@ xlog_bwrite(
 	ASSERT(nbblks <= bp->b_length);
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
-	XFS_BUF_ZEROFLAGS(bp);
 	xfs_buf_hold(bp);
 	xfs_buf_lock(bp);
 	bp->b_io_length = nbblks;

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs