Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine

Petr Vorel
2023-02-17T15:28:02Z

Hi Andrei,

> 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]. In the case of a 300Mb that
> would end up as FAT16.
Interesting. BTW we plan to change 300 MB to minimal filesystem which would fit
to all existing tests (255 MB was for Btrfs, 300 MB was for XFS, but there might
be minimal systems which can use vfat, ext4, ... with smaller resources, e.g.
16 MB for filesystem). Therefore I wonder what is minimal reasonable required
size for vfat. i.e. what MB is required for FAT32? (I guess we don't want to
check FAT12 or FAT16).

> This is linked with another configuration that is the actual impact on
> this issue: the maximum number of directories in the root of the
> filesystem. FAT12 and FAT16 use a special system region: "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 test filesystem's root
> which can break the limit explained above on systems with a number of
> CPUs that would amount above the limit.

> This change addresses this situation by using a subdirectory in the test
> filesystem which is not subject to this limit.

> Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com>
> ---
> testcases/kernel/fs/fs_fill/fs_fill.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)

> diff --git a/testcases/kernel/fs/fs_fill/fs_fill.c b/testcases/kernel/fs/fs_fill/fs_fill.c
> index 2027b6df1..dd8ad2935 100644
> --- a/testcases/kernel/fs/fs_fill/fs_fill.c
> +++ b/testcases/kernel/fs/fs_fill/fs_fill.c
> @@ -18,6 +18,7 @@
> #include "tst_test.h"

> #define MNTPOINT "mntpoint"
> +#define THREADS_SUBDIR "subdir"

> static volatile int run;
> static unsigned int nthreads;
> @@ -99,9 +100,13 @@ static void setup(void)
> nthreads = tst_ncpus_conf() + 2;
> workers = SAFE_MALLOC(sizeof(struct worker) * nthreads);

> + // Avoid creating the thread directories in the root of the filesystem
> + // to not hit the root entries limit on a FAT16 filesystem.
> + SAFE_MKDIR(MNTPOINT "/" THREADS_SUBDIR, 0700);
> +
> for (i = 0; i < nthreads; i++) {
> snprintf(workers[i].dir, sizeof(workers[i].dir),
> - MNTPOINT "/thread%i", i + 1);
> + MNTPOINT "/" THREADS_SUBDIR "/thread%i", i + 1);
> SAFE_MKDIR(workers[i].dir, 0700);
> }

Reviewed-by: Petr Vorel <pvorel@suse.cz>

If you don't mind, I suggest to merge slightly changed patch.

Kind regards,
Petr

+++ testcases/kernel/fs/fs_fill/fs_fill.c
@@ -18,6 +18,7 @@
#include "tst_test.h"

#define MNTPOINT "mntpoint"
+#define THREADS_DIR MNTPOINT "/subdir"

static volatile int run;
static unsigned int nthreads;
@@ -99,9 +100,15 @@ static void setup(void)
nthreads = tst_ncpus_conf() + 2;
workers = SAFE_MALLOC(sizeof(struct worker) * nthreads);

+ /*
+ * Avoid creating the thread directories in the root of the filesystem
+ * to not hit the root entries limit on a FAT16 filesystem.
+ */
+ SAFE_MKDIR(THREADS_DIR, 0700);
+
for (i = 0; i < nthreads; i++) {
snprintf(workers[i].dir, sizeof(workers[i].dir),
- MNTPOINT "/thread%i", i + 1);
+ THREADS_DIR "/thread%i", i + 1);
SAFE_MKDIR(workers[i].dir, 0700);
}