| Message ID | 20240222044119.28500-1-wegao@suse.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1] swapoff01.c: Adjust blocks size base on pagesize | expand |
On Thu, Feb 22, 2024 at 12:41 PM Wei Gao via ltp <ltp@lists.linux.it> wrote: > The make_swapfile function will encounter no space error if pagesize > is bigger then 4096, such as ppc64 system use default pagesize 65535. > What erros does it shows in the log? I can't reproduce this on my aarch64 (pagesize == 65536). Seems the '.dev_min_size = 350' is large enough for testing. > > Signed-off-by: Wei Gao <wegao@suse.com> > --- > testcases/kernel/syscalls/swapoff/swapoff01.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/swapoff/swapoff01.c > b/testcases/kernel/syscalls/swapoff/swapoff01.c > index c303588df..71d6c6c04 100644 > --- a/testcases/kernel/syscalls/swapoff/swapoff01.c > +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c > @@ -44,7 +44,12 @@ static void setup(void) > { > is_swap_supported(TEST_FILE); > > - if (make_swapfile(SWAP_FILE, 65536, 1)) > + int blocks = 65535; > + > + if (getpagesize() > 4096) > + blocks = 65535 * 4096 / getpagesize(); > + > + if (make_swapfile(SWAP_FILE, blocks, 1)) > tst_brk(TBROK, "Failed to create file for swap"); > } > > -- > 2.35.3 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp > >
Hi! > The make_swapfile function will encounter no space error if pagesize > is bigger then 4096, such as ppc64 system use default pagesize 65535. ^ 65536? Isn't this more about Btrfs though? Looking at the make_swapfile() we do use statvfs to get filesystem block size and if that is Btrfs with 64k blocks we end up with swapfile of a size of 4GB that sounds like a bit too much I guess. > Signed-off-by: Wei Gao <wegao@suse.com> > --- > testcases/kernel/syscalls/swapoff/swapoff01.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/testcases/kernel/syscalls/swapoff/swapoff01.c b/testcases/kernel/syscalls/swapoff/swapoff01.c > index c303588df..71d6c6c04 100644 > --- a/testcases/kernel/syscalls/swapoff/swapoff01.c > +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c > @@ -44,7 +44,12 @@ static void setup(void) > { > is_swap_supported(TEST_FILE); > > - if (make_swapfile(SWAP_FILE, 65536, 1)) > + int blocks = 65535; > + > + if (getpagesize() > 4096) > + blocks = 65535 * 4096 / getpagesize(); > + > + if (make_swapfile(SWAP_FILE, blocks, 1)) > tst_brk(TBROK, "Failed to create file for swap"); I do not think that this is a right solution though. Is there any reason why we pass number of blocks to the make_swapfile instead of megabytes? > } > > -- > 2.35.3 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
On Thu, Feb 22, 2024 at 09:39:43AM +0100, Cyril Hrubis wrote: > Hi! > > The make_swapfile function will encounter no space error if pagesize > > is bigger then 4096, such as ppc64 system use default pagesize 65535. > ^ > 65536? Yes > > Isn't this more about Btrfs though? Looking at the make_swapfile() we do > use statvfs to get filesystem block size and if that is Btrfs with 64k > blocks we end up with swapfile of a size of 4GB that sounds like a bit > too much I guess. > Sorry for confusing. It should caused by block size instead of page size. The code change should change like following, but i suppose you will not agree on this solution, correct? --- a/testcases/kernel/syscalls/swapoff/swapoff01.c +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c @@ -12,6 +12,7 @@ #include <unistd.h> #include <errno.h> #include <stdlib.h> +#include <sys/statvfs.h> #include "tst_test.h" #include "lapi/syscalls.h" @@ -44,8 +45,21 @@ static void setup(void) { is_swap_supported(TEST_FILE); - if (make_swapfile(SWAP_FILE, 65536, 1)) + struct statvfs fs_info; + unsigned long blk_size; + if (statvfs(".", &fs_info) == -1) + tst_brk(TBROK, "Failed to get statvfs info"); + + blk_size = fs_info.f_bsize; + + int blocks = 65535; + + if (blk_size > 4096) + blocks = 65535 * 4096 / blk_size; + + if (make_swapfile(SWAP_FILE, blocks, 1)) tst_brk(TBROK, "Failed to create file for swap"); + } > > Signed-off-by: Wei Gao <wegao@suse.com> > > --- > > testcases/kernel/syscalls/swapoff/swapoff01.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/testcases/kernel/syscalls/swapoff/swapoff01.c b/testcases/kernel/syscalls/swapoff/swapoff01.c > > index c303588df..71d6c6c04 100644 > > --- a/testcases/kernel/syscalls/swapoff/swapoff01.c > > +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c > > @@ -44,7 +44,12 @@ static void setup(void) > > { > > is_swap_supported(TEST_FILE); > > > > - if (make_swapfile(SWAP_FILE, 65536, 1)) > > + int blocks = 65535; > > + > > + if (getpagesize() > 4096) > > + blocks = 65535 * 4096 / getpagesize(); > > + > > + if (make_swapfile(SWAP_FILE, blocks, 1)) > > tst_brk(TBROK, "Failed to create file for swap"); > > > I do not think that this is a right solution though. Is there any reason > why we pass number of blocks to the make_swapfile instead of megabytes? > @Li Wang, could you give some clue for above question(for why pass number of blocks instead of megabytes)? Thanks all for comments! > > } > > > > -- > > 2.35.3 > > > > > > -- > > Mailing list info: https://lists.linux.it/listinfo/ltp > > -- > Cyril Hrubis > chrubis@suse.cz
On Thu, Feb 22, 2024 at 03:57:57PM +0800, Li Wang wrote: > On Thu, Feb 22, 2024 at 12:41 PM Wei Gao via ltp <ltp@lists.linux.it> wrote: > > > The make_swapfile function will encounter no space error if pagesize > > is bigger then 4096, such as ppc64 system use default pagesize 65535. > > > > What erros does it shows in the log? > > I can't reproduce this on my aarch64 (pagesize == 65536). > Seems the '.dev_min_size = 350' is large enough for testing. > Sorry , it should be block size == 65536. > > > > > > Signed-off-by: Wei Gao <wegao@suse.com> > > --- > > testcases/kernel/syscalls/swapoff/swapoff01.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/testcases/kernel/syscalls/swapoff/swapoff01.c > > b/testcases/kernel/syscalls/swapoff/swapoff01.c > > index c303588df..71d6c6c04 100644 > > --- a/testcases/kernel/syscalls/swapoff/swapoff01.c > > +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c > > @@ -44,7 +44,12 @@ static void setup(void) > > { > > is_swap_supported(TEST_FILE); > > > > - if (make_swapfile(SWAP_FILE, 65536, 1)) > > + int blocks = 65535; > > + > > + if (getpagesize() > 4096) > > + blocks = 65535 * 4096 / getpagesize(); > > + > > + if (make_swapfile(SWAP_FILE, blocks, 1)) > > tst_brk(TBROK, "Failed to create file for swap"); > > } > > > > -- > > 2.35.3 > > > > > > -- > > Mailing list info: https://lists.linux.it/listinfo/ltp > > > > > > -- > Regards, > Li Wang
Hi! > > Isn't this more about Btrfs though? Looking at the make_swapfile() we do > > use statvfs to get filesystem block size and if that is Btrfs with 64k > > blocks we end up with swapfile of a size of 4GB that sounds like a bit > > too much I guess. > > > > Sorry for confusing. It should caused by block size instead of page size. > The code change should change like following, but i suppose you will not agree > on this solution, correct? > > --- a/testcases/kernel/syscalls/swapoff/swapoff01.c > +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c > @@ -12,6 +12,7 @@ > #include <unistd.h> > #include <errno.h> > #include <stdlib.h> > +#include <sys/statvfs.h> > > #include "tst_test.h" > #include "lapi/syscalls.h" > @@ -44,8 +45,21 @@ static void setup(void) > { > is_swap_supported(TEST_FILE); > > - if (make_swapfile(SWAP_FILE, 65536, 1)) > + struct statvfs fs_info; > + unsigned long blk_size; > + if (statvfs(".", &fs_info) == -1) > + tst_brk(TBROK, "Failed to get statvfs info"); > + > + blk_size = fs_info.f_bsize; > + > + int blocks = 65535; > + > + if (blk_size > 4096) > + blocks = 65535 * 4096 / blk_size; > + > + if (make_swapfile(SWAP_FILE, blocks, 1)) > tst_brk(TBROK, "Failed to create file for swap"); > + > } It may be easier to change the make_swapfile() function to accept megabytes instead of working around it like this.
Hi Wei, Cyril, > > I do not think that this is a right solution though. Is there any reason > > why we pass number of blocks to the make_swapfile instead of megabytes? > > > @Li Wang, could you give some clue for above question(for why pass number > of blocks instead of megabytes)? > I just keep the function interface like the original (it wasn't designed by me:), but I guess the only advantage is to test 1 block for different FS types.
Hi all, > Hi Wei, Cyril, > > > I do not think that this is a right solution though. Is there any reason > > > why we pass number of blocks to the make_swapfile instead of megabytes? > > @Li Wang, could you give some clue for above question(for why pass number > > of blocks instead of megabytes)? > I just keep the function interface like the original > (it wasn't designed by me:), but I guess the only > advantage is to test 1 block for different FS types. We use this function with 1 block, 10 blocks and 65536 blocks How about to have special function for 1 block and then other function which accepts MB? (e.g. 1 MB and 256 MB)? But could we first merge Yang Xu patchset so that he does not have to rebase it for ever? https://patchwork.ozlabs.org/project/ltp/list/?series=395713&state=* Also it'd be nice to have, but this can definitely wait after other things are solved. -int make_swapfile(const char *swapfile, int blocks, int safe) +int _make_swapfile(const char *swapfile, int blocks, int safe) +#define MAKE_SWAPFILE(const char *swapfile, int blocks, int safe) \ + _make_swapfile(swapfile, blocks, 0) +#define SAFE_SWAPFILE(const char *swapfile, int blocks, int safe) \ + _make_swapfile(swapfile, blocks, 1) Kind regards, Petr
diff --git a/testcases/kernel/syscalls/swapoff/swapoff01.c b/testcases/kernel/syscalls/swapoff/swapoff01.c index c303588df..71d6c6c04 100644 --- a/testcases/kernel/syscalls/swapoff/swapoff01.c +++ b/testcases/kernel/syscalls/swapoff/swapoff01.c @@ -44,7 +44,12 @@ static void setup(void) { is_swap_supported(TEST_FILE); - if (make_swapfile(SWAP_FILE, 65536, 1)) + int blocks = 65535; + + if (getpagesize() > 4096) + blocks = 65535 * 4096 / getpagesize(); + + if (make_swapfile(SWAP_FILE, blocks, 1)) tst_brk(TBROK, "Failed to create file for swap"); }
The make_swapfile function will encounter no space error if pagesize is bigger then 4096, such as ppc64 system use default pagesize 65535. Signed-off-by: Wei Gao <wegao@suse.com> --- testcases/kernel/syscalls/swapoff/swapoff01.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)