On Wed, 9 Jan 2019 at 18:14, Tim Smith <tim.sm...@citrix.com> 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.
Yes, that's intentional. An empty file without extended attributes
consumes one block. Extended attributes consume at least one
additional block.
Andreas
