Skip to contentSkip to conversation

xfs: Use BUG_ON rather than BUG() to remove unreachable code

PATCH In review Austin Kim 3 participants 3 messages Aug 22, 2019

Subsystems (2)
Kernel janitors XFS filesystem
lore
Share & tools
lore

Austin Kim

author
#0 lore raw

PATCH 1/1 xfs: Use BUG_ON rather than BUG() to remove unreachable code

Code after BUG is unreachable since system would be crashed after the call to BUG is made.
So change BUG_ON instead of BUG() to remove unreachable code.

---
 fs/xfs/xfs_mount.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
fs/xfs/xfs_mount.c +1 -7
index 322da69..a681808 100644
@@ -213,13 +213,7 @@ xfs_initialize_perag(
 			goto out_hash_destroy;
 
 		spin_lock(&mp->m_perag_lock);
-		if (radix_tree_insert(&mp->m_perag_tree, index, pag)) {
-			BUG();
-			spin_unlock(&mp->m_perag_lock);
-			radix_tree_preload_end();
-			error = -EEXIST;
-			goto out_hash_destroy;
-		}
+		BUG_ON(radix_tree_insert(&mp->m_perag_tree, index, pag));
 		spin_unlock(&mp->m_perag_lock);
 		radix_tree_preload_end();
 		/* first new pag is fully initialized */
Signature · 1 line
2.6.2
#1 lore raw

Depending on the config BUG() might be a no-op. Outside of filesystems everyone ignores that and crashes ungracefully, but in filesystems they don't want to risk corrupting your files.

regards,
dan carpenter

#2 lore raw
In reply to Austin Kim
Quoted 13 lines · Austin Kim

Code after BUG is unreachable since system would be crashed after the call to BUG is made.
So change BUG_ON instead of BUG() to remove unreachable code.

---
 fs/xfs/xfs_mount.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
fs/xfs/xfs_mount.c +0 -0
index 322da69..a681808 100644
@@ -213,13 +213,7 @@ xfs_initialize_perag(
 			goto out_hash_destroy;

fs/xfs/xfs_mount.c xfs_initialize_perag( · line 216

Quoted 10 lines
fs/xfs/xfs_mount.c +1 -7
 
 		spin_lock(&mp->m_perag_lock);
-		if (radix_tree_insert(&mp->m_perag_tree, index, pag)) {
-			BUG();
-			spin_unlock(&mp->m_perag_lock);
-			radix_tree_preload_end();
-			error = -EEXIST;
-			goto out_hash_destroy;
-		}
+		BUG_ON(radix_tree_insert(&mp->m_perag_tree, index, pag));

/me thinks this should WARN on the insertion error but otherwise preserve the existing bailout behavior, since we can always fail the mount or shutdown the fs and move on....

--D

Quoted 6 lines
fs/xfs/xfs_mount.c +0 -0
 		spin_unlock(&mp->m_perag_lock);
 		radix_tree_preload_end();
 		/* first new pag is fully initialized */
Signature · 1 line
2.6.2