On Wednesday, 9 January 2019 15:35:05 GMT Andreas Gruenbacher wrote:
> On Wed, 9 Jan 2019 at 14:43, Mark Syms <mark.s...@citrix.com> wrote:
> > We don't yet know how the assert got triggered as we've only seen it once
> > and in the original form it looks like it would be very hard to trigger in
> > any normal case (given that in default usage i_blocks should be at least 8
> > times what any putative value for change could be). So, for the assert to
> > have triggered we've been asked to remove at least 8 times the number of
> > blocks currently allocated to the inode. Possible causes could be a double
> > release or some other higher level bug that will require further
> > investigation to uncover.
> 
> The following change has at least survived xfstests:
> 
> --- a/fs/gfs2/inode.h
> +++ b/fs/gfs2/inode.h
> @@ -61,8 +61,8 @@ static inline u64 gfs2_get_inode_blocks(const struct
> inode *inode)
> 
>  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);
> +    change <<= inode->i_blkbits - 9;
> +    gfs2_assert(GFS2_SB(inode), change >= 0 || inode->i_blocks >= -change);
> inode->i_blocks += change;
>  }
> 
> Andreas

I'll use 

change <<= (GFS2_SB(inode)->sd_sb.sb_bsize_shift - GFS2_BASIC_BLOCK_SHIFT);

for consistency with the gfs2_get/set_inode_blocks(). I'll send the patch in a 
bit.

Given what it was like before, either i_blocks was already 0 or -change 
somehow became stupidly huge. Anything else looks like it would be hard to 
reproduce without mkfs.gfs2 -b 512 (so that GFS2_SB(inode)->sd_sb.sb_bsize == 
GFS2_BASIC_BLOCK) which we don't do.

I'll try to work out what could have caused it and see if we can provoke it 
again.

Out of curiosity I did a few tests where I created a file on GFS2, copied /
dev/null on top of it, and then ran stat on the file. It seems like GFS2 never 
frees the last allocation on truncate; stat always reports 1, 2, 4 or 8 blocks 
in use for a zero-length file depending on the underlying filesystem block 
size, unlike (say) ext3/4 where it reports 0. I presume this is intentional so 
maybe some corner case where it *is* trying to do that is the root of the 
problem.

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


Reply via email to