| Message ID | 20231005134504.3828-1-mkittler@suse.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1] Fix memcontrol tests under Tumbleweed | expand |
Hi! > These tests use `all_filesystems` in combination with > `dev_min_size` which does not work on Tumbleweed as it results > in: > > ``` > tst_test.c:1644: TINFO: === Testing on xfs === > tst_test.c:1099: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts='' > Filesystem must be larger than 300MB. > … > tst_test.c:1105: TBROK: mkfs.xfs failed with exit code 1 > ``` > > This is likely a limitation introduced in newer kernel versions. > > Increasing `dev_min_size` to 300 fixes the tests. In > `memcontrol03` and `memcontrol04` the `dev_min_size` setting > can be dropped completely. Shouldn't we just remove all of dev_min_size records? We already have DEV_SIZE_MB set to 300 in lib/tst_device.c so with no limits we will use the default 300Mb.
Am Donnerstag, 12. Oktober 2023, 11:19:44 CEST schrieb Cyril Hrubis: > > Shouldn't we just remove all of dev_min_size records? > > We already have DEV_SIZE_MB set to 300 in lib/tst_device.c so with no > limits we will use the default 300Mb. I thought so, too. However, when running this particular test without this minimum specified explicitly, it runs into the following error: ``` tst_test.c:1650: TINFO: === Testing on tmpfs === tst_test.c:1105: TINFO: Skipping mkfs for TMPFS filesystem tst_test.c:1086: TINFO: Limiting tmpfs size to 32MB tst_test.c:1119: TINFO: Mounting ltp-tmpfs to /tmp/LTP_memkrqX1e/mntdir fstyp=tmpfs flags=0 memcontrol02.c:93: TPASS: Expect: (current=0) == 0 memcontrol02.c:99: TINFO: Added proc to memcg: memory.current=262144 memcontrol02.c:46: TPASS: Expect: (memory.current=52690944) >= (size=52428800) memcontrol02.c:51: TPASS: Expect: (memory.stat.anon=52449280) > 0 memcontrol02.c:52: TPASS: Expect: (size=52428800) ~= (memory.stat.anon=52449280) memcontrol02.c:54: TPASS: Expect: (memory.current=52690944) ~= (memory.stat.anon=52449280) memcontrol02.c:93: TPASS: Expect: (current=0) == 0 memcontrol02.c:99: TINFO: Added proc to memcg: memory.current=262144 memcontrol02.c:69: TINFO: Created temp file: memory.current=262144 memcontrol_common.h:34: TBROK: write(9,0x7ffda0f93710,8192) failed: ENOSPC (28) ``` Judging by the 3rd TINFO message the size for tmpfs filesystems is intentionally limited to 32MB which presumably also makes generally sense. However, here we *really* need more space. This is most likely also the reason why this test had `.dev_min_size = 256,` before in the first place. The other tests don't need it, though (and I guess the `.dev_min_size = 256,` had just been copied over from the first test).
Hi Marius, Cyril, > Am Donnerstag, 12. Oktober 2023, 11:19:44 CEST schrieb Cyril Hrubis: > > Shouldn't we just remove all of dev_min_size records? > > We already have DEV_SIZE_MB set to 300 in lib/tst_device.c so with no > > limits we will use the default 300Mb. > I thought so, too. However, when running this particular test without this > minimum specified explicitly, it runs into the following error: > ``` > tst_test.c:1650: TINFO: === Testing on tmpfs === > tst_test.c:1105: TINFO: Skipping mkfs for TMPFS filesystem > tst_test.c:1086: TINFO: Limiting tmpfs size to 32MB > tst_test.c:1119: TINFO: Mounting ltp-tmpfs to /tmp/LTP_memkrqX1e/mntdir > fstyp=tmpfs flags=0 > memcontrol02.c:93: TPASS: Expect: (current=0) == 0 > memcontrol02.c:99: TINFO: Added proc to memcg: memory.current=262144 > memcontrol02.c:46: TPASS: Expect: (memory.current=52690944) >= (size=52428800) > memcontrol02.c:51: TPASS: Expect: (memory.stat.anon=52449280) > 0 > memcontrol02.c:52: TPASS: Expect: (size=52428800) ~= > (memory.stat.anon=52449280) > memcontrol02.c:54: TPASS: Expect: (memory.current=52690944) ~= > (memory.stat.anon=52449280) > memcontrol02.c:93: TPASS: Expect: (current=0) == 0 > memcontrol02.c:99: TINFO: Added proc to memcg: memory.current=262144 > memcontrol02.c:69: TINFO: Created temp file: memory.current=262144 > memcontrol_common.h:34: TBROK: write(9,0x7ffda0f93710,8192) failed: ENOSPC (28) > ``` > Judging by the 3rd TINFO message the size for tmpfs filesystems is > intentionally limited to 32MB which presumably also makes generally sense. > However, here we *really* need more space. This is most likely also the reason > why this test had `.dev_min_size = 256,` before in the first place. The other > tests don't need it, though (and I guess the `.dev_min_size = 256,` had just > been copied over from the first test). +1. Although it's unlikely we would really implement smaller loop device on demand (nobody was really interested and 300 MB for a rootfs is not that much nowadays), IMHO it's better to keep the real size request in case we really implement it one day. BTW more than 300 MB in the error message is 301 MB or more, right? I'm for 300 MB if it works (I suppose you have checked it), but XFS developers are wrong :). Reviewed-by: Petr Vorel <pvorel@suse.cz> Thanks for handling this long standing issue (it looks like nobody runs controllers on mainline :(. Kind regards, Petr
Hi! > I thought so, too. However, when running this particular test without this > minimum specified explicitly, it runs into the following error: Ah, right, we create the tmpfs with size 32MB by default, but the test creates 50MB worth of files so it needs slightly more than that, I would say 60MB. However the .dev_min_size does limit the size of other FS types as well, so we can't se this easily. I suppose that we need the per fs minimal size stored in the library as we discussed with Peter back then and with that we can set the real minimum in the test and override when filesystem needs more.
> Hi! > > I thought so, too. However, when running this particular test without this > > minimum specified explicitly, it runs into the following error: > Ah, right, we create the tmpfs with size 32MB by default, but the test > creates 50MB worth of files so it needs slightly more than that, I would > say 60MB. However the .dev_min_size does limit the size of other FS > types as well, so we can't se this easily. I suppose that we need the > per fs minimal size stored in the library as we discussed with Peter > back then and with that we can set the real minimum in the test and > override when filesystem needs more. I guess I should finish this implementation, thanks for reminding me. Kind regards, Petr
diff --git a/testcases/kernel/controllers/memcg/memcontrol02.c b/testcases/kernel/controllers/memcg/memcontrol02.c index 1656176b6..0d93abd9e 100644 --- a/testcases/kernel/controllers/memcg/memcontrol02.c +++ b/testcases/kernel/controllers/memcg/memcontrol02.c @@ -134,7 +134,7 @@ static struct tst_test test = { .tcnt = 2, .test = test_memcg_current, .mount_device = 1, - .dev_min_size = 256, + .dev_min_size = 300, .mntpoint = TMPDIR, .all_filesystems = 1, .forks_child = 1, diff --git a/testcases/kernel/controllers/memcg/memcontrol03.c b/testcases/kernel/controllers/memcg/memcontrol03.c index bc726f395..9c6c808e0 100644 --- a/testcases/kernel/controllers/memcg/memcontrol03.c +++ b/testcases/kernel/controllers/memcg/memcontrol03.c @@ -239,7 +239,6 @@ static struct tst_test test = { .cleanup = cleanup, .test_all = test_memcg_min, .mount_device = 1, - .dev_min_size = 256, .mntpoint = TMPDIR, .all_filesystems = 1, .skip_filesystems = (const char *const[]){ diff --git a/testcases/kernel/controllers/memcg/memcontrol04.c b/testcases/kernel/controllers/memcg/memcontrol04.c index c963a1cd8..32a0b9fd4 100644 --- a/testcases/kernel/controllers/memcg/memcontrol04.c +++ b/testcases/kernel/controllers/memcg/memcontrol04.c @@ -232,7 +232,6 @@ static struct tst_test test = { .cleanup = cleanup, .test_all = test_memcg_low, .mount_device = 1, - .dev_min_size = 256, .mntpoint = TMPDIR, .all_filesystems = 1, .skip_filesystems = (const char *const[]){
These tests use `all_filesystems` in combination with `dev_min_size` which does not work on Tumbleweed as it results in: ``` tst_test.c:1644: TINFO: === Testing on xfs === tst_test.c:1099: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts='' Filesystem must be larger than 300MB. … tst_test.c:1105: TBROK: mkfs.xfs failed with exit code 1 ``` This is likely a limitation introduced in newer kernel versions. Increasing `dev_min_size` to 300 fixes the tests. In `memcontrol03` and `memcontrol04` the `dev_min_size` setting can be dropped completely. Signed-off-by: Marius Kittler <mkittler@suse.de> --- testcases/kernel/controllers/memcg/memcontrol02.c | 2 +- testcases/kernel/controllers/memcg/memcontrol03.c | 1 - testcases/kernel/controllers/memcg/memcontrol04.c | 1 - 3 files changed, 1 insertion(+), 3 deletions(-)