On Tuesday, 8 January 2019 13:32:20 GMT Mark Syms wrote:
> Hi,
> 
> We've seen this in testing with 4.19.
> 
> Full trace is at the bottom.
> 
> Looking at the code though it looks like it will assert if the value of
> change is equal to the number of blocks currently allocated to the inode.
> Is this expected or should the assert be using >= instead of > ?

It's weirder. Looking at

static inline void gfs2_add_inode_blocks(struct inode *inode, s64 change)
{
        gfs2_assert(GFS2_SB(inode), (change >= 0 || inode->i_blocks > -change));
        change *= (GFS2_SB(inode)->sd_sb.sb_bsize/GFS2_BASIC_BLOCK);
        inode->i_blocks += change;
}

where the BUG is happening, "inode->i_blocks" is counting of 512b blocks and
"change" is in units of whatever-the-superblock-said which will probably be
counting 4096b blocks, so the comparison makes no sense.

It should probably read

static inline void gfs2_add_inode_blocks(struct inode *inode, s64 change)
{
        change *= (GFS2_SB(inode)->sd_sb.sb_bsize/GFS2_BASIC_BLOCK);
        gfs2_assert(GFS2_SB(inode), (change >= 0 || inode->i_blocks >= 
-change));
        inode->i_blocks += change;
}

so I'll make a patch for that unless someone wants to correct me.

As for why it's triggering, given the current scale difference I can only
assume it's an attempt to remove blocks from something which has none left.
I'd suspect a boundary case in sweep_bh_for_rgrps().

(It *is* possible currently to remove 1 4096b block from something which has
8 512b blocks so you can get to the state of "inode->i_blocks == 0)" It would
only blow up all the time if you did a "mkfs.gfs2 -b 512" which I don't
imagine anyone does that often...)

> Thanks,
> 
>       Mark
> 
> --------------
> 
> 2018-12-24 00:07:03 UTC] [ 3366.209273] kernel BUG at fs/gfs2/inode.h:64!

[snip]

> [2018-12-24 00:07:03 UTC] [ 3366.209802] Call Trace:
> [2018-12-24 00:07:03 UTC] [ 3366.209835]  punch_hole+0xf4f/0x10d0 [gfs2]
> [2018-12-24 00:07:03 UTC] [ 3366.209866]  ? __switch_to_asm+0x40/0x70
> [2018-12-24 00:07:03 UTC] [ 3366.209907]  ? __switch_to_asm+0x40/0x70
> [2018-12-24 00:07:03 UTC] [ 3366.209924]  ? __switch_to_asm+0x40/0x70
> [2018-12-24 00:07:03 UTC] [ 3366.209949]  ? punch_hole+0xb3e/0x10d0 [gfs2]
> [2018-12-24 00:07:03 UTC] [ 3366.209983]  gfs2_evict_inode+0x57b/0x680 [gfs2]
> [2018-12-24 00:07:03 UTC] [ 3366.210036]  ? 
> __inode_wait_for_writeback+0x75/0xe0
> [2018-12-24 00:07:03 UTC] [ 3366.210066]  ? gfs2_evict_inode+0x1c7/0x680 
> [gfs2]
> [2018-12-24 00:07:03 UTC] [ 3366.210090]  evict+0xc6/0x1a0
> [2018-12-24 00:07:03 UTC] [ 3366.210151]  delete_work_func+0x60/0x70 [gfs2]
> [2018-12-24 00:07:03 UTC] [ 3366.210177]  process_one_work+0x165/0x370
> [2018-12-24 00:07:03 UTC] [ 3366.210197]  worker_thread+0x49/0x3e0
> [2018-12-24 00:07:03 UTC] [ 3366.210216]  kthread+0xf8/0x130
> [2018-12-24 00:07:03 UTC] [ 3366.210235]  ? rescuer_thread+0x310/0x310
> [2018-12-24 00:07:03 UTC] [ 3366.210284]  ? kthread_bind+0x10/0x10
> [2018-12-24 00:07:04 UTC] [ 3366.210301]  ret_from_fork+0x35/0x40


-- 
Tim Smith <tim.sm...@citrix.com>


Reply via email to