From: Darrick J. Wong <djwong@xxxxxxxxxx> While stress-testing online repair of btrees, I noticed periodic assertion failures from the buffer cache about buffer readers encountering buffers with DELWRI_Q set, even though the btree bulk load had already committed and the buffer itself wasn't on any delwri list. I traced this to a misunderstanding of how the delwri lists work, particularly with regards to the AIL's buffer list. If a buffer is logged and committed, the buffer can end up on that AIL buffer list. If btree repairs are run twice in rapid succession, it's possible that the first repair will invalidate the buffer and free it before the next time the AIL wakes up. This clears DELWRI_Q from the buffer state. If the second repair allocates the same block, it will then recycle the buffer to start writing the new btree block. Meanwhile, if the AIL wakes up and walks the buffer list, it will ignore the buffer because it can't lock it, and go back to sleep. When the second repair calls delwri_queue to put the buffer on the list of buffers to write before committing the new btree, it will set DELWRI_Q again, but since the buffer hasn't been removed from the AIL's buffer list, it won't add it to the bulkload buffer's list. This is incorrect, because the bulkload caller relies on delwri_submit to ensure that all the buffers have been sent to disk /before/ committing the new btree root pointer. This ordering requirement is required for data consistency. Worse, the AIL won't clear DELWRI_Q from the buffer when it does finally drop it, so the next thread to walk through the btree will trip over a debug assertion on that flag. To fix this, create a new thread that waits for the buffer to be removed from any other delwri lists before adding the buffer to the caller's delwri list. Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> --- libxfs/libxfs_io.h | 11 +++++++++++ libxfs/xfs_btree_staging.c | 4 +--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h index fae86427201..4ffe788d446 100644 --- a/libxfs/libxfs_io.h +++ b/libxfs/libxfs_io.h @@ -243,6 +243,17 @@ xfs_buf_delwri_queue(struct xfs_buf *bp, struct list_head *buffer_list) return true; } +static inline void +xfs_buf_delwri_queue_here(struct xfs_buf *bp, struct list_head *buffer_list) +{ + ASSERT(list_empty(&bp->b_list)); + + /* This buffer is uptodate; don't let it get reread. */ + libxfs_buf_mark_dirty(bp); + + xfs_buf_delwri_queue(bp, buffer_list); +} + int xfs_buf_delwri_submit(struct list_head *buffer_list); void xfs_buf_delwri_cancel(struct list_head *list); diff --git a/libxfs/xfs_btree_staging.c b/libxfs/xfs_btree_staging.c index a6a90791668..baf7f422603 100644 --- a/libxfs/xfs_btree_staging.c +++ b/libxfs/xfs_btree_staging.c @@ -342,9 +342,7 @@ xfs_btree_bload_drop_buf( if (*bpp == NULL) return; - if (!xfs_buf_delwri_queue(*bpp, buffers_list)) - ASSERT(0); - + xfs_buf_delwri_queue_here(*bpp, buffers_list); xfs_buf_relse(*bpp); *bpp = NULL; }