Skip to contentSkip to conversation

xfs_repair: take the ag_lock before recording rmap for a bmbt record

PATCH In review Darrick J. Wong 2 participants 2 messages Aug 31, 2017 – Sep 1, 2017

lore
Share & tools
lore

Darrick J. Wong

author
#0 lore raw

PATCH 1/1 xfs_repair: take the ag_lock before recording rmap for a bmbt record

When the (threaded) inode scanner iterates the blocks of a bmbt tree and wants to record the bmbt blocks in the in-core rmap database, we have to take the ag_lock for the AG that the bmbt block is in, or else we can accidentally corrupt the rmap slab by calling slab_add from two threads.

Reported-by [email protected]
Signed-off-by Darrick J. Wong <[email protected]>
---
 repair/scan.c |    3 +++
 1 file changed, 3 insertions(+)
repair/scan.c +3 -0
index 6c9f5e7..ccf60a6 100644
@@ -386,7 +386,10 @@ _("bad state %d, inode %" PRIu64 " bmap block 0x%" PRIx64 "\n"),
 
 	/* Record BMBT blocks in the reverse-mapping data. */
 	if (check_dups && collect_rmaps) {
+		agno = XFS_FSB_TO_AGNO(mp, bno);
+		pthread_mutex_lock(&ag_locks[agno].lock);
 		error = rmap_add_bmbt_rec(mp, ino, whichfork, bno);
+		pthread_mutex_unlock(&ag_locks[agno].lock);
 		if (error)
 			do_error(
 _("couldn't add inode %"PRIu64" bmbt block %"PRIu64" reverse-mapping data."),
Reviewed-by on v1 1/1
#1 lore raw
Quoted 7 lines · Darrick J. Wong

When the (threaded) inode scanner iterates the blocks of a bmbt tree and wants to record the bmbt blocks in the in-core rmap database, we have to take the ag_lock for the AG that the bmbt block is in, or else we can accidentally corrupt the rmap slab by calling slab_add from two threads.

Reported-by [email protected]
Signed-off-by Darrick J. Wong <[email protected]>

Looks good,

Reviewed-by Carlos Maiolino <[email protected]>
Quoted 23 lines
---
 repair/scan.c |    3 +++
 1 file changed, 3 insertions(+)
repair/scan.c +3 -1
index 6c9f5e7..ccf60a6 100644
@@ -386,7 +386,10 @@ _("bad state %d, inode %" PRIu64 " bmap block 0x%" PRIx64 "\n"),
 
 	/* Record BMBT blocks in the reverse-mapping data. */
 	if (check_dups && collect_rmaps) {
+		agno = XFS_FSB_TO_AGNO(mp, bno);
+		pthread_mutex_lock(&ag_locks[agno].lock);
 		error = rmap_add_bmbt_rec(mp, ino, whichfork, bno);
+		pthread_mutex_unlock(&ag_locks[agno].lock);
 		if (error)
 			do_error(
 _("couldn't add inode %"PRIu64" bmbt block %"PRIu64" reverse-mapping data."),
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 1 line
Carlos