[LTP] LTP fs_fill test on vfat - ENOSPC

[LTP] LTP fs_fill test on vfat - ENOSPC

[Andrei Gherzan]

Hi all,

tl;dr fs_fill test broken for vfat on systems with more than 256 CPUs.

The fs_fill test runs a fill test on all the supported filesystems. One
of them being vfat. This filesystem is configured dynamically or through
flags/arguments for its file allocation table type (12/16/32).

The size of the test device (which is a loop mounted fs) is 300MB. When
not instructed, mkfs will "automatically select between 12, 16 and 32
bit, whatever fits better for the filesystem size"[1]. For the case of
a 300Mb that wwould end up as FAT16:

# sudo file -s /dev/loop0
/dev/loop0: DOS/MBR boot sector, code offset 0x3c+2, OEM-ID "mkfs.fat",
sectors/cluster 16, reserved sectors 16, root entries 512, Media
descriptor 0xf8, sectors/FAT 160, sectors/track 32, heads 64, sectors
600000 (volumes > 32 MB), serial number 0x5de1e96b, unlabeled, FAT (16
bit)

This is pretty well known, but it's important to start with because it is
linked with another configuration that is the actual impact on this
issue and that is the maximum number of directories in the root of the
filesystem.  FAT12 and FAT16 uses a special system region for "Root
Directory Region". And by default (there is also an argument to
configure this at mkfs-time) this ends up being 256 when no custom
arguments are provided.

On the other hand, the test sets up the filesystem with a
"tst_ncpus_conf + 2" number of directories in the root which can break
the limit explained above on systems that take the value over the
default 256 limit of root directories.

There a couple of ways to deal with this:

1. Force the filesystem creation in FAT32. This would be the best way
forward in my opinion, but I can't see anything that supports doing so on
a per-filesystem basis in LTP.
2. Increase the minimal numbers of entries available in the root
directory via "-r ROOT-DIR-ENTRIES"[1]. This would only push the crash
on fewer systems.
3. Use a subdirectory in the test setup. Something like /subdir/threadX.
If I'm not missing any support to do 1, this would probably be the
easiest to do.

I'm happy to go forward with a PR to fix this properly but looking
forward to your feedback on how to steer this.

[1] MKFS.FAT(8)

-- 
Andrei Gherzan

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

Re: [LTP] LTP fs_fill test on vfat - ENOSPC

[Cyril Hrubis]

Hi!
> There a couple of ways to deal with this:
> 
> 1. Force the filesystem creation in FAT32. This would be the best way
> forward in my opinion, but I can't see anything that supports doing so on
> a per-filesystem basis in LTP.
> 2. Increase the minimal numbers of entries available in the root
> directory via "-r ROOT-DIR-ENTRIES"[1]. This would only push the crash
> on fewer systems.
> 3. Use a subdirectory in the test setup. Something like /subdir/threadX.
> If I'm not missing any support to do 1, this would probably be the
> easiest to do.
> 
> I'm happy to go forward with a PR to fix this properly but looking
> forward to your feedback on how to steer this.

For number 1 we could probably add a special case in the test library,
something as (beware untested):

diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
index 736324f04..0e6e9ebd1 100644
--- a/lib/tst_mkfs.c
+++ b/lib/tst_mkfs.c
@@ -50,6 +50,9 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
                return;
        }

+       if (!strcmp(fs_type, "vfat"))
+               argv[pos++] = "-F 32";
+
        snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);

        if (fs_opts) {


-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] LTP fs_fill test on vfat - ENOSPC

[Andrei Gherzan]

Hi. Thanks for your feedback.

On 23/02/16 10:51AM, Cyril Hrubis wrote:
> Hi!
> > There a couple of ways to deal with this:
> > 
> > 1. Force the filesystem creation in FAT32. This would be the best way
> > forward in my opinion, but I can't see anything that supports doing so on
> > a per-filesystem basis in LTP.
> > 2. Increase the minimal numbers of entries available in the root
> > directory via "-r ROOT-DIR-ENTRIES"[1]. This would only push the crash
> > on fewer systems.
> > 3. Use a subdirectory in the test setup. Something like /subdir/threadX.
> > If I'm not missing any support to do 1, this would probably be the
> > easiest to do.
> > 
> > I'm happy to go forward with a PR to fix this properly but looking
> > forward to your feedback on how to steer this.
> 
> For number 1 we could probably add a special case in the test library,
> something as (beware untested):
> 
> diff --git a/lib/tst_mkfs.c b/lib/tst_mkfs.c
> index 736324f04..0e6e9ebd1 100644
> --- a/lib/tst_mkfs.c
> +++ b/lib/tst_mkfs.c
> @@ -50,6 +50,9 @@ void tst_mkfs_(const char *file, const int lineno, void (cleanup_fn)(void),
>                 return;
>         }
> 
> +       if (!strcmp(fs_type, "vfat"))
> +               argv[pos++] = "-F 32";
> +
>         snprintf(mkfs, sizeof(mkfs), "mkfs.%s", fs_type);
> 
>         if (fs_opts) {

I did consider this as well, but I haven't proposed it initially because
I didn't want to come with a solution that will switch all tests to
Fat32. This is a limitation for tests that create a large enough number
of directories in the root of the filesystem, so I wanted to keep the
scope there.

I have tested, and we will locally go with my proposed 3rd option for
now. Does that sound resonable for a push to upstrem too? 

-- 
Andrei Gherzan

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

Re: [LTP] LTP fs_fill test on vfat - ENOSPC

[Cyril Hrubis]

Hi!
> I did consider this as well, but I haven't proposed it initially because
> I didn't want to come with a solution that will switch all tests to
> Fat32. This is a limitation for tests that create a large enough number
> of directories in the root of the filesystem, so I wanted to keep the
> scope there.
> 
> I have tested, and we will locally go with my proposed 3rd option for
> now. Does that sound resonable for a push to upstrem too? 

Sure, this sounds reasonable as well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] LTP fs_fill test on vfat - ENOSPC

[Andrei Gherzan]

On 23/02/16 11:49AM, Cyril Hrubis wrote:
> Hi!
> > I did consider this as well, but I haven't proposed it initially because
> > I didn't want to come with a solution that will switch all tests to
> > Fat32. This is a limitation for tests that create a large enough number
> > of directories in the root of the filesystem, so I wanted to keep the
> > scope there.
> > 
> > I have tested, and we will locally go with my proposed 3rd option for
> > now. Does that sound resonable for a push to upstrem too? 
> 
> Sure, this sounds reasonable as well.

Great. What's the default in LTP: GitHub PR or mailing list patches?

-- 
Andrei Gherzan

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

Re: [LTP] LTP fs_fill test on vfat - ENOSPC

[Cyril Hrubis]

Hi!
> > > I have tested, and we will locally go with my proposed 3rd option for
> > > now. Does that sound resonable for a push to upstrem too? 
> > 
> > Sure, this sounds reasonable as well.
> 
> Great. What's the default in LTP: GitHub PR or mailing list patches?

We do prefer mailing list.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] LTP fs_fill test on vfat - ENOSPC

[Andrei Gherzan]

On 23/02/16 12:44PM, Cyril Hrubis wrote:
> Hi!
> > > > I have tested, and we will locally go with my proposed 3rd option for
> > > > now. Does that sound resonable for a push to upstrem too? 
> > > 
> > > Sure, this sounds reasonable as well.
> > 
> > Great. What's the default in LTP: GitHub PR or mailing list patches?
> 
> We do prefer mailing list.

Thanks. Sent it over ml.
https://lore.kernel.org/ltp/20230216114745.2389810-1-andrei.gherzan@canonical.com/T/#u

-- 
Andrei Gherzan

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