review: handle barriers being switched off dynamically.
As pointed out by Neil Brown, MD can switch barriers off dynamically underneath a mounted filesystem. If this happens to XFS, it will shutdown the filesystem immediately.
Handle this more sanely by yelling into the syslog, retrying the I/O without barriers and if that is successful, turn off barriers.
Also remove an unnecessary check when first checking to see if the underlying device supports barriers.
Cheers,
Dave.
Signature · 76 lines
Dave Chinner
Principal Engineer
SGI Australian Software Group
---
fs/xfs/linux-2.6/xfs_buf.c | 13 ++++++++++++-
fs/xfs/linux-2.6/xfs_super.c | 8 --------
fs/xfs/xfs_log.c | 13 +++++++++++++
3 files changed, 25 insertions(+), 9 deletions(-)
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_buf.c 2007-04-19 13:26:49.000000000 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_buf.c 2007-04-19 13:27:01.733786992 +1000
@@ -1000,7 +1000,18 @@ xfs_buf_iodone_work(
xfs_buf_t *bp =
container_of(work, xfs_buf_t, b_iodone_work);
- if (bp->b_iodone)
+ /*
+ * We can get an EOPNOTSUPP to ordered writes. Here we clear the
+ * ordered flag and reissue them. Because we can't tell the higher
+ * layers directly that they should not issue ordered I/O anymore, they
+ * need to check if the ordered flag was cleared during I/O completion.
+ */
+ if ((bp->b_error == EOPNOTSUPP) &&
+ (bp->b_flags & (XBF_ORDERED|XBF_ASYNC)) == (XBF_ORDERED|XBF_ASYNC)) {
+ XB_TRACE(bp, "ordered_retry", bp->b_iodone);
+ bp->b_flags &= ~XBF_ORDERED;
+ xfs_buf_iorequest(bp);
+ } else if (bp->b_iodone)
(*(bp->b_iodone))(bp);
else if (bp->b_flags & XBF_ASYNC)
xfs_buf_relse(bp);
Index: 2.6.x-xfs-new/fs/xfs/xfs_log.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/xfs_log.c 2007-04-19 13:27:00.245980891 +1000
+++ 2.6.x-xfs-new/fs/xfs/xfs_log.c 2007-04-19 13:27:01.753784386 +1000
@@ -961,6 +961,19 @@ xlog_iodone(xfs_buf_t *bp)
l = iclog->ic_log;
/*
+ * If the ordered flag has been removed by a lower
+ * layer, it means the underlyin device no longer supports
+ * barrier I/O. Warn loudly and turn off barriers.
+ */
+ if ((l->l_mp->m_flags & XFS_MOUNT_BARRIER) && !XFS_BUF_ORDERED(bp)) {
+ l->l_mp->m_flags &= ~XFS_MOUNT_BARRIER;
+ xfs_fs_cmn_err(CE_WARN, l->l_mp,
+ "xlog_iodone: Barriers are no longer supported"
+ " by device. Disabling barriers\n");
+ xfs_buftrace("XLOG_IODONE BARRIERS OFF", bp);
+ }
+
+ /*
* Race to shutdown the filesystem if we see an error.
*/
if (XFS_TEST_ERROR((XFS_BUF_GETERROR(bp)), l->l_mp,
Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_super.c 2007-04-19 13:27:00.277976721 +1000
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_super.c 2007-04-19 13:27:01.757783865 +1000
@@ -314,14 +314,6 @@ xfs_mountfs_check_barriers(xfs_mount_t *
return;
}
- if (mp->m_ddev_targp->bt_bdev->bd_disk->queue->ordered ==
- QUEUE_ORDERED_NONE) {
- xfs_fs_cmn_err(CE_NOTE, mp,
- "Disabling barriers, not supported by the underlying device");
- mp->m_flags &= ~XFS_MOUNT_BARRIER;
- return;
- }
-
if (xfs_readonly_buftarg(mp->m_ddev_targp)) {
xfs_fs_cmn_err(CE_NOTE, mp,
"Disabling barriers, underlying device is readonly");Quoted 11 lines · David Chinner
As pointed out by Neil Brown, MD can switch barriers off dynamically underneath a mounted filesystem. If this happens to XFS, it will shutdown the filesystem immediately.
Handle this more sanely by yelling into the syslog, retrying the I/O without barriers and if that is successful, turn off barriers.
Also remove an unnecessary check when first checking to see if the underlying device supports barriers.
Looks good to me (well, not really good, but as good as it can be given the circumstances..)
Review
Quoted 7 lines
(quoted diff) +7
-0+ /* + * We can get an EOPNOTSUPP to ordered writes. Here we clear the + * ordered flag and reissue them. Because we can't tell the higher + * layers directly that they should not issue ordered I/O anymore, they + * need to check if the ordered flag was cleared during I/O completion. + */ + if ((bp->b_error == EOPNOTSUPP) &&
no need for the additional braces here, though.