On Thu, Jan 19, 2023 at 04:14:11PM +1100, Dave Chinner wrote:
> If we hit this race condition, re-reading the extent list from disk
> isn't going to fix the corruption, so I don't see much point in
> papering over the problem just by changing the locking and failing
> to read in the extent list again and returning -EFSCORRUPTED to the
> operation.
Yep.
> So.... shouldn't we mark the inode as sick when we detect the extent
> list corruption issue? i.e. before destroying the iext tree, calling
> xfs_inode_mark_sick(XFS_SICK_INO_BMBTD) (or BMBTA, depending on the
> fork being read) so that there is a record of the BMBT being
> corrupt?
Yes.
> That would mean that this path simply becomes:
>
> if (ip->i_sick & XFS_SICK_INO_BMBTD) {
> xfs_iunlock(ip, lock_mode);
> return -EFSCORRUPTED;
> }
This path being xfs_ilock_{data,attr}_map_shared? These don't
return an error. But if we make sure xfs_need_iread_extents
returns true for XFS_SICK_INO_BMBTD, xfs_iread_extents can
return -EFSCORRUPTED.
