On 8/13/20 6:53 AM, Gao Xiang wrote:
> Hi,
>
> On Thu, Aug 13, 2020 at 02:03:24PM +0800, Zorro Lang wrote:
...
>> diff --git a/db/io.c b/db/io.c
>> index 6628d061..61940a07 100644
>> --- a/db/io.c
>> +++ b/db/io.c
>> @@ -591,6 +591,7 @@ set_iocur_type(
>> /* Inodes are special; verifier checks all inodes in the chunk */
>> if (type->typnm == TYP_INODE) {
>> xfs_daddr_t b = iocur_top->bb;
>> + int bo = iocur_top->boff;
>> xfs_ino_t ino;
>>
>> /*
>> @@ -598,7 +599,7 @@ set_iocur_type(
>> * which contains the current disk location; daddr may change.
>> */
>> ino = XFS_AGINO_TO_INO(mp, xfs_daddr_to_agno(mp, b),
>> - ((b << BBSHIFT) >> mp->m_sb.sb_inodelog) %
>> + (((b << BBSHIFT) + bo) >> mp->m_sb.sb_inodelog) %
>> XFS_AGB_TO_AGINO(mp, mp->m_sb.sb_agblocks));
>> set_cur_inode(ino);
>> return;
>
> Not familar with such code, but after looking into a bit, (my premature
> thought is that) I'm wondering if we need to reverify original buffer in
>
> if (type->fields) {
> ...
> set_cur()
> }
>
> iocur_top->typ = type;
>
> /* verify the buffer if the type has one. */
> ...
>
> since set_cur() already verified the buffer in
> set_cur->libxfs_buf_read->...->libxfs_readbuf_verify?
>
> Not related to this patchset but I'm a bit curious about it now...
I'm wondering about all this, too.
set_cur_inode() actually calls set_cur, which /does/ get into the verifier.
It definitely seems like a mess; the early return from the
if (type->typnm == TYP_INODE) {
block is a little weird, and the explicit verify_read() later in the
function seems like it might be unnecessary?
Agreed that it's unrelated to this bug, though.
-Eric
