Re: [LTP] [PATCH 2/2] df01.sh: Use tst_fsfreeze for XFS on kernel >= 5.19

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Mar 13, 2023 at 04:33:09PM +0100, Petr Vorel wrote:
> > > On 3/13/23 9:38 AM, Cyril Hrubis wrote:
> > > > Hi!
> > > >> XFS since kernel 5.19 postpone certain operation.  Use LTP fsfreeze
> > > >> implementation to force all the background garbage collection to run
> > > >> to completion.
> 
> > > >> Link: https://lore.kernel.org/linux-block/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/
> > > >> Suggested-by: Eric Sandeen <sandeen@redhat.com>
> > > >> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > >> ---
> > > >>  testcases/commands/df/df01.sh | 7 ++++++-
> > > >>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> > > >> diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh
> > > >> index ae0449c3c..699d1538f 100755
> > > >> --- a/testcases/commands/df/df01.sh
> > > >> +++ b/testcases/commands/df/df01.sh
> > > >> @@ -1,7 +1,7 @@
> > > >>  #!/bin/sh
> > > >>  # SPDX-License-Identifier: GPL-2.0-or-later
> > > >>  # Copyright (c) 2015 Fujitsu Ltd.
> > > >> -# Copyright (c) 2018-2022 Petr Vorel <pvorel@suse.cz>
> > > >> +# Copyright (c) 2018-2023 Petr Vorel <pvorel@suse.cz>
> > > >>  # Author: Zhang Jin <jy_zhangjin@cn.fujitsu.com>
> 
> > > >>  # Test df command with some basic options.
> > > >> @@ -46,6 +46,11 @@ df_test()
> 
> > > >>  	ROD_SILENT rm -rf $TST_MNTPOINT/testimg
> 
> > > >> +	# force all the background garbage collection to run to completion
> > > >> +	if [ "$TST_FS_TYPE" = "xfs" ] && tst_kvcmp -ge "5.19"; then
> > > >> +		tst_fsfreeze $TST_MNTPOINT
> > > >> +	fi
> 
> > > > This looks overly specific, can't we just freeze and unfreeze the FS
> > > > without looking at kernel version? Or will we get errors on older XFS?
> 
> > > > I suppose that this may still start to fail on distribution kernels if
> > > > some of the newer functionality gets backported...
> 
> > So far it's OK on SLES, likely nothing related has been backported to it.
> > I wonder if we should remove the check or just wait till first complaint.
> 
> Well, I haven't seen any problems with older kernels, but I'll retest it more.
> I don't expect any problems, thus I'm OK with removing it.

If the fs doesn't support freezing (e.g. tmpfs), you'll get a -1 return
value and errno == EOPNOTUPP or NOTTY.  Is that going to cause the test
to fail due to tst_brk?

> Kind regards,
> Petr
> 
> > > Yup, I agree. Freeze should be safe for any kernel, I wouldn't condition it either.
> 
> > > (You do want to be very sure that you're not freezing the root fs, tho,
> > > if that is any possibility.)

Why?  Are you worried about not being able to unfreeze the rootfs?
I know fstests has had problems with people doing:

xfs_io -c freeze "/$moo"	# whoops, moo is undefined
<something that blocks>
xfs_io -c thaw "/$moo"		# never gets here

But that's not the case here.

Even if it is the rootfs, the text page containing the two ioctls should
be in memory due to the instruction pointer, and even if it gets paged
out, freezes don't block read faults.

--D

> > $TST_MNTPOINT in on $TMPDIR, which is by default /tmp. In case of /tmp being on
> > root fs we're freezing root tmpfs. But it works on openSUSE, which found the
> > problem.
> 
> > Kind regards,
> > Petr
> 
> > > -Eric
> 
> > > > Otherwise it looks good.
> 
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp