On Wed, Nov 30, 2022 at 05:32:38PM +0800, Yang, Xiao/杨 晓 wrote: > On 2022/11/30 6:01, Darrick J. Wong wrote: > > From: Darrick J. Wong<djwong@xxxxxxxxxx> > > > > Upon enabling fsdax + reflink for XFS, xfs/179 began to report refcount > > metadata corruptions after being run. Specifically, xfs_repair noticed > > single-block refcount records that could be combined but had not been. > Hi Darrick, > > I am investigating the issue as well. Thanks a lot for your quick fix. > I have confirmed that xfs/179 with the fix patch can works well in DAX mode. > Reviewed-by: Xiao Yang <yangx.jy@xxxxxxxxxxx> > > > > > The root cause of this is improper MAXREFCOUNT edge case handling in > > xfs_refcount_merge_extents. When we're trying to find candidates for a > > refcount btree record merge, we compute the refcount attribute of the > > merged record, but we fail to account for the fact that once a record > > hits rc_refcount == MAXREFCOUNT, it is pinned that way forever. Hence > > One question: > Is it reansonable/expected to pin rc_refcount forever when a record hits > rc_refcount == MAXREFCOUNT? In the ideal world we'd have a way for refcount_adjust to return early if it hit a MAXREFCOUNT record, and stop the reflink operation right there and then. *Unfortunately* due to the way defer ops work, by the time we're walking through the refcount btree, we've already committed to adding the entire mapping. There's no good way to back out once we start, since transactions do not log undo items, only redo items. Augmenting the log to support undo items is a very big ask. We could try to work around that by walking the refcount records *before* committing log items, but that second walk will increase the overhead for a fairly difficult to hit corner case. We'd also need to hold the AGF buffer lock from the start of the chain all the way until we start the deferred refcount processing. Or we'd need to add a separate AG lock. Even if we did all that, FICLONE* doesn't have any means to communicate that a short reflink occurred. copy_file_range does, but then it has other weird warts, and cannot reflink more than 2^32 bytes in one go. The simplest solution is to pin the extent if its refcount hits MAXREFCOUNT, so that's what I went with. :/ --D > > the computed refcount is wrong, and we fail to merge the extents. > > > > Fix this by adjusting the merge predicates to compute the adjusted > > refcount correctly. > > Best Regards, > Xiao Yang
