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

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

[Andrei Gherzan]

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.

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


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

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

[Petr Vorel]

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

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

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

[Andrei Gherzan]

On 23/02/17 04:27PM, Petr Vorel wrote:
> 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).

The minimum/maximum values depend on the sector size. I'm not sure about
F12 but for the other ones I think it was 32MB for 512b sectors.

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

Happy with the changes below.

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

-- 
Andrei Gherzan

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

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

[Richard Palethorpe]

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

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

[Petr Vorel]

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

> The minimum/maximum values depend on the sector size. I'm not sure about
> F12 but for the other ones I think it was 32MB for 512b sectors.

Thanks a lot for info. BTW I'm not convinced any more that using as smallest
loop device size as possible is a good idea.

Kind regards,
Petr

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

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

[Andrei Gherzan]

On 23/03/08 05:55PM, Petr Vorel wrote:
> 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).
> 
> > The minimum/maximum values depend on the sector size. I'm not sure about
> > F12 but for the other ones I think it was 32MB for 512b sectors.
> 
> Thanks a lot for info. BTW I'm not convinced any more that using as smallest
> loop device size as possible is a good idea.

I agree. The moment we start getting into the limits of fs sizes, we
will end up in issues like this.

-- 
Andrei Gherzan

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

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

[Petr Vorel]

Hi Andrei, Richie,

merged with proposed fixes.
I'm sorry it took me some time to get back to this.

Kind regards,
Petr

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

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

[Andrei Gherzan]

Thanks for the heads up.

On Mon, 20 Mar 2023 at 07:51, Petr Vorel <pvorel@suse.cz> wrote:

> Hi Andrei, Richie,
>
> merged with proposed fixes.
> I'm sorry it took me some time to get back to this.
>
> Kind regards,
> Petr
>


-- 
Andrei Gherzan

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