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

[Petr Vorel <pvorel@suse.cz>]

> 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?

FYI I haven't noticed any problem so far, but need to test more.

> > 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.

Yes, if you mean "/", that's not going to happen ($TST_MNTPOINT is never going
to be "/").  I'm sorry, I mixed up mountpoint (used by fsfreeze and therefore
also tst_fsfreeze.c) and device (=> there shouldn't be a problem).

Kind regards,
Petr

> 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