xfs: fix realtime device assert failures
cover xfs: fix realtime device assert failures
Hi folks,
This is a followup to Eric's patch that attempted to fix the assert failure that Ross reported. I'd mentioned I'd looked at this recently, because xfs/090 was triggering the same failure when configured with a RT device. This series fixes the original locking based assert failures, followed by the issues that fixing the original assert uncovered. This now results in xfs/090 failing because of xfs_db check problem (repair is clean) but it (and other tests) no longer assert fail.
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/3 xfs: lock rt summary inode on allocation
From: Dave Chinner <[email protected]>
RT allocation can fail on a debug kernel with:
XFS: Assertion failed: xfs_isilocked(ip, XFS_ILOCK_SHARED|XFS_ILOCK_EXCL), file: fs/xfs/libxfs/xfs_bmap.c, line: 4039
When modifying the summary inode during allocation. This occurs because the summary inode is never locked, and xfs_bmapi_* operations expect it to be locked. The summary inode is effectively protected byt he lock on the bitmap inode, so this really is only a debug kernel issue.
--- fs/xfs/xfs_bmap_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/xfs/xfs_bmap_util.c +3 -1
@@ -202,10 +202,12 @@ xfs_bmap_rtalloc( ralen =MAXEXTLEN / mp->m_sb.sb_rextsize;/* -* Lock out other modifications to the RT bitmap inode. +* Lock out modifications to both the RT bitmap and summary inodes */ xfs_ilock (mp->m_rbmip,XFS_ILOCK_EXCL );xfs_trans_ijoin (ap->tp, mp->m_rbmip,XFS_ILOCK_EXCL ); +xfs_ilock (mp->m_rsumip,XFS_ILOCK_EXCL ); +xfs_trans_ijoin (ap->tp, mp->m_rsumip,XFS_ILOCK_EXCL );/* * If it's an allocation to an empty file at offset 0,
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/3] xfs: lock rt summary inode on allocation
Quoted 6 lines · Dave Chinner
From: Dave Chinner <[email protected]>
RT allocation can fail on a debug kernel with:
XFS: Assertion failed:
xfs_isilocked(ip, XFS_ILOCK_SHARED|XFS_ILOCK_EXCL), file:fs/xfs/libxfs/xfs_bmap.c, line: 4039
Quoted 7 lines
When modifying the summary inode during allocation. This occurs because the summary inode is never locked, and xfs_bmapi_* operations expect it to be locked. The summary inode is effectively protected byt he lock on the bitmap inode, so this really is only a debug kernel issue.
Signed-off-by Dave Chinner <[email protected]>
Oh, it was easy after all...
Quoted 13 lines
--- fs/xfs/xfs_bmap_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)fs/xfs/xfs_bmap_util.c +0
-0@@ -202,10 +202,12 @@ xfs_bmap_rtalloc( ralen =MAXEXTLEN / mp->m_sb.sb_rextsize;/*
fs/xfs/xfs_bmap_util.c xfs_bmap_rtalloc( · line 213
Quoted 10 lines
fs/xfs/xfs_bmap_util.c +3
-1-* Lock out other modifications to the RT bitmap inode. +* Lock out modifications to both the RT bitmap and summary inodes */ xfs_ilock (mp->m_rbmip,XFS_ILOCK_EXCL );xfs_trans_ijoin (ap->tp, mp->m_rbmip,XFS_ILOCK_EXCL ); +xfs_ilock (mp->m_rsumip,XFS_ILOCK_EXCL ); +xfs_trans_ijoin (ap->tp, mp->m_rsumip,XFS_ILOCK_EXCL );/* * If it's an allocation to an empty file at offset 0,
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/3 xfs: RT bitmap and summary buffers are not typed
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
When logging buffers, we attach a type to them that follows the buffer all the way into the log and is used to identify the buffer contents in log recovery. Both the realtime summary buffers and the bitmap buffers do not have types defined or set, so when we try to log them we see assert failure:
XFS: Assertion failed: (bip->bli_flags & XFS_BLI_STALE) || (xfs_blft_from_flags(&bip->__bli_format) > XFS_BLFT_UNKNOWN_BUF && xfs_blft_from_flags(&bip->__bli_format) < XFS_BLFT_MAX_BUF), file: fs/xfs/xfs_buf_item.c, line: 294
Fix this by adding buffer log format types for these buffers, and add identification support into log recovery for them.
--- fs/xfs/libxfs/xfs_log_format.h | 2 ++ fs/xfs/libxfs/xfs_rtbitmap.c | 3 +++ fs/xfs/xfs_log_recover.c | 5 +++++ 3 files changed, 10 insertions(+)
fs/xfs/libxfs/xfs_log_format.h +2 -0
@@ -496,6 +496,8 @@ enum xfs_blft {XFS_BLFT_ATTR_LEAF_BUF ,XFS_BLFT_ATTR_RMT_BUF ,XFS_BLFT_SB_BUF , +XFS_BLFT_RTBITMAP_BUF , +XFS_BLFT_RTSUMMARY_BUF , XFS_BLFT_MAX_BUF = (1 << XFS_BLFT_BITS), };
fs/xfs/libxfs/xfs_rtbitmap.c +3 -0
@@ -71,6 +71,9 @@ xfs_rtbuf_get( mp->m_bsize, 0, &bp, NULL);if (error)return error; + +xfs_trans_buf_set_type (tp, bp, issum ?XFS_BLFT_RTSUMMARY_BUF + :XFS_BLFT_RTBITMAP_BUF ); *bpp = bp;return 0 ; }
fs/xfs/xfs_log_recover.c +5 -0
@@ -2473,6 +2473,11 @@ xlog_recover_validate_buf_type( } bp->b_ops = &xfs_sb_buf_ops;break ; +case XFS_BLFT_RTBITMAP_BUF : +case XFS_BLFT_RTSUMMARY_BUF : +/* no verification of RT buffers is done */ + bp->b_ops =NULL ; +break ;default :xfs_warn (mp,"Unknown buffer type %d!" ,xfs_blft_from_flags (buf_f));
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/3] xfs: RT bitmap and summary buffers are not typed
Quoted 8 lines · Dave Chinner
From: Dave Chinner <[email protected]>
When logging buffers, we attach a type to them that follows the buffer all the way into the log and is used to identify the buffer contents in log recovery. Both the realtime summary buffers and the bitmap buffers do not have types defined or set, so when we try to log them we see assert failure:
Quoted 6 lines
XFS: Assertion failed: (
bip->bli_flags&XFS_BLI_STALE) || (xfs_blft_from_flags(&bip->__bli_format)>XFS_BLFT_UNKNOWN_BUF&&xfs_blft_from_flags(&bip->__bli_format)<XFS_BLFT_MAX_BUF), file:fs/xfs/xfs_buf_item.c, line: 294Fix this by adding buffer log format types for these buffers, and add identification support into log recovery for them.
Signed-off-by Dave Chinner <[email protected]>
Quoted 40 lines
--- fs/xfs/libxfs/xfs_log_format.h | 2 ++ fs/xfs/libxfs/xfs_rtbitmap.c | 3 +++ fs/xfs/xfs_log_recover.c | 5 +++++ 3 files changed, 10 insertions(+)fs/xfs/libxfs/xfs_log_format.h +2
-0@@ -496,6 +496,8 @@ enum xfs_blft {XFS_BLFT_ATTR_LEAF_BUF ,XFS_BLFT_ATTR_RMT_BUF ,XFS_BLFT_SB_BUF , +XFS_BLFT_RTBITMAP_BUF , +XFS_BLFT_RTSUMMARY_BUF , XFS_BLFT_MAX_BUF = (1 << XFS_BLFT_BITS), };fs/xfs/libxfs/xfs_rtbitmap.c +3
-0@@ -71,6 +71,9 @@ xfs_rtbuf_get( mp->m_bsize, 0, &bp, NULL);if (error)return error; + +xfs_trans_buf_set_type (tp, bp, issum ?XFS_BLFT_RTSUMMARY_BUF + :XFS_BLFT_RTBITMAP_BUF ); *bpp = bp;return 0 ; }fs/xfs/xfs_log_recover.c +0
-0@@ -2473,6 +2473,11 @@ xlog_recover_validate_buf_type( }
fs/xfs/xfs_log_recover.c xlog_recover_validate_buf_type( · line 2483
Quoted 10 lines
fs/xfs/xfs_log_recover.c +5
-0bp->b_ops = &xfs_sb_buf_ops;break ; +case XFS_BLFT_RTBITMAP_BUF : +case XFS_BLFT_RTSUMMARY_BUF : +/* no verification of RT buffers is done */ + bp->b_ops =NULL ; +break ;default :xfs_warn (mp,"Unknown buffer type %d!" ,xfs_blft_from_flags (buf_f));
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 3/3 xfs: RT bitmap and summary buffers need verifiers
In reply to Dave ChinnerFrom: Dave Chinner <[email protected]>
Buffers without verifiers issue runtime warnings on XFS. We don't have anything we can actually verify in the RT buffers (no CRCs, not magic numbers, etc), but we still need verifiers to avoid the warnings.
Add a set of dummy verifier operations for the realtime buffers and apply them in the appropriate places.
--- fs/xfs/libxfs/xfs_rtbitmap.c | 27 ++++++++++++++++++++++++++- fs/xfs/libxfs/xfs_shared.h | 1 + fs/xfs/xfs_log_recover.c | 4 ++-- 3 files changed, 29 insertions(+), 3 deletions(-)
fs/xfs/libxfs/xfs_rtbitmap.c +26 -1
@@ -42,6 +42,31 @@*/ /* +* Real time buffers need verifiers to avoid runtime warnigns during IO. +* We don't have anything to verify, however, so these are just dummy +* operations. +*/ +static void +xfs_rtbuf_verify_read ( +struct xfs_buf *bp) +{ +return ; +} + +static void +xfs_rtbuf_verify_write ( +struct xfs_buf *bp) +{ +return ; +} + +const struct xfs_buf_ops xfs_rtbuf_ops = { + .name ="rtbuf" , + .verify_read = xfs_rtbuf_verify_read, + .verify_write = xfs_rtbuf_verify_write, +}; + +/* * Get a buffer for the bitmap or summary file block specified. * The buffer is returned read and locked. */
@@ -68,7 +93,7 @@ xfs_rtbuf_get(ASSERT (map.br_startblock !=NULLFSBLOCK ); error =xfs_trans_read_buf (mp, tp, mp->m_ddev_targp,XFS_FSB_TO_DADDR (mp, map.br_startblock), - mp->m_bsize,0 , &bp,NULL ); + mp->m_bsize,0 , &bp, &xfs_rtbuf_ops);if (error)return error;
fs/xfs/libxfs/xfs_shared.h +1 -0
@@ -53,6 +53,7 @@ extern const struct xfs_buf_ops xfs_dquot_buf_ra_ops;extern const struct xfs_buf_ops xfs_sb_buf_ops;extern const struct xfs_buf_ops xfs_sb_quiet_buf_ops;extern const struct xfs_buf_ops xfs_symlink_buf_ops; +extern const struct xfs_buf_ops xfs_rtbuf_ops;/* * Transaction types. Used to distinguish types of buffers. These never reach
fs/xfs/xfs_log_recover.c +2 -2
@@ -2475,8 +2475,8 @@ xlog_recover_validate_buf_type(break ;case XFS_BLFT_RTBITMAP_BUF :case XFS_BLFT_RTSUMMARY_BUF : -/* no verification of RT buffers is done */ - bp->b_ops =NULL ; +/* no magic numbers for verification of RT buffers */ + bp->b_ops = &xfs_rtbuf_ops;break ;default : xfs_warn(mp, "Unknown buffer type %d!",
Signature · 6 lines
2.5.0 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 3/3] xfs: RT bitmap and summary buffers need verifiers
Quoted 22 lines · Dave Chinner
From: Dave Chinner <[email protected]>
Buffers without verifiers issue runtime warnings on XFS. We don't have anything we can actually verify in the RT buffers (no CRCs, not magic numbers, etc), but we still need verifiers to avoid the warnings.
Add a set of dummy verifier operations for the realtime buffers and apply them in the appropriate places.
Signed-off-by Dave Chinner <[email protected]>--- fs/xfs/libxfs/xfs_rtbitmap.c | 27 ++++++++++++++++++++++++++- fs/xfs/libxfs/xfs_shared.h | 1 + fs/xfs/xfs_log_recover.c | 4 ++-- 3 files changed, 29 insertions(+), 3 deletions(-)fs/xfs/libxfs/xfs_rtbitmap.c +0
-0
fs/xfs/libxfs/xfs_rtbitmap.c line 45
Quoted 4 lines
fs/xfs/libxfs/xfs_rtbitmap.c +1
-0@@ -42,6 +42,31 @@*/ /* +* Real time buffers need verifiers to avoid runtime warnigns during IO.
"warnings" ;)
Otherwise:
Quoted 54 lines
fs/xfs/libxfs/xfs_rtbitmap.c +25
-1+* We don't have anything to verify, however, so these are just dummy +* operations. +*/ +static void +xfs_rtbuf_verify_read ( +struct xfs_buf *bp) +{ +return ; +} + +static void +xfs_rtbuf_verify_write ( +struct xfs_buf *bp) +{ +return ; +} + +const struct xfs_buf_ops xfs_rtbuf_ops = { + .name ="rtbuf" , + .verify_read = xfs_rtbuf_verify_read, + .verify_write = xfs_rtbuf_verify_write, +}; + +/* * Get a buffer for the bitmap or summary file block specified. * The buffer is returned read and locked. */ @@ -68,7 +93,7 @@ xfs_rtbuf_get(ASSERT (map.br_startblock !=NULLFSBLOCK ); error =xfs_trans_read_buf (mp, tp, mp->m_ddev_targp,XFS_FSB_TO_DADDR (mp, map.br_startblock), - mp->m_bsize,0 , &bp,NULL ); + mp->m_bsize,0 , &bp, &xfs_rtbuf_ops);if (error)return error;fs/xfs/libxfs/xfs_shared.h +1
-0@@ -53,6 +53,7 @@ extern const struct xfs_buf_ops xfs_dquot_buf_ra_ops;extern const struct xfs_buf_ops xfs_sb_buf_ops;extern const struct xfs_buf_ops xfs_sb_quiet_buf_ops;extern const struct xfs_buf_ops xfs_symlink_buf_ops; +extern const struct xfs_buf_ops xfs_rtbuf_ops;/* * Transaction types. Used to distinguish types of buffers. These never reach fs/xfs/xfs_log_recover.c +0
-0
fs/xfs/xfs_log_recover.c xlog_recover_validate_buf_type( · line 2482
Quoted 10 lines
fs/xfs/xfs_log_recover.c +2
-2@@ -2475,8 +2475,8 @@ xlog_recover_validate_buf_type(break ;case XFS_BLFT_RTBITMAP_BUF :case XFS_BLFT_RTSUMMARY_BUF : -/* no verification of RT buffers is done */ - bp->b_ops =NULL ; +/* no magic numbers for verification of RT buffers */ + bp->b_ops = &xfs_rtbuf_ops;break ;default : xfs_warn(mp, "Unknown buffer type %d!",
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
This series solves the assert and the follow-up XFS warning for me.
The lockdep splat is still there, but I assume you know that.
Quoted 11 lines · Dave Chinner
Hi folks,
This is a followup to Eric's patch that attempted to fix the assert failure that Ross reported. I'd mentioned I'd looked at this recently, because xfs/090 was triggering the same failure when configured with a RT device. This series fixes the original locking based assert failures, followed by the issues that fixing the original assert uncovered. This now results in xfs/090 failing because of
xfs_dbcheck problem (repair is clean) but it (and other tests) no longer assert fail.
Quoted 8 lines
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 5 lines · Ross Zwisler
This series solves the assert and the follow-up XFS warning for me.
Tested-by Ross Zwisler <[email protected]>The lockdep splat is still there, but I assume you know that.
Mostly I don't care about lockdep splats because experience tells me they are wild goose chases that end with us jumping through ever more complex flaming hoops to annotate the special corner case lockdep has falsely triggered on.
I also don't have time to care about lockdep right now, so I'm ignoring it until it manifests itself as a real deadlock.
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs