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>
