* [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine @ 2023-02-16 11:47 Andrei Gherzan 2023-02-17 15:27 ` Petr Vorel 0 siblings, 1 reply; 8+ messages in thread From: Andrei Gherzan @ 2023-02-16 11:47 UTC (permalink / raw) To: ltp; +Cc: 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-02-16 11:47 [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine Andrei Gherzan @ 2023-02-17 15:27 ` Petr Vorel 2023-02-20 13:47 ` Andrei Gherzan 2023-03-07 12:43 ` Richard Palethorpe 0 siblings, 2 replies; 8+ messages in thread From: Petr Vorel @ 2023-02-17 15:27 UTC (permalink / raw) To: Andrei Gherzan; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-02-17 15:27 ` Petr Vorel @ 2023-02-20 13:47 ` Andrei Gherzan 2023-03-08 16:55 ` Petr Vorel 2023-03-07 12:43 ` Richard Palethorpe 1 sibling, 1 reply; 8+ messages in thread From: Andrei Gherzan @ 2023-02-20 13:47 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-02-20 13:47 ` Andrei Gherzan @ 2023-03-08 16:55 ` Petr Vorel 2023-03-09 16:24 ` Andrei Gherzan 0 siblings, 1 reply; 8+ messages in thread From: Petr Vorel @ 2023-03-08 16:55 UTC (permalink / raw) To: Andrei Gherzan; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-03-08 16:55 ` Petr Vorel @ 2023-03-09 16:24 ` Andrei Gherzan 2023-03-20 7:51 ` Petr Vorel 0 siblings, 1 reply; 8+ messages in thread From: Andrei Gherzan @ 2023-03-09 16:24 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-03-09 16:24 ` Andrei Gherzan @ 2023-03-20 7:51 ` Petr Vorel 2023-03-20 16:04 ` Andrei Gherzan 0 siblings, 1 reply; 8+ messages in thread From: Petr Vorel @ 2023-03-20 7:51 UTC (permalink / raw) To: Andrei Gherzan; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-03-20 7:51 ` Petr Vorel @ 2023-03-20 16:04 ` Andrei Gherzan 0 siblings, 0 replies; 8+ messages in thread From: Andrei Gherzan @ 2023-03-20 16:04 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine 2023-02-17 15:27 ` Petr Vorel 2023-02-20 13:47 ` Andrei Gherzan @ 2023-03-07 12:43 ` Richard Palethorpe 1 sibling, 0 replies; 8+ messages in thread From: Richard Palethorpe @ 2023-03-07 12:43 UTC (permalink / raw) To: Petr Vorel; +Cc: ltp, Andrei Gherzan 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-20 16:05 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-16 11:47 [LTP] [PATCH] fs_fill: Fix test when running on a 256 CPU+ machine Andrei Gherzan 2023-02-17 15:27 ` Petr Vorel 2023-02-20 13:47 ` Andrei Gherzan 2023-03-08 16:55 ` Petr Vorel 2023-03-09 16:24 ` Andrei Gherzan 2023-03-20 7:51 ` Petr Vorel 2023-03-20 16:04 ` Andrei Gherzan 2023-03-07 12:43 ` Richard Palethorpe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox