Bug 950691 - xfs_repair emits "7f61c041b700: Badness in key lookup (length)"
Summary: xfs_repair emits "7f61c041b700: Badness in key lookup (length)"
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: xfsprogs
Version: 6.3
Hardware: x86_64
OS: Linux
medium
medium
Target Milestone: rc
: ---
Assignee: Eric Sandeen
QA Contact: Boris Ranto
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-04-10 16:39 UTC by Eric Sandeen
Modified: 2013-11-21 21:19 UTC (History)
3 users (show)

Fixed In Version: xfsprogs-3.1.1-13.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-11-21 21:19:32 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2013:1657 normal SHIPPED_LIVE xfsprogs bug fix update 2013-11-20 21:53:13 UTC

Description Eric Sandeen 2013-04-10 16:39:41 UTC
xfs_repair sometimes emits "7f61c041b700: Badness in key lookup (length)"

While this should be harmless, it's concerning to the user running xfs_repair.

It's a result of get_agino_buf() reading in a full inode cluster's worth of data, and populating the cache with that length.

Then i.e. pf_queuing_worker() reads the same block but with a shorter length, XFS_INODE_CLUSTER_SIZE(mp).

Upstream this is gone simply because:

2b00e6325060e22b6a749abf0d7fc1ea50b1fd6 repair: do not walk the unlinked inode list

removed the only caller of get_agino_buf(), so we never get the larger reads of the same block.

We could possibly backport that commit, or alternately tell get_agino_buf() to read a smaller length, i.e.:

diff --git a/repair/dinode.c b/repair/dinode.c
index 5a2da39..442a82c 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -834,7 +834,7 @@ get_agino_buf(xfs_mount_t    *mp,
        if ((irec = find_inode_rec(mp, agno, agino)) == NULL)
                return(NULL);
 
-       size = XFS_FSB_TO_BB(mp, MAX(1, XFS_INODES_PER_CHUNK/inodes_per_block));
+       size = XFS_FSB_TO_BB(mp, MAX(1, XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_blocklog));
        bp = libxfs_readbuf(mp->m_dev, XFS_AGB_TO_DADDR(mp, agno,
                XFS_AGINO_TO_AGBNO(mp, irec->ino_startnum)), size, 0);
        if (!bp) {


which I think is safe, but need to look at it a bit more.

Comment 1 Dave Chinner 2013-05-24 05:50:03 UTC
Eric,

having looked at teh patch in question, I think that back porting the patch is less risky that changing the logic in get_ino_buf(). Especially as the logic in get_agino_buf makes the assumption that all the inodes in a inobt record is in a single buffer, and your buffer length change makes that no longer true.

-Dave.

Comment 2 Eric Sandeen 2013-05-24 16:22:39 UTC
Thanks Dave.

Comment 7 Eric Sandeen 2013-07-25 17:18:32 UTC
Built in xfsprogs-3.1.1-13.el6.src.rpm

Comment 10 errata-xmlrpc 2013-11-21 21:19:32 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-1657.html


Note You need to log in before you can comment on or make changes to this bug.