diff mbox series

[RFC,1/1] API: Allow to use xfs filesystems < 300 MB

Message ID 20220817204015.31420-1-pvorel@suse.cz
State Superseded
Headers show
Series [RFC,1/1] API: Allow to use xfs filesystems < 300 MB | expand

Commit Message

Petr Vorel Aug. 17, 2022, 8:40 p.m. UTC
mkfs.xfs since v5.19.0-rc1 [1] refuses to create filesystems < 300 MB.
Reuse workaround intended for fstests: set 3 environment variables:
export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1

Workaround added to both C API (for .needs_device) and shell API (for
TST_NEEDS_DEVICE=1).

Fix includes any use of filesystem (C API: .all_filesystems,
.format_device, shell API: TST_MOUNT_DEVICE=1, TST_FORMAT_DEVICE=1).

Fixes various C and shell API failures, e.g.:

./mkfs01.sh -f xfs
mkfs01 1 TINFO: timeout per run is 0h 5m 0s
tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
mkfs01 1 TFAIL: 'mkfs -t xfs  -f /dev/loop0 ' failed.
Filesystem must be larger than 300MB.

./creat09
...
tst_test.c:1599: TINFO: Testing on xfs
tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
Filesystem must be larger than 300MB.

Link: https://lore.kernel.org/all/164738662491.3191861.15611882856331908607.stgit@magnolia/

Reported-by: Martin Doucha <mdoucha@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Dave, please next time remember there are other testsuites testing XFS,
not just fstests :). How long do you plan to keep this workaround?

LTP community: do we want to depend on this behavior or we just increase from 256MB to 301 MB
(either for XFS or for all). It might not be a good idea to test size users are required
to use.

Kind regards,
Petr
 lib/tst_test.c            | 7 +++++++
 testcases/lib/tst_test.sh | 6 +++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Darrick J. Wong Aug. 17, 2022, 11:59 p.m. UTC | #1
On Wed, Aug 17, 2022 at 10:40:15PM +0200, Petr Vorel wrote:
> mkfs.xfs since v5.19.0-rc1 [1] refuses to create filesystems < 300 MB.
> Reuse workaround intended for fstests: set 3 environment variables:
> export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
> 
> Workaround added to both C API (for .needs_device) and shell API (for
> TST_NEEDS_DEVICE=1).
> 
> Fix includes any use of filesystem (C API: .all_filesystems,
> .format_device, shell API: TST_MOUNT_DEVICE=1, TST_FORMAT_DEVICE=1).
> 
> Fixes various C and shell API failures, e.g.:
> 
> ./mkfs01.sh -f xfs
> mkfs01 1 TINFO: timeout per run is 0h 5m 0s
> tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> mkfs01 1 TFAIL: 'mkfs -t xfs  -f /dev/loop0 ' failed.
> Filesystem must be larger than 300MB.
> 
> ./creat09
> ...
> tst_test.c:1599: TINFO: Testing on xfs
> tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> Filesystem must be larger than 300MB.
> 
> Link: https://lore.kernel.org/all/164738662491.3191861.15611882856331908607.stgit@magnolia/
> 
> Reported-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Dave, please next time remember there are other testsuites testing XFS,

Dave?? <cough>

> not just fstests :). How long do you plan to keep this workaround?

Forever.  In the ideal world we'll some day get around to restructuring
all the xfstests that do tricky things with sub-500M filesystems, but
that's the unfortunate part of removing support for small disks.

Most of the fstests don't care about the fs size and so they'll run with
the configured storage (some tens or millions of gigabytes) so we're
mostly using the same fs sizes that users are expected to have.

> LTP community: do we want to depend on this behavior or we just increase from 256MB to 301 MB
> (either for XFS or for all). It might not be a good idea to test size users are required
> to use.

It might *not*? <confused>

--D

> 
> Kind regards,
> Petr
>  lib/tst_test.c            | 7 +++++++
>  testcases/lib/tst_test.sh | 6 +++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 4b4dd125d..657348732 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1160,6 +1160,13 @@ static void do_setup(int argc, char *argv[])
>  	if (tst_test->all_filesystems)
>  		tst_test->needs_device = 1;
>  
> +	/* allow to use XFS filesystem < 300 MB */
> +	if (tst_test->needs_device) {
> +		putenv("TEST_DIR=1");
> +		putenv("TEST_DEV=1");
> +		putenv("QA_CHECK_FS=1");
> +	}
> +
>  	if (tst_test->min_cpus > (unsigned long)tst_ncpus())
>  		tst_brk(TCONF, "Test needs at least %lu CPUs online", tst_test->min_cpus);
>  
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 24a3d29d8..b42e54ca1 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -671,7 +671,11 @@ tst_run()
>  
>  	[ "$TST_MOUNT_DEVICE" = 1 ] && TST_FORMAT_DEVICE=1
>  	[ "$TST_FORMAT_DEVICE" = 1 ] && TST_NEEDS_DEVICE=1
> -	[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
> +	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
> +		TST_NEEDS_TMPDIR=1
> +		# allow to use XFS filesystem < 300 MB
> +		export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
> +	fi
>  
>  	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
>  		if [ -z "$TMPDIR" ]; then
> -- 
> 2.37.1
>
Amir Goldstein Aug. 18, 2022, 5:08 a.m. UTC | #2
On Thu, Aug 18, 2022 at 2:59 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, Aug 17, 2022 at 10:40:15PM +0200, Petr Vorel wrote:
> > mkfs.xfs since v5.19.0-rc1 [1] refuses to create filesystems < 300 MB.
> > Reuse workaround intended for fstests: set 3 environment variables:
> > export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
> >
> > Workaround added to both C API (for .needs_device) and shell API (for
> > TST_NEEDS_DEVICE=1).
> >
> > Fix includes any use of filesystem (C API: .all_filesystems,
> > .format_device, shell API: TST_MOUNT_DEVICE=1, TST_FORMAT_DEVICE=1).
> >
> > Fixes various C and shell API failures, e.g.:
> >
> > ./mkfs01.sh -f xfs
> > mkfs01 1 TINFO: timeout per run is 0h 5m 0s
> > tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> > mkfs01 1 TFAIL: 'mkfs -t xfs  -f /dev/loop0 ' failed.
> > Filesystem must be larger than 300MB.
> >
> > ./creat09
> > ...
> > tst_test.c:1599: TINFO: Testing on xfs
> > tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> > Filesystem must be larger than 300MB.
> >
> > Link: https://lore.kernel.org/all/164738662491.3191861.15611882856331908607.stgit@magnolia/
> >
> > Reported-by: Martin Doucha <mdoucha@suse.cz>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Dave, please next time remember there are other testsuites testing XFS,
>
> Dave?? <cough>
>

TBH, it is not about remembering, it is about running integration tests
that catch these test bugs.

Obviously, xfsprogs maintainer (Eric) is running fstests before an
xfsprogs release, but I cannot blame Eric for not running the entire
LTS test suite for xfsprogs release...

I suppose that the bots running LTP on rc kernels might want
to consider also running LTP with rc xfsprogs/e2fsprogs/...
otherwise, those bugs would be caught when *progs hits a distro
that is used to run LTP.

> > not just fstests :). How long do you plan to keep this workaround?
>
> Forever.  In the ideal world we'll some day get around to restructuring
> all the xfstests that do tricky things with sub-500M filesystems, but
> that's the unfortunate part of removing support for small disks.
>

If it's forever, then it should probably have been a command line option.
IIUC, the motivation was to discourage users from formatting too small
filesystems, but if users have a way to do it, they will find it anyway.

Petr,

Notice that the fstests hack was needed for fstests that require MAX fs size,
while the existing LTP lib and tests only have MIN dev size requirement.

> Most of the fstests don't care about the fs size and so they'll run with
> the configured storage (some tens or millions of gigabytes) so we're
> mostly using the same fs sizes that users are expected to have.
>
> > LTP community: do we want to depend on this behavior or we just increase from 256MB to 301 MB
> > (either for XFS or for all). It might not be a good idea to test size users are required
> > to use.
>

For most LTS tests, all you need to do is increase the default (DEV_MIN_SIZE)
from 300MB to 301MB so that's not worth doing any workarounds.

For the 3 memcontrol tests that require dev_min_size = 256 and run on
all_filesystems, it does not look like changing min size is needed at all.

For squashfs01 the xfs limitation is irrelevant, but generally,
If the test min requirement (1MB) is smaller than the lib default,
DEV_MIN_SIZE still meets the test requirement, so why bother
going below the lib default DEV_MIN_SIZE?

Thanks,
Amir.
Petr Vorel Aug. 18, 2022, 9:01 a.m. UTC | #3
> On Wed, Aug 17, 2022 at 10:40:15PM +0200, Petr Vorel wrote:
> > mkfs.xfs since v5.19.0-rc1 [1] refuses to create filesystems < 300 MB.
> > Reuse workaround intended for fstests: set 3 environment variables:
> > export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1

> > Workaround added to both C API (for .needs_device) and shell API (for
> > TST_NEEDS_DEVICE=1).

> > Fix includes any use of filesystem (C API: .all_filesystems,
> > .format_device, shell API: TST_MOUNT_DEVICE=1, TST_FORMAT_DEVICE=1).

> > Fixes various C and shell API failures, e.g.:

> > ./mkfs01.sh -f xfs
> > mkfs01 1 TINFO: timeout per run is 0h 5m 0s
> > tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> > mkfs01 1 TFAIL: 'mkfs -t xfs  -f /dev/loop0 ' failed.
> > Filesystem must be larger than 300MB.

> > ./creat09
> > ...
> > tst_test.c:1599: TINFO: Testing on xfs
> > tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> > Filesystem must be larger than 300MB.

> > Link: https://lore.kernel.org/all/164738662491.3191861.15611882856331908607.stgit@magnolia/

> > Reported-by: Martin Doucha <mdoucha@suse.cz>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Dave, please next time remember there are other testsuites testing XFS,

> Dave?? <cough>
Eh, I'm sorry.

> > not just fstests :). How long do you plan to keep this workaround?

> Forever.  In the ideal world we'll some day get around to restructuring
> all the xfstests that do tricky things with sub-500M filesystems, but
> that's the unfortunate part of removing support for small disks.

> Most of the fstests don't care about the fs size and so they'll run with
> the configured storage (some tens or millions of gigabytes) so we're
> mostly using the same fs sizes that users are expected to have.

Thanks for info.

> > LTP community: do we want to depend on this behavior or we just increase from 256MB to 301 MB
> > (either for XFS or for all). It might not be a good idea to test size users are required
> > to use.

> It might *not*? <confused>
Again, I'm sorry, missing another not. I.e. I suppose normal users will not try
to go below 301MB, therefore LTP probably should not do it either. That's why
RFC.

Kind regards,
Petr

> --D


> > Kind regards,
> > Petr
> >  lib/tst_test.c            | 7 +++++++
> >  testcases/lib/tst_test.sh | 6 +++++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)

> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 4b4dd125d..657348732 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -1160,6 +1160,13 @@ static void do_setup(int argc, char *argv[])
> >  	if (tst_test->all_filesystems)
> >  		tst_test->needs_device = 1;

> > +	/* allow to use XFS filesystem < 300 MB */
> > +	if (tst_test->needs_device) {
> > +		putenv("TEST_DIR=1");
> > +		putenv("TEST_DEV=1");
> > +		putenv("QA_CHECK_FS=1");
> > +	}
> > +
> >  	if (tst_test->min_cpus > (unsigned long)tst_ncpus())
> >  		tst_brk(TCONF, "Test needs at least %lu CPUs online", tst_test->min_cpus);

> > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > index 24a3d29d8..b42e54ca1 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -671,7 +671,11 @@ tst_run()

> >  	[ "$TST_MOUNT_DEVICE" = 1 ] && TST_FORMAT_DEVICE=1
> >  	[ "$TST_FORMAT_DEVICE" = 1 ] && TST_NEEDS_DEVICE=1
> > -	[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
> > +	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
> > +		TST_NEEDS_TMPDIR=1
> > +		# allow to use XFS filesystem < 300 MB
> > +		export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
> > +	fi

> >  	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
> >  		if [ -z "$TMPDIR" ]; then
> > -- 
> > 2.37.1
Petr Vorel Aug. 18, 2022, 9:45 a.m. UTC | #4
> On Thu, Aug 18, 2022 at 2:59 AM Darrick J. Wong <djwong@kernel.org> wrote:

> > On Wed, Aug 17, 2022 at 10:40:15PM +0200, Petr Vorel wrote:
> > > mkfs.xfs since v5.19.0-rc1 [1] refuses to create filesystems < 300 MB.
> > > Reuse workaround intended for fstests: set 3 environment variables:
> > > export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1

> > > Workaround added to both C API (for .needs_device) and shell API (for
> > > TST_NEEDS_DEVICE=1).

> > > Fix includes any use of filesystem (C API: .all_filesystems,
> > > .format_device, shell API: TST_MOUNT_DEVICE=1, TST_FORMAT_DEVICE=1).

> > > Fixes various C and shell API failures, e.g.:

> > > ./mkfs01.sh -f xfs
> > > mkfs01 1 TINFO: timeout per run is 0h 5m 0s
> > > tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> > > mkfs01 1 TFAIL: 'mkfs -t xfs  -f /dev/loop0 ' failed.
> > > Filesystem must be larger than 300MB.

> > > ./creat09
> > > ...
> > > tst_test.c:1599: TINFO: Testing on xfs
> > > tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> > > Filesystem must be larger than 300MB.

> > > Link: https://lore.kernel.org/all/164738662491.3191861.15611882856331908607.stgit@magnolia/

> > > Reported-by: Martin Doucha <mdoucha@suse.cz>
> > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > ---
> > > Dave, please next time remember there are other testsuites testing XFS,

> > Dave?? <cough>


> TBH, it is not about remembering, it is about running integration tests
> that catch these test bugs.
You're right, that's the only reliable thing.

> Obviously, xfsprogs maintainer (Eric) is running fstests before an
> xfsprogs release, but I cannot blame Eric for not running the entire
> LTS test suite for xfsprogs release...
Not expecting that.

> I suppose that the bots running LTP on rc kernels might want
> to consider also running LTP with rc xfsprogs/e2fsprogs/...
That would be great, but don't expect it'd happen.

> otherwise, those bugs would be caught when *progs hits a distro
> that is used to run LTP.
That's what happen on openSUSE.

> > > not just fstests :). How long do you plan to keep this workaround?

> > Forever.  In the ideal world we'll some day get around to restructuring
> > all the xfstests that do tricky things with sub-500M filesystems, but
> > that's the unfortunate part of removing support for small disks.


> If it's forever, then it should probably have been a command line option.
> IIUC, the motivation was to discourage users from formatting too small
> filesystems, but if users have a way to do it, they will find it anyway.
+1

> Petr,

> Notice that the fstests hack was needed for fstests that require MAX fs size,
> while the existing LTP lib and tests only have MIN dev size requirement.

> > Most of the fstests don't care about the fs size and so they'll run with
> > the configured storage (some tens or millions of gigabytes) so we're
> > mostly using the same fs sizes that users are expected to have.

> > > LTP community: do we want to depend on this behavior or we just increase from 256MB to 301 MB
> > > (either for XFS or for all). It might not be a good idea to test size users are required
> > > to use.


> For most LTS tests, all you need to do is increase the default (DEV_MIN_SIZE)
> from 300MB to 301MB so that's not worth doing any workarounds.
FYI DEV_MIN_SIZE is used just for tests which test LTP API itself. For real
tests is DEV_SIZE_MB (lib/tst_device.c). And actually 300 MB is enough,
therefore the only change for the default values is: DEV_SIZE_MB from 256 to 300
(should be enough for all LTP tests: C and shell API, also these tests of LTP
API itself).  I also think 46MB is not worth of hacks.

BTW for some reason DEV_MIN_SIZE was higher than the default (probably to test
library with non-default value, but now is the same, we might want to increase
it (or remove it).

> For the 3 memcontrol tests that require dev_min_size = 256 and run on
> all_filesystems, it does not look like changing min size is needed at all.
Indeed, this value can stay. This check is here because although LTP uses loop
device by default, any device can be used via LTP_DEV env variable.

> For squashfs01 the xfs limitation is irrelevant, but generally,
> If the test min requirement (1MB) is smaller than the lib default,
> DEV_MIN_SIZE still meets the test requirement, so why bother
> going below the lib default DEV_MIN_SIZE?
For use with LTP_DEV.

Anyway, Darrick, Amir, thank you both for your input.

Kind regards,
Petr

> Thanks,
> Amir.
Cyril Hrubis Aug. 18, 2022, 9:53 a.m. UTC | #5
Hi!
I'm starting to wonder if we should start tracking minimal FS size per
filesystem since btrfs and xfs will likely to continue to grow and with
that we will end up disabling the whole fs related testing on embedded
boards with a little disk space. If we tracked that per filesystem we
would be able to skip a subset of filesystems when there is not enough
space. The downside is obviously that we would have to add a bit more
complexity to the test library.
Petr Vorel Aug. 18, 2022, 11:12 a.m. UTC | #6
Hi Cyril,

> Hi!
> I'm starting to wonder if we should start tracking minimal FS size per
> filesystem since btrfs and xfs will likely to continue to grow and with
> that we will end up disabling the whole fs related testing on embedded
> boards with a little disk space. If we tracked that per filesystem we
> would be able to skip a subset of filesystems when there is not enough
> space. The downside is obviously that we would have to add a bit more
> complexity to the test library.

Maybe I could for start rewrite v2 (I've sent it without Cc kernel devs now it's
mainly LTP internal thing) as it just to have 300 MB for XFS and 256 MB for the
rest. That would require to specify filesystem when acquiring device (NULL would
be for the default filesystem), that's would be worth if embedded folks counter
each MB. It'd be nice to hear from them.

Kind regards,
Petr
Cyril Hrubis Aug. 18, 2022, 11:30 a.m. UTC | #7
Hi!
> > I'm starting to wonder if we should start tracking minimal FS size per
> > filesystem since btrfs and xfs will likely to continue to grow and with
> > that we will end up disabling the whole fs related testing on embedded
> > boards with a little disk space. If we tracked that per filesystem we
> > would be able to skip a subset of filesystems when there is not enough
> > space. The downside is obviously that we would have to add a bit more
> > complexity to the test library.
> 
> Maybe I could for start rewrite v2 (I've sent it without Cc kernel devs now it's
> mainly LTP internal thing) as it just to have 300 MB for XFS and 256 MB for the
> rest. That would require to specify filesystem when acquiring device (NULL would
> be for the default filesystem), that's would be worth if embedded folks counter
> each MB. It'd be nice to hear from them.

The 256MB limit was set previously due to btrfs, I bet that we can
create smaller images for ext filesytems for example.
Geert Uytterhoeven Aug. 18, 2022, 11:55 a.m. UTC | #8
Hi Cyril,

On Thu, Aug 18, 2022 at 1:28 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> > > I'm starting to wonder if we should start tracking minimal FS size per
> > > filesystem since btrfs and xfs will likely to continue to grow and with
> > > that we will end up disabling the whole fs related testing on embedded
> > > boards with a little disk space. If we tracked that per filesystem we
> > > would be able to skip a subset of filesystems when there is not enough
> > > space. The downside is obviously that we would have to add a bit more
> > > complexity to the test library.
> >
> > Maybe I could for start rewrite v2 (I've sent it without Cc kernel devs now it's
> > mainly LTP internal thing) as it just to have 300 MB for XFS and 256 MB for the
> > rest. That would require to specify filesystem when acquiring device (NULL would
> > be for the default filesystem), that's would be worth if embedded folks counter
> > each MB. It'd be nice to hear from them.
>
> The 256MB limit was set previously due to btrfs, I bet that we can
> create smaller images for ext filesytems for example.

Yeah, we used to have ext2 root file systems that fit on 1440 KiB floppies.
IIRC, ext3 does have a minimum size of 32 MiB or so.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Petr Vorel Aug. 19, 2022, 7:28 p.m. UTC | #9
Hi all,

> Hi Cyril,

> On Thu, Aug 18, 2022 at 1:28 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> > > > I'm starting to wonder if we should start tracking minimal FS size per
> > > > filesystem since btrfs and xfs will likely to continue to grow and with
> > > > that we will end up disabling the whole fs related testing on embedded
> > > > boards with a little disk space. If we tracked that per filesystem we
> > > > would be able to skip a subset of filesystems when there is not enough
> > > > space. The downside is obviously that we would have to add a bit more
> > > > complexity to the test library.

> > > Maybe I could for start rewrite v2 (I've sent it without Cc kernel devs now it's
> > > mainly LTP internal thing) as it just to have 300 MB for XFS and 256 MB for the
> > > rest. That would require to specify filesystem when acquiring device (NULL would
> > > be for the default filesystem), that's would be worth if embedded folks counter
> > > each MB. It'd be nice to hear from them.

> > The 256MB limit was set previously due to btrfs, I bet that we can
> > create smaller images for ext filesytems for example.

Thanks for input, Geert!

> Yeah, we used to have ext2 root file systems that fit on 1440 KiB floppies.
These nice times when everything simple hadn't been solved yet ... :).
> IIRC, ext3 does have a minimum size of 32 MiB or so.
Interesting, I was able to create smaller.

I did some testing minimal size (verified on chdir01 test):
XFS: 300 MB, btrfs: 109 MB, ntfs: 2 MB, ext3: 2 MB, ext[24]: 1 MB, vfat: 1 MB, exfat: 1 MB.

I guess using XFS: 300 MB, btrfs: 109 MB and 16 MB for the rest could be enough.
But that would require to run all tests to see how many tests actually use
bigger data.

Kind regards,
Petr

> Gr{oetje,eeting}s,

>                         Geert
Li Wang Aug. 20, 2022, 2:37 a.m. UTC | #10
Hi Petr, All,

On Sat, Aug 20, 2022 at 3:28 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> > Hi Cyril,
>
> > On Thu, Aug 18, 2022 at 1:28 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> > > > > I'm starting to wonder if we should start tracking minimal FS size
> per
> > > > > filesystem since btrfs and xfs will likely to continue to grow and
> with
> > > > > that we will end up disabling the whole fs related testing on
> embedded
> > > > > boards with a little disk space. If we tracked that per filesystem
> we
> > > > > would be able to skip a subset of filesystems when there is not
> enough
> > > > > space. The downside is obviously that we would have to add a bit
> more
> > > > > complexity to the test library.
>
> > > > Maybe I could for start rewrite v2 (I've sent it without Cc kernel
> devs now it's
> > > > mainly LTP internal thing) as it just to have 300 MB for XFS and 256
> MB for the
> > > > rest. That would require to specify filesystem when acquiring device
> (NULL would
> > > > be for the default filesystem), that's would be worth if embedded
> folks counter
> > > > each MB. It'd be nice to hear from them.
>
> > > The 256MB limit was set previously due to btrfs, I bet that we can
> > > create smaller images for ext filesytems for example.
>
> Thanks for input, Geert!
>
> > Yeah, we used to have ext2 root file systems that fit on 1440 KiB
> floppies.
> These nice times when everything simple hadn't been solved yet ... :).
> > IIRC, ext3 does have a minimum size of 32 MiB or so.
> Interesting, I was able to create smaller.
>
> I did some testing minimal size (verified on chdir01 test):
> XFS: 300 MB, btrfs: 109 MB, ntfs: 2 MB, ext3: 2 MB, ext[24]: 1 MB, vfat: 1
> MB, exfat: 1 MB.
>
> I guess using XFS: 300 MB, btrfs: 109 MB and 16 MB for the rest could be
> enough.
>

I think so, tracking minimal FS size per FS is a practical idea.
But one thing we have to be aware of is that there may be different
minimal sizes for each FS version.
(so we'd better choose the maximum of minimal sizes).

16MB for general FS should be fine, I will help to test that if someone
works out the patch.



> But that would require to run all tests to see how many tests actually use
> bigger data.
>

Absolutely YES!
Li Wang Aug. 20, 2022, 2:52 a.m. UTC | #11
On Thu, Aug 18, 2022 at 5:51 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> I'm starting to wonder if we should start tracking minimal FS size per
> filesystem since btrfs and xfs will likely to continue to grow and with
> that we will end up disabling the whole fs related testing on embedded
>

So from this point, it still seems to make sense to have this patch[1].
Canceling the limit of 300MB for XFS will benefit embedded system.

[1] export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
Petr Vorel Aug. 22, 2022, 9:36 a.m. UTC | #12
Hi Li, all,

> Hi Petr, All,

> On Sat, Aug 20, 2022 at 3:28 AM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > > Hi Cyril,

> > > On Thu, Aug 18, 2022 at 1:28 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> > > > > > I'm starting to wonder if we should start tracking minimal FS size
> > per
> > > > > > filesystem since btrfs and xfs will likely to continue to grow and
> > with
> > > > > > that we will end up disabling the whole fs related testing on
> > embedded
> > > > > > boards with a little disk space. If we tracked that per filesystem
> > we
> > > > > > would be able to skip a subset of filesystems when there is not
> > enough
> > > > > > space. The downside is obviously that we would have to add a bit
> > more
> > > > > > complexity to the test library.

> > > > > Maybe I could for start rewrite v2 (I've sent it without Cc kernel
> > devs now it's
> > > > > mainly LTP internal thing) as it just to have 300 MB for XFS and 256
> > MB for the
> > > > > rest. That would require to specify filesystem when acquiring device
> > (NULL would
> > > > > be for the default filesystem), that's would be worth if embedded
> > folks counter
> > > > > each MB. It'd be nice to hear from them.

> > > > The 256MB limit was set previously due to btrfs, I bet that we can
> > > > create smaller images for ext filesytems for example.

> > Thanks for input, Geert!

> > > Yeah, we used to have ext2 root file systems that fit on 1440 KiB
> > floppies.
> > These nice times when everything simple hadn't been solved yet ... :).
> > > IIRC, ext3 does have a minimum size of 32 MiB or so.
> > Interesting, I was able to create smaller.

> > I did some testing minimal size (verified on chdir01 test):
> > XFS: 300 MB, btrfs: 109 MB, ntfs: 2 MB, ext3: 2 MB, ext[24]: 1 MB, vfat: 1
> > MB, exfat: 1 MB.

> > I guess using XFS: 300 MB, btrfs: 109 MB and 16 MB for the rest could be
> > enough.


> I think so, tracking minimal FS size per FS is a practical idea.
> But one thing we have to be aware of is that there may be different
> minimal sizes for each FS version.
> (so we'd better choose the maximum of minimal sizes).

> 16MB for general FS should be fine, I will help to test that if someone
> works out the patch.

So should we combine both: minimal FS size and those XFS variables which would
allow to use lower size for XFS? I wonder which which size would be relevant,
it might be safer to use 64 MB:

https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/tree/mkfs/xfs_mkfs.c
/*
 * Realistically, the log should never be smaller than 64MB.  Studies by the
 * kernel maintainer in early 2022 have shown a dramatic reduction in long tail
 * latency of the xlog grant head waitqueue when running a heavy metadata
 * update workload when the log size is at least 64MB.
 */

Because there is really not a big difference between 256MB and 300MB.

Kind regards,
Petr

> > But that would require to run all tests to see how many tests actually use
> > bigger data.


> Absolutely YES!
Petr Vorel Aug. 24, 2022, 10:21 a.m. UTC | #13
Hi all,

Could we in the end accept this patch?

It'd fix the issue for now and I could set size of the XFS loop device smaller
than 300 MB (better for embedded). i.e. 16 MB (or 32 or 64 MB or anything higher
if XFS developers are convinced it's needed).

As I wrote before I plan to suggest sizes:
btrfs 110 MB
the rest (ext[234], xfs, ntfs, vfat, exfat, tmpfs): 16 MB

Based on quick test:
https://lore.kernel.org/ltp/Yv%2FkVXSK0xJGb3RO@pevik/
btrfs: 109 MB, ntfs: 2 MB, ext3: 2 MB, ext[24]: 1 MB, vfat: 1 MB, exfat: 1 MB.

Kind regards,
Petr

> mkfs.xfs since v5.19.0-rc1 [1] refuses to create filesystems < 300 MB.
> Reuse workaround intended for fstests: set 3 environment variables:
> export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1

> Workaround added to both C API (for .needs_device) and shell API (for
> TST_NEEDS_DEVICE=1).

> Fix includes any use of filesystem (C API: .all_filesystems,
> .format_device, shell API: TST_MOUNT_DEVICE=1, TST_FORMAT_DEVICE=1).

> Fixes various C and shell API failures, e.g.:

> ./mkfs01.sh -f xfs
> mkfs01 1 TINFO: timeout per run is 0h 5m 0s
> tst_device.c:89: TINFO: Found free device 0 '/dev/loop0'
> mkfs01 1 TFAIL: 'mkfs -t xfs  -f /dev/loop0 ' failed.
> Filesystem must be larger than 300MB.

> ./creat09
> ...
> tst_test.c:1599: TINFO: Testing on xfs
> tst_test.c:1064: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> Filesystem must be larger than 300MB.

> Link: https://lore.kernel.org/all/164738662491.3191861.15611882856331908607.stgit@magnolia/

> Reported-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Dave, please next time remember there are other testsuites testing XFS,
> not just fstests :). How long do you plan to keep this workaround?

> LTP community: do we want to depend on this behavior or we just increase from 256MB to 301 MB
> (either for XFS or for all). It might not be a good idea to test size users are required
> to use.

> Kind regards,
> Petr
>  lib/tst_test.c            | 7 +++++++
>  testcases/lib/tst_test.sh | 6 +++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)

> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 4b4dd125d..657348732 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -1160,6 +1160,13 @@ static void do_setup(int argc, char *argv[])
>  	if (tst_test->all_filesystems)
>  		tst_test->needs_device = 1;

> +	/* allow to use XFS filesystem < 300 MB */
> +	if (tst_test->needs_device) {
> +		putenv("TEST_DIR=1");
> +		putenv("TEST_DEV=1");
> +		putenv("QA_CHECK_FS=1");
> +	}
> +
>  	if (tst_test->min_cpus > (unsigned long)tst_ncpus())
>  		tst_brk(TCONF, "Test needs at least %lu CPUs online", tst_test->min_cpus);

> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 24a3d29d8..b42e54ca1 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -671,7 +671,11 @@ tst_run()

>  	[ "$TST_MOUNT_DEVICE" = 1 ] && TST_FORMAT_DEVICE=1
>  	[ "$TST_FORMAT_DEVICE" = 1 ] && TST_NEEDS_DEVICE=1
> -	[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
> +	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
> +		TST_NEEDS_TMPDIR=1
> +		# allow to use XFS filesystem < 300 MB
> +		export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
> +	fi

>  	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
>  		if [ -z "$TMPDIR" ]; then
Li Wang Aug. 29, 2022, 1:45 a.m. UTC | #14
Hi Petr, All,


Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> Could we in the end accept this patch?
>
> It'd fix the issue for now and I could set size of the XFS loop device
> smaller
> than 300 MB (better for embedded). i.e. 16 MB (or 32 or 64 MB or anything
> higher
> if XFS developers are convinced it's needed).
>

I personally think YES!
(sorry for replying so late, I was on vacation last week)



>
> As I wrote before I plan to suggest sizes:
> btrfs 110 MB
> the rest (ext[234], xfs, ntfs, vfat, exfat, tmpfs): 16 MB
>

+1 thanks for finding the minimal size.
Petr Vorel April 3, 2024, 2:24 p.m. UTC | #15
Hi all,

> Hi Petr, All,


> Petr Vorel <pvorel@suse.cz> wrote:

> > Hi all,

> > Could we in the end accept this patch?

> > It'd fix the issue for now and I could set size of the XFS loop device
> > smaller
> > than 300 MB (better for embedded). i.e. 16 MB (or 32 or 64 MB or anything
> > higher
> > if XFS developers are convinced it's needed).


> I personally think YES!
> (sorry for replying so late, I was on vacation last week)

I still plan to implement 

Old thread thus recap Cyril's original proposal [1]

	I'm starting to wonder if we should start tracking minimal FS size per
	filesystem since btrfs and xfs will likely to continue to grow and with
	that we will end up disabling the whole fs related testing on embedded
	boards with a little disk space. If we tracked that per filesystem we
	would be able to skip a subset of filesystems when there is not enough
	space. The downside is obviously that we would have to add a bit more
	complexity to the test library.

I'm willing to implement this Cyril's proposal, but what puts me off is that
Btrfs and XFS developers are already complaining that testing on minimal size is
not a realistic testing. fstests are testing on realistic sizes, Darrick J. Wong
[2]:

	... In the ideal world we'll some day get around to restructuring
	all the xfstests that do tricky things with sub-500M filesystems, but
	that's the unfortunate part of removing support for small disks.

	Most of the fstests don't care about the fs size and so they'll run with
	the configured storage (some tens or millions of gigabytes) so we're
	mostly using the same fs sizes that users are expected to have.

Geert Uytterhoeven suggested [3]:
Yeah, we used to have ext2 root file systems that fit on 1440 KiB floppies.
IIRC, ext3 does have a minimum size of 32 MiB or so.

> > As I wrote before I plan to suggest sizes:
> > btrfs 110 MB
> > the rest (ext[234], xfs, ntfs, vfat, exfat, tmpfs): 16 MB

Maybe safer would be (from more realistic point of testing)
* Btrfs, XFS: 300 MB (keep the current size for Btrfs)
* Maybe raise ext4 to 32 MB (more realistic?)
* others: 16 MB

> +1 thanks for finding the minimal size.

Also Amir noted [4] that we have squashfs01.c asks for .dev_min_size = 1, but we
do the minimal default (300 MB now, it would be 16 MB). Do we want to force
.dev_min_size in this case or use the default minimal?

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/Yv4MBF79PnJKJbwm@yuki/
[2] https://lore.kernel.org/ltp/Yv2A9Ggkv%2FNBrTd4@magnolia/
[3] https://lore.kernel.org/ltp/CAMuHMdUMBjCTwPu7wxrnagXnbyVxxmXN+vHmML0Lr=SyrTw0nQ@mail.gmail.com/
[4] https://lore.kernel.org/ltp/CAOQ4uxjMEHYQwO25dhs5WtzbOkJcee0HofQDTT3cD-qXJn7xQw@mail.gmail.com/
diff mbox series

Patch

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 4b4dd125d..657348732 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1160,6 +1160,13 @@  static void do_setup(int argc, char *argv[])
 	if (tst_test->all_filesystems)
 		tst_test->needs_device = 1;
 
+	/* allow to use XFS filesystem < 300 MB */
+	if (tst_test->needs_device) {
+		putenv("TEST_DIR=1");
+		putenv("TEST_DEV=1");
+		putenv("QA_CHECK_FS=1");
+	}
+
 	if (tst_test->min_cpus > (unsigned long)tst_ncpus())
 		tst_brk(TCONF, "Test needs at least %lu CPUs online", tst_test->min_cpus);
 
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 24a3d29d8..b42e54ca1 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -671,7 +671,11 @@  tst_run()
 
 	[ "$TST_MOUNT_DEVICE" = 1 ] && TST_FORMAT_DEVICE=1
 	[ "$TST_FORMAT_DEVICE" = 1 ] && TST_NEEDS_DEVICE=1
-	[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
+	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
+		TST_NEEDS_TMPDIR=1
+		# allow to use XFS filesystem < 300 MB
+		export TEST_DIR=1 TEST_DEV=1 QA_CHECK_FS=1
+	fi
 
 	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
 		if [ -z "$TMPDIR" ]; then