xfs: Fix oops on IO error during xlog_recover_process_iunlinks()

When an IO error happens during inode deletion run from xlog_recover_process_iunlinks() filesystem gets shutdown. Thus any subsequent attempt to read buffers fails. Code in xlog_recover_process_iunlinks() does not count with the fact that read of a buffer which was read a while ago can really fail which results in the oops on agi = XFS_BUF_TO_AGI(agibp); Fix the problem by cleaning up the buffer handling in xlog_recover_process_iunlinks() as suggested by Dave Chinner. We release buffer lock but keep buffer reference to AG buffer. That is enough for buffer to stay pinned in memory and we don't have to call xfs_read_agi() all the time. CC: stable@kernel.org Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Ben Myers <bpm@sgi.com>

xfs: Fix oops on IO error during xlog_recover_process_iunlinks()
When an IO error happens during inode deletion run from xlog_recover_process_iunlinks() filesystem gets shutdown. Thus any subsequent attempt to read buffers fails. Code in xlog_recover_process_iunlinks() does not count with the fact that read of a buffer which was read a while ago can really fail which results in the oops on agi = XFS_BUF_TO_AGI(agibp); Fix the problem by cleaning up the buffer handling in xlog_recover_process_iunlinks() as suggested by Dave Chinner. We release buffer lock but keep buffer reference to AG buffer. That is enough for buffer to stay pinned in memory and we don't have to call xfs_read_agi() all the time. CC: stable@kernel.org Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Ben Myers <bpm@sgi.com>
Jan Kara · Ben Myers
1 parent a66d636385
Showing 1 changed file with 11 additions and 22 deletions Side-by-side Diff
fs/xfs/xfs_log_recover.c
@@ -3161,37 +3161,26 @@
 			 */
 			continue;
 		}
+		/*
+		 * Unlock the buffer so that it can be acquired in the normal
+		 * course of the transaction to truncate and free each inode.
+		 * Because we are not racing with anyone else here for the AGI
+		 * buffer, we don't even need to hold it locked to read the
+		 * initial unlinked bucket entries out of the buffer. We keep
+		 * buffer reference though, so that it stays pinned in memory
+		 * while we need the buffer.
+		 */
 		agi = XFS_BUF_TO_AGI(agibp);
+		xfs_buf_unlock(agibp);
  
 		for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++) {
 			agino = be32_to_cpu(agi->agi_unlinked[bucket]);
 			while (agino != NULLAGINO) {
-				/*
-				 * Release the agi buffer so that it can
-				 * be acquired in the normal course of the
-				 * transaction to truncate and free the inode.
-				 */
-				xfs_buf_relse(agibp);
-
 				agino = xlog_recover_process_one_iunlink(mp,
 							agno, agino, bucket);
-
-				/*
-				 * Reacquire the agibuffer and continue around
-				 * the loop. This should never fail as we know
-				 * the buffer was good earlier on.
-				 */
-				error = xfs_read_agi(mp, NULL, agno, &agibp);
-				ASSERT(error == 0);
-				agi = XFS_BUF_TO_AGI(agibp);
 			}
 		}
-
-		/*
-		 * Release the buffer for the current agi so we can
-		 * go on to the next one.
-		 */
-		xfs_buf_relse(agibp);
+		xfs_buf_rele(agibp);
 	}
  
 	mp->m_dmevmask = mp_dmevmask;
...	...	@@ -3161,37 +3161,26 @@
3161	3161	*/
3162	3162	continue;
3163	3163	}
	3164	+ /*
	3165	+ * Unlock the buffer so that it can be acquired in the normal
	3166	+ * course of the transaction to truncate and free each inode.
	3167	+ * Because we are not racing with anyone else here for the AGI
	3168	+ * buffer, we don't even need to hold it locked to read the
	3169	+ * initial unlinked bucket entries out of the buffer. We keep
	3170	+ * buffer reference though, so that it stays pinned in memory
	3171	+ * while we need the buffer.
	3172	+ */
3164	3173	agi = XFS_BUF_TO_AGI(agibp);
	3174	+ xfs_buf_unlock(agibp);
3165	3175
3166	3176	for (bucket = 0; bucket < XFS_AGI_UNLINKED_BUCKETS; bucket++) {
3167	3177	agino = be32_to_cpu(agi->agi_unlinked[bucket]);
3168	3178	while (agino != NULLAGINO) {
3169		- /*
3170		- * Release the agi buffer so that it can
3171		- * be acquired in the normal course of the
3172		- * transaction to truncate and free the inode.
3173		- */
3174		- xfs_buf_relse(agibp);
3175		-
3176	3179	agino = xlog_recover_process_one_iunlink(mp,
3177	3180	agno, agino, bucket);
3178		-
3179		- /*
3180		- * Reacquire the agibuffer and continue around
3181		- * the loop. This should never fail as we know
3182		- * the buffer was good earlier on.
3183		- */
3184		- error = xfs_read_agi(mp, NULL, agno, &agibp);
3185		- ASSERT(error == 0);
3186		- agi = XFS_BUF_TO_AGI(agibp);
3187	3181	}
3188	3182	}
3189		-
3190		- /*
3191		- * Release the buffer for the current agi so we can
3192		- * go on to the next one.
3193		- */
3194		- xfs_buf_relse(agibp);
	3183	+ xfs_buf_rele(agibp);
3195	3184	}
3196	3185
3197	3186	mp->m_dmevmask = mp_dmevmask;