zram01: Fix on ppc64le
RFC PATCH 1/1 zram01: Fix on ppc64le
The minimal size for Btrfs filesystem on ppc64le is around 139MB, most likely because of larger page size, hence the test fails.
This commit adds tst_min_fs_size.sh library, that parses mkfs.btrfs output which includes minimal filesystem size in case of a failure.
The code also falls back to 150MB if the mkfs.btrfs output wasn't parsed correctly.
--- testcases/kernel/device-drivers/zram/zram01.sh | 8 ++++-- testcases/lib/tst_min_fs_size.sh | 35 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 testcases/lib/tst_min_fs_size.sh
testcases/kernel/device-drivers/zram/zram01.sh +6 -2
@@ -24,6 +24,7 @@ TCID="zram01" TST_TOTAL=8 . test.sh +. tst_min_fs_size.sh . zram_lib.sh # Test will create the following number of zram devices:
@@ -39,8 +40,11 @@ zram_max_streams="2 3 5 8" # not support mem suffixes, in some newer kernels, they use # memparse() which supports mem suffixes. So here we just use # bytes to make sure everything works correctly. -zram_sizes="26214400 26214400 26214400 41943040" # 25MB, 40MB for btrfs -zram_mem_limits="25M 25M 25M 40M" +min_btrfs_size=$(tst_min_btrfs_size) +min_btrfs_size_mb=$((min_btrfs_size/1024/1024)) + +zram_sizes="26214400 26214400 26214400 $min_btrfs_size" # 25MB, 40MB for btrfs +zram_mem_limits="25M 25M 25M ${min_btrfs_size_mb}M" zram_filesystems="ext3 ext4 xfs btrfs" zram_algs="lzo lzo lzo lzo"
testcases/lib/tst_min_fs_size.sh new file +35 -0
@@ -0,0 +1,35 @@ +#!/bin/sh +# +# Copyright (c) Linux Test Project, 2017 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Written by Cyril Hrubis <[email protected]> +# + +tst_min_btrfs_size() +{ + local output + + output=$(mkfs.btrfs -q /dev/null 2>&1) + output=$(echo $output | awk -F' ' '{print substr($NF, 1, length($NF) - 1)}' -) + + if [ "$output" -a "$output" -eq "$output" ] 2>/dev/null; then + echo "$output" + else + echo "Invalid size ($output) reported by mkfs.btrfs, defaulting to 150MB" 1>&2 + echo $((150 * 1024 * 1024)) + fi +}
Signature · 1 line
2.7.3
RFC PATCH 1/1 zram01: Fix on ppc64le
----- Original Message -----
Quoted 14 lines
From: "Cyril Hrubis" <[email protected]>
To: [email protected]
Sent: Tuesday, 31 January, 2017 2:44:33 PM
Subject: [LTP] [PATCH] [RFC] zram01: Fix on ppc64leThe minimal size for Btrfs filesystem on ppc64le is around 139MB, most likely because of larger page size, hence the test fails.
This commit adds
tst_min_fs_size.shlibrary, that parses mkfs.btrfs output which includes minimal filesystem size in case of a failure.The code also falls back to 150MB if the mkfs.btrfs output wasn't parsed correctly.
Quoted 7 lines
Signed-off-by Cyril Hrubis <[email protected]>--- testcases/kernel/device-drivers/zram/zram01.sh | 8 ++++-- testcases/lib/tst_min_fs_size.sh | 35 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 testcases/lib/tst_min_fs_size.sh
This is ~4 years old comment from Zach Brown, when I hit an issue on ppc, where I could alloc only 1/2 of the volume size:
"That small volume mkfs warning is issued for devices less than a gig. It indicates that btrfs has gone in to a weird degenerate allocation scheme. We'd only support volumes much larger than that, though I have no quick rule to say how large starts to be reasonable. Multiple gig, certainly."
I'm running with 384M since then, so far successfully. If we don't allocate too much data on it, we might be OK, but still I'd go with minimum default of 256M.
Regards,
Jan
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 7 lines
This is ~4 years old comment from Zach Brown, when I hit an issue on ppc, where I could alloc only 1/2 of the volume size:
"That small volume mkfs warning is issued for devices less than a gig. It indicates that btrfs has gone in to a weird degenerate allocation scheme. We'd only support volumes much larger than that, though I have no quick rule to say how large starts to be reasonable. Multiple gig, certainly."
I'm running with 384M since then, so far successfully. If we don't allocate too much data on it, we might be OK, but still I'd go with minimum default of 256M.
What exactly do you have in mind? Using 256MB by default for any Btrfs filesystem or fallback to 256MB if mkfs.btrfs output cannot be parsed?
I guess that for any other testcase it would be fine enough to bump the minimal device size to 256MB unconditionally, but in this case we create the data in RAM albeit compressed, and so I would like to keep it as small as possible, since otherwise it may fail on embedded hardware. Maybe we should just remove Btrfs from the zram01.sh test so that we don't have to keep bumping the minimal size each time the minimal Btrfs size calculation changes...
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
----- Original Message -----
Quoted 15 lines
From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Wednesday, 1 February, 2017 10:45:41 AM
Subject: Re: [LTP] [PATCH] [RFC] zram01: Fix on ppc64leHi!
Quoted 8 lines
This is ~4 years old comment from Zach Brown, when I hit an issue on ppc, where I could alloc only 1/2 of the volume size:
"That small volume mkfs warning is issued for devices less than a gig. It indicates that btrfs has gone in to a weird degenerate allocation scheme. We'd only support volumes much larger than that, though I have no quick rule to say how large starts to be reasonable. Multiple gig, certainly."
Quoted 6 lines
Quoted 3 lines
I'm running with 384M since then, so far successfully. If we don't allocate too much data on it, we might be OK, but still I'd go with minimum default of 256M.
What exactly do you have in mind? Using 256MB by default for any Btrfs filesystem or fallback to 256MB if mkfs.btrfs output cannot be parsed?
I meant default size.
Quoted 5 lines
I guess that for any other testcase it would be fine enough to bump the minimal device size to 256MB unconditionally, but in this case we create the data in RAM albeit compressed, and so I would like to keep it as small as possible, since otherwise it may fail on embedded hardware.
I didn't have a look at zram01, but can't we detect this and TCONF? We can try with minimum and see how frequently it changes, I just wanted to share Zach's quote and my experience with tiny btrfs volumes.
Regards,
Jan
Quoted 8 lines
Maybe we should just remove Btrfs from the
zram01.shtest so that we don't have to keep bumping the minimal size each time the minimal Btrfs size calculation changes...--
Cyril Hrubis
[email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
This is ~4 years old comment from Zach Brown, when I hit an issue on ppc, where I could alloc only 1/2 of the volume size:
"That small volume mkfs warning is issued for devices less than a gig. It indicates that btrfs has gone in to a weird degenerate allocation scheme. We'd only support volumes much larger than that, though I have no quick rule to say how large starts to be reasonable. Multiple gig, certainly."
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
I'm running with 384M since then, so far successfully. If we don't allocate too much data on it, we might be OK, but still I'd go with minimum default of 256M.
What exactly do you have in mind? Using 256MB by default for any Btrfs filesystem or fallback to 256MB if mkfs.btrfs output cannot be parsed?
I meant default size.
Then we should increase the default size for the LTP_DEV as well. Unfortunately it's defined at three different places at this point. We have it in runltp script, the C library and the shell library. I was thinking of exporting the C library as a binary for the shell tests to use in order to reduce the complexity. I will look into that and also bump the minimal size as well.
Quoted 6 lines
Quoted 4 lines
I guess that for any other testcase it would be fine enough to bump the minimal device size to 256MB unconditionally, but in this case we create the data in RAM albeit compressed, and so I would like to keep it as small as possible, since otherwise it may fail on embedded hardware.
I didn't have a look at zram01, but can't we detect this and TCONF?
I guess that we can look at total amount of RAM and enable/disable the test with Btrfs filesystem if it's deemed too small.
Quoted 2 lines
We can try with minimum and see how frequently it changes, I just wanted to share Zach's quote and my experience with tiny btrfs volumes.
I've been adjusting the minimal size (of the TST_DEVICE) two times already, we bumped it to 100MB then to 150MB. The first one was because mkfs.btrfs change, the second was because the size depends on PAGE_SIZE and 100MB wasn't enough for ppc64le. I guess that bumping it to 256MB is not unreasonable, and hopefully that would be enough for some time.
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 11 lines
Quoted 4 lines
Quoted 2 lines
What exactly do you have in mind? Using 256MB by default for any Btrfs filesystem or fallback to 256MB if mkfs.btrfs output cannot be parsed?
I meant default size.
Then we should increase the default size for the
LTP_DEVas well. Unfortunately it's defined at three different places at this point. We have it in runltp script, the C library and the shell library. I was thinking of exporting the C library as a binary for the shell tests to use in order to reduce the complexity. I will look into that and also bump the minimal size as well.
And apparenlty it was defined in four different places, since we have two versions of shell library each with it's own code to acquire device.
I've send a patchset that exports the acquire/release interface from C as a single binary and makes use of that in the tst_test.sh.
There is still one unsolved problem since the TWARN messages from the tst_device.c will trigger TBROK in tst_test.c since the IPC is not initialized. I'm still unsure how to fix that, maybe we should allow to allocate non-shared structure for the results in the special case that the library code is being reused in shell helpers.
Finally the last patch of the series bumps the minima size to 256MB and touched only two places, the runltp script and tst_device.c, which is way better than four...
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 5 lines
There is still one unsolved problem since the TWARN messages from the
tst_device.cwill trigger TBROK intst_test.csince the IPC is not initialized. I'm still unsure how to fix that, maybe we should allow to allocate non-shared structure for the results in the special case that the library code is being reused in shell helpers.
Maybe we should just add an API to redirect tst_brk() and tst_res() so that we could use library code in the various non-test utilities. Maybe we could patch things up so that we could use SAFE_MACROS() in cleanup as well...
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
----- Original Message -----
Quoted 7 lines
From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Thursday, 9 February, 2017 1:02:45 PM
Subject: Re: [LTP] [PATCH] [RFC] zram01: Fix on ppc64leHi!
Quoted 8 lines
Quoted 5 lines
There is still one unsolved problem since the TWARN messages from the
tst_device.cwill trigger TBROK intst_test.csince the IPC is not initialized. I'm still unsure how to fix that, maybe we should allow to allocate non-shared structure for the results in the special case that the library code is being reused in shell helpers.Maybe we should just add an API to redirect
tst_brk()andtst_res()so that we could use library code in the various non-test utilities.
Or drop that TBROK?
lib/tst_test.c +2 -4
@@ -146,10 +146,8 @@ void tst_reinit(void)static void update_results (const char *file,unsigned int lineno,int ttype) { -if (!results) { -tst_brk (TBROK , -"%s: %d: Results IPC not initialized!" , file, lineno); - } +if (!results) +return ;switch (ttype) {case TCONF :
There's no way "results" can be NULL when in test:
main
tst_run_tcases
do_setup
setup_ipc
SAFE_MMAP
Quoted 3 lines
Maybe
we could patch things up so that we could useSAFE_MACROS()in cleanup as well...
This should be doable with some flag we set in do_test_cleanup(), to skip further calls.
Regards,
Jan
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 10 lines
Quoted 8 lines
Quoted 5 lines
There is still one unsolved problem since the TWARN messages from the
tst_device.cwill trigger TBROK intst_test.csince the IPC is not initialized. I'm still unsure how to fix that, maybe we should allow to allocate non-shared structure for the results in the special case that the library code is being reused in shell helpers.Maybe we should just add an API to redirect
tst_brk()andtst_res()so that we could use library code in the various non-test utilities.Or drop that TBROK?
It's especially there for code that defines TST_NO_DEFAULT_MAIN but one can argue that this is special case and if you define that macro you should know what you are doing anyway...
But still I prefer to throw error message instead of segfaulting while trying to deference NULL pointer.
Quoted 6 lines
Quoted 3 lines
Maybe
we could patch things up so that we could useSAFE_MACROS()in cleanup as well...This should be doable with some flag we set in
do_test_cleanup(), to skip further calls.
I'm nearly finished with RFC patch. The main problem is the __attribute__((noreturn)) that has been added for various tst_brk* variants. So in the end it looks like only solution is to do the tst_brk_() redirection in the safe_macros.c, since we cannot return from tst_brkm_() since the return address is not stored on stack because it has noreturn attribute. And dropping the noreturn attribute from tst_brkm_() is not an option either, since that generates a ton of "control reaches end of non-void function" warnings.
But it's quite easy to define a macro or two that does the same redirection as we do in tst_brkm_() so that we never reach the oldlib code from safe_macros.c if newlib test is running.
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
----- Original Message -----
Quoted 16 lines
From: "Cyril Hrubis" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: [email protected]
Sent: Thursday, 9 February, 2017 3:00:42 PM
Subject: Re: [LTP] [PATCH] [RFC] zram01: Fix on ppc64leHi!
Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
There is still one unsolved problem since the TWARN messages from the
tst_device.cwill trigger TBROK intst_test.csince the IPC is not initialized. I'm still unsure how to fix that, maybe we should allow to allocate non-shared structure for the results in the special case that the library code is being reused in shell helpers.Maybe we should just add an API to redirect
tst_brk()andtst_res()so that we could use library code in the various non-test utilities.
Quoted 8 lines
Quoted 1 line
Or drop that TBROK?
It's especially there for code that defines
TST_NO_DEFAULT_MAINbut one can argue that this is special case and if you define that macro you should know what you are doing anyway...But still I prefer to throw error message instead of segfaulting while trying to deference NULL pointer.
I was suggesting we skip update_results(), not to let it segfault.
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
Maybe
we could patch things up so that we could useSAFE_MACROS()in cleanup as well...This should be doable with some flag we set in
do_test_cleanup(), to skip further calls.
Quoted 8 lines
I'm nearly finished with RFC patch. The main problem is the __attribute__((noreturn)) that has been added for various tst_brk* variants. So in the end it looks like only solution is to do the
tst_brk_()redirection in thesafe_macros.c, since we cannot return fromtst_brkm_()since the return address is not stored on stack because it has noreturn attribute. And dropping the noreturn attribute fromtst_brkm_()is not an option either, since that generates a ton of "control reaches end of non-void function" warnings.
Vast majority is from single header file:
$ grep "reaches end of non-void" log.txt | sort | uniq | wc -l 328
$ grep "reaches end of non-void" log.txt | sort | uniq | grep compat_16.h | wc -l 289
Quoted 9 lines
But it's quite easy to define a macro or two that does the same redirection as we do in
tst_brkm_()so that we never reach the oldlib code fromsafe_macros.cif newlib test is running.--
Cyril Hrubis
[email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 8 lines
Quoted 6 lines
It's especially there for code that defines
TST_NO_DEFAULT_MAINbut one can argue that this is special case and if you define that macro you should know what you are doing anyway...But still I prefer to throw error message instead of segfaulting while trying to deference NULL pointer.
I was suggesting we skip
update_results(), not to let it segfault.
Sorry I was blind. That makes much more sense. Let's go with that one, instead of the:
"[LTP] [PATCH 1/6] tst_test: Allow priting TINFO without initialized IPC"
Quoted 16 lines
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
Maybe
we could patch things up so that we could useSAFE_MACROS()in cleanup as well...This should be doable with some flag we set in
do_test_cleanup(), to skip further calls.Quoted 8 lines
I'm nearly finished with RFC patch. The main problem is the __attribute__((noreturn)) that has been added for various tst_brk* variants. So in the end it looks like only solution is to do the
tst_brk_()redirection in thesafe_macros.c, since we cannot return fromtst_brkm_()since the return address is not stored on stack because it has noreturn attribute. And dropping the noreturn attribute fromtst_brkm_()is not an option either, since that generates a ton of "control reaches end of non-void function" warnings.
Quoted 7 lines
Vast majority is from single header file:
$ grep "reaches end of non-void" log.txt | sort | uniq | wc -l 328$ grep "reaches end of non-void" log.txt | sort | uniq | grep compat_16.h | wc -l 289
That is still 39 occurences to fix all over the place plus the compat_16.h header. Have a look at the patch I've just send, I think that it's a bit easier than patching all the tests to avoids warnings...
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
In reply to Cyril HrubisHi!
Looks like we went off-topic and the zram01.sh test wasn't fixed in the end and it still fails on ppc64le.
I guess I can prepare a patch that checks if there is enough free RAM for a minimal Btrfs fs but I would still like to obtain the minimal size from mkfs.btrfs. Does that sound reasonable?
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
----- Original Message -----
Quoted 7 lines
Hi!
Looks like we went off-topic and thezram01.shtest wasn't fixed in the end and it still fails on ppc64le.I guess I can prepare a patch that checks if there is enough free RAM for a minimal Btrfs fs but I would still like to obtain the minimal size from mkfs.btrfs. Does that sound reasonable?
Do you mean these messages from mkfs.btrfs:
"ERROR: minimum size for each btrfs device is 41943040"
or some other way to find minimal supported size?
Regards,
Jan
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 10 lines
Quoted 6 lines
Looks like we went off-topic and the
zram01.shtest wasn't fixed in the end and it still fails on ppc64le.I guess I can prepare a patch that checks if there is enough free RAM for a minimal Btrfs fs but I would still like to obtain the minimal size from mkfs.btrfs. Does that sound reasonable?
Do you mean these messages from mkfs.btrfs:
"ERROR: minimum size for each btrfs device is 41943040"
or some other way to find minimal supported size?
Do do not know other way that the one used in the original patch, i.e. running mkfs.btrfs on /dev/null and parsing the output.
Or is there something better than that?
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 zram01: Fix on ppc64le
----- Original Message -----
Quoted 8 lines
Hi!
Quoted 7 lines
Quoted 6 lines
Looks like we went off-topic and the
zram01.shtest wasn't fixed in the end and it still fails on ppc64le.I guess I can prepare a patch that checks if there is enough free RAM for a minimal Btrfs fs but I would still like to obtain the minimal size from mkfs.btrfs. Does that sound reasonable?
Quoted 8 lines
Quoted 3 lines
Do you mean these messages from mkfs.btrfs:
"ERROR: minimum size for each btrfs device is 41943040"
or some other way to find minimal supported size?Do do not know other way that the one used in the original patch, i.e. running mkfs.btrfs on
/dev/nulland parsing the output.Or is there something better than that?
Other than picking a number by ourselves, nothing comes to mind. Numbers from mkfs.btrfs error message seem too low, even 2.5x larger number still fails:
# modprobe brd rd_nr=1 rd_size=100000 # mkfs.btrfs /dev/ram0 btrfs-progs v4.9.1 See http://btrfs.wiki.kernel.org for more information.
ERROR: not enough free space to allocate chunk
Regards,
Jan
RFC PATCH 1/1 zram01: Fix on ppc64le
Hi!
Quoted 10 lines
Other than picking a number by ourselves, nothing comes to mind. Numbers from mkfs.btrfs error message seem too low, even 2.5x larger number still fails:
# modprobe brd rd_nr=1 rd_size=100000 # mkfs.btrfs /dev/ram0 btrfs-progs v4.9.1 See http://btrfs.wiki.kernel.org for more information.ERROR: not enough free space to allocate chunk
Now that you mention it I remember some patches for mkfs.btrfs that fixed the minimal FS size computation. So I guess that hardcoding 384MB would be a bit safer.
Signature · 2 lines
Cyril Hrubis [email protected]