Hi,
On 09/01/2019 17:14, Tim Smith wrote:
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.
That is because gfs2 uses a block for each inode, so it makes sense to
account for it in that way. For ext* the inodes are in separate areas of
the disk, and they only use up a partial block, and they are also
counted separately too. So it is a historical design difference I think,
Steve.
