Re: [PATCH 2/2] xfs: remove bp->b_error check in xfs_attr3_root_inactive

[Long Li <leo.lilong@huawei.com>]

On Sat, Dec 21, 2024 at 12:08:56AM -0800, Christoph Hellwig wrote:
> 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.
> 

It's true that the commit message is not clearly explained, and I'll rewrite
it in the next version.

Thanks,
Long Li