Re: [PATCH] xfs_db: properly set inode type

[Eric Sandeen <sandeen@sandeen.net>]

On 6/28/17 6:54 PM, Darrick J. Wong wrote:
> On Wed, Jun 28, 2017 at 05:45:55PM -0500, Eric Sandeen wrote:
>> On 6/27/17 7:42 PM, Darrick J. Wong wrote:
>>>> +		ino = XFS_AGINO_TO_INO(mp, xfs_daddr_to_agno(mp, b),
>>>> +			((b << BBSHIFT) >> mp->m_sb.sb_inodelog) %
>>>> +			(mp->m_sb.sb_agblocks << mp->m_sb.sb_inopblog));
>>
>>> XFS_OFFBNO_TO_AGINO(mp, xfs_daddr_to_agbno(mp, b), 0) instead of that
>>> long third argument?
>>
>> Hm, nope:
>>
>> xfs_db> inode 99
>> xfs_db> daddr
>> current daddr is 99
>>
>> xfs_db> daddr 99
>> xfs_db> type inode
>> xfs_db> inode
>> current inode number is 96
>>
>>
>> That ends up taking the first inode in the fsblock (12), not the sector
>> (99) I guess.
>>
>> The macro needs a non-zero offset into the fsblock... found by, um...
>> I'm not sure that's going to be much prettier.
>>
>> How much do you hate how I wrote it first? ;)  (I kind of hate it a
>> lot but dunno what else we have?)
> 
> I guess there's also the problem that if inodesize != 512 then what are
> we targeting, anyway?  If inodesize = 256 then we can only hit
> even-numbered inodes (not so bad) but if inodesize > 512 then do we jump
> back to wherever the inode starts?  Or just give the user what they
> asked for, even if it's garbage? 

Oh, hum. Right, big inodes.... 

So I'm trying to go from disk location to inode number, but the current
disk location may not even be the start of an inode.  Hell.  Well, I'm
sure I could craft a test to disallow "type inode" if the current location
cannot be on an inode boundary.  But is that too clever?  Should the
debug tool just do what the user asks?

> (FWIW I was fine with xfs_db being dumb and giving you exactly what you
> point it at, even if that makes no sense. :P)

yeah, if you do "daddr 0" "type agf" it'll squawk, too, so it's fine to
do the dumb user's bidding...

The difference here is that to set things up properly we need "an" inode
number.  Probably best to reject locations that cannot be the starts
of inodes, I guess, but that kind of violates the spirit of a debug tool.
Maybe we're /looking/ for misaligned inodes ...

Bleah.

-Eric



> --D
> 
>>
>> -Eric
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>