Message ID | 20241221063043.106037-3-leo.lilong@huawei.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | xfs: two small cleanup | expand |
On Sat, Dec 21, 2024 at 02:30:43PM +0800, Long Li wrote: > The xfs_da3_node_read earlier in the function will catch most cases of > incoming on-disk corruption, which makes this check mostly redundant, > unless someone corrupts the buffer and the AIL pushes it out to disk > while the buffer's unlocked. > > In the first case we'll never reach this check, and in the second case > the AIL will shut down the log, at which point checking b_error becomes > meaningless. Remove the check to make the code consistent with most other > xfs_trans_get_buf() callers in XFS. Hmm. I don't really understand the commit log. The b_error check is right after a call to xfs_trans_get_buf_map. xfs_trans_get_buf_map either reads the buffer from disk using xfs_buf_get_map which propagates b_error, or finds it in the transaction, where whoever read it from disk should have done the same. So I think the change looks fine, but I don't think the commit log really explains it very well.
diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c index 24fb12986a56..319004bf089f 100644 --- a/fs/xfs/xfs_attr_inactive.c +++ b/fs/xfs/xfs_attr_inactive.c @@ -305,11 +305,6 @@ xfs_attr3_root_inactive( XFS_FSB_TO_BB(mp, mp->m_attr_geo->fsbcount), 0, &bp); if (error) return error; - error = bp->b_error; - if (error) { - xfs_trans_brelse(*trans, bp); - return error; - } xfs_trans_binval(*trans, bp); /* remove from cache */ /* * Commit the invalidate and start the next transaction.
The xfs_da3_node_read earlier in the function will catch most cases of incoming on-disk corruption, which makes this check mostly redundant, unless someone corrupts the buffer and the AIL pushes it out to disk while the buffer's unlocked. In the first case we'll never reach this check, and in the second case the AIL will shut down the log, at which point checking b_error becomes meaningless. Remove the check to make the code consistent with most other xfs_trans_get_buf() callers in XFS. Signed-off-by: Long Li <leo.lilong@huawei.com> --- fs/xfs/xfs_attr_inactive.c | 5 ----- 1 file changed, 5 deletions(-)