* [PATCH] xfs: revert "xfs: fix rmap key and record comparison functions"
@ 2020-11-19 23:39 Darrick J. Wong
2020-11-19 23:44 ` Eric Sandeen
0 siblings, 1 reply; 2+ messages in thread
From: Darrick J. Wong @ 2020-11-19 23:39 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
From: Darrick J. Wong <darrick.wong@oracle.com>
This reverts commit 6ff646b2ceb0eec916101877f38da0b73e3a5b7f.
Your maintainer committed a major braino in the rmap code by adding the
attr fork, bmbt, and unwritten extent usage bits into rmap record key
comparisons. While XFS uses the usage bits *in the rmap records* for
cross-referencing metadata in xfs_scrub and xfs_repair, it only needs
the owner and offset information to distinguish between reverse mappings
of the same physical extent into the data fork of a file at multiple
offsets. The other bits are not important for key comparisons for index
lookups, and never have been.
Eric Sandeen reports that this causes regressions in generic/299, so
undo this patch before it does more damage.
Reported-by: Eric Sandeen <sandeen@sandeen.net>
Fixes: 6ff646b2ceb0 ("xfs: fix rmap key and record comparison functions")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_rmap_btree.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index 577a66381327..beb81c84a937 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -243,8 +243,8 @@ xfs_rmapbt_key_diff(
else if (y > x)
return -1;
- x = be64_to_cpu(kp->rm_offset);
- y = xfs_rmap_irec_offset_pack(rec);
+ x = XFS_RMAP_OFF(be64_to_cpu(kp->rm_offset));
+ y = rec->rm_offset;
if (x > y)
return 1;
else if (y > x)
@@ -275,8 +275,8 @@ xfs_rmapbt_diff_two_keys(
else if (y > x)
return -1;
- x = be64_to_cpu(kp1->rm_offset);
- y = be64_to_cpu(kp2->rm_offset);
+ x = XFS_RMAP_OFF(be64_to_cpu(kp1->rm_offset));
+ y = XFS_RMAP_OFF(be64_to_cpu(kp2->rm_offset));
if (x > y)
return 1;
else if (y > x)
@@ -390,8 +390,8 @@ xfs_rmapbt_keys_inorder(
return 1;
else if (a > b)
return 0;
- a = be64_to_cpu(k1->rmap.rm_offset);
- b = be64_to_cpu(k2->rmap.rm_offset);
+ a = XFS_RMAP_OFF(be64_to_cpu(k1->rmap.rm_offset));
+ b = XFS_RMAP_OFF(be64_to_cpu(k2->rmap.rm_offset));
if (a <= b)
return 1;
return 0;
@@ -420,8 +420,8 @@ xfs_rmapbt_recs_inorder(
return 1;
else if (a > b)
return 0;
- a = be64_to_cpu(r1->rmap.rm_offset);
- b = be64_to_cpu(r2->rmap.rm_offset);
+ a = XFS_RMAP_OFF(be64_to_cpu(r1->rmap.rm_offset));
+ b = XFS_RMAP_OFF(be64_to_cpu(r2->rmap.rm_offset));
if (a <= b)
return 1;
return 0;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] xfs: revert "xfs: fix rmap key and record comparison functions"
2020-11-19 23:39 [PATCH] xfs: revert "xfs: fix rmap key and record comparison functions" Darrick J. Wong
@ 2020-11-19 23:44 ` Eric Sandeen
0 siblings, 0 replies; 2+ messages in thread
From: Eric Sandeen @ 2020-11-19 23:44 UTC (permalink / raw)
To: Darrick J. Wong, Eric Sandeen; +Cc: xfs
On 11/19/20 5:39 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> This reverts commit 6ff646b2ceb0eec916101877f38da0b73e3a5b7f.
>
> Your maintainer committed a major braino in the rmap code by adding the
> attr fork, bmbt, and unwritten extent usage bits into rmap record key
> comparisons. While XFS uses the usage bits *in the rmap records* for
> cross-referencing metadata in xfs_scrub and xfs_repair, it only needs
> the owner and offset information to distinguish between reverse mappings
> of the same physical extent into the data fork of a file at multiple
> offsets. The other bits are not important for key comparisons for index
> lookups, and never have been.
>
> Eric Sandeen reports that this causes regressions in generic/299, so
> undo this patch before it does more damage.
>
> Reported-by: Eric Sandeen <sandeen@sandeen.net>
> Fixes: 6ff646b2ceb0 ("xfs: fix rmap key and record comparison functions")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fs/xfs/libxfs/xfs_rmap_btree.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
> index 577a66381327..beb81c84a937 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.c
> @@ -243,8 +243,8 @@ xfs_rmapbt_key_diff(
> else if (y > x)
> return -1;
>
> - x = be64_to_cpu(kp->rm_offset);
> - y = xfs_rmap_irec_offset_pack(rec);
> + x = XFS_RMAP_OFF(be64_to_cpu(kp->rm_offset));
> + y = rec->rm_offset;
> if (x > y)
> return 1;
> else if (y > x)
> @@ -275,8 +275,8 @@ xfs_rmapbt_diff_two_keys(
> else if (y > x)
> return -1;
>
> - x = be64_to_cpu(kp1->rm_offset);
> - y = be64_to_cpu(kp2->rm_offset);
> + x = XFS_RMAP_OFF(be64_to_cpu(kp1->rm_offset));
> + y = XFS_RMAP_OFF(be64_to_cpu(kp2->rm_offset));
> if (x > y)
> return 1;
> else if (y > x)
> @@ -390,8 +390,8 @@ xfs_rmapbt_keys_inorder(
> return 1;
> else if (a > b)
> return 0;
> - a = be64_to_cpu(k1->rmap.rm_offset);
> - b = be64_to_cpu(k2->rmap.rm_offset);
> + a = XFS_RMAP_OFF(be64_to_cpu(k1->rmap.rm_offset));
> + b = XFS_RMAP_OFF(be64_to_cpu(k2->rmap.rm_offset));
> if (a <= b)
> return 1;
> return 0;
> @@ -420,8 +420,8 @@ xfs_rmapbt_recs_inorder(
> return 1;
> else if (a > b)
> return 0;
> - a = be64_to_cpu(r1->rmap.rm_offset);
> - b = be64_to_cpu(r2->rmap.rm_offset);
> + a = XFS_RMAP_OFF(be64_to_cpu(r1->rmap.rm_offset));
> + b = XFS_RMAP_OFF(be64_to_cpu(r2->rmap.rm_offset));
> if (a <= b)
> return 1;
> return 0;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-19 23:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19 23:39 [PATCH] xfs: revert "xfs: fix rmap key and record comparison functions" Darrick J. Wong
2020-11-19 23:44 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).