Re: [PATCH ] xfs: check for COW overflows in i_delayed_blks

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, May 30, 2019 at 03:20:23PM +0800, Eryu Guan wrote:
> On Tue, May 28, 2019 at 10:01:32AM -0700, Darrick J. Wong wrote:
> > On Sun, May 26, 2019 at 10:27:35PM +0800, Eryu Guan wrote:
> > > On Mon, May 20, 2019 at 03:31:52PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > > 
> > > > With the new copy on write functionality it's possible to reserve so
> > > > much COW space for a file that we end up overflowing i_delayed_blks.
> > > > The only user-visible effect of this is to cause totally wrong i_blocks
> > > > output in stat, so check for that.
> > > > 
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > I hit xfs_db killed by OOM killer (2 vcpu, 8G memory kvm guest) when
> > > trying this test and the test takes too long time (I changed the fs size
> > > from 300T to 300G and tried a test run), perhaps that's why you don't
> > > put it in auto group?
> > 
> > Oh.  Right.  I forget that I patched out xfs_db from
> > check_xfs_filesystem on my dev tree years ago.
> > 
> > Um... do we want to remove xfs_db from the check function?  Or just open
> > code a call to xfs_repair $SCRATCH_MNT/a.img at the end of the test?
> 
> If XFS maintainer removes the xfs_check call in _check_xfs_filesystem(),
> I'd say I like to see it being removed :)

I tried to remove it last year[1] but Dave wanted us to precede that
with a comprehensive analysis of what corruptions are caught via
xfs_repair vs. xfs_check.  I haven't had time to do that.

> > As for the 300T size, the reason I picked that is to force the
> > filesystem to have large enough AGs to support the maximum cowextsize
> > hint.  I'll see if it still works with a 4TB filesystem.
> 
> After removeing the xfs_db call, I can finish the test within 20s on the
> same test vm, and a.img only takes 159MB space on $SCRATCH_DEV, so I
> think 300T fs size is fine.

Hm.  I /do/ have a separate patch banging around in my xfstests tree
that bumps up the oom score adjustment of a test (and lowers it for the
./check process).  One could do a similar thing to the post-test
filesystem check, though at some point the bash code becomes messy.

Or I guess we could just allow a hidden environment variable to turn off
xfs_db in _check_xfs_filesystem and call it explicitly from the test.
What do you all think about that?

--D

[1] https://patchwork.kernel.org/patch/10206103/

> 
> Thanks,
> Eryu