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

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

> 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);
>  	}

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

FWIW I think you could have merged this after a few days or even
straight away.

-- 
Thank you,
Richard.

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