* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
@ 2019-08-15 8:36 Petr Vorel
2019-08-16 8:53 ` Murphy Zhou
2019-08-23 7:23 ` Li Wang
0 siblings, 2 replies; 9+ messages in thread
From: Petr Vorel @ 2019-08-15 8:36 UTC (permalink / raw)
To: ltp
Some libc implementations on arm (at least AArch32 target with hard
float (arm-linux-gnueabihf)) or some libc (musl, bionic) does not
export PAGE_SHIFT. Replace it with own inline function.
This required to replace MAX_LFS_FILESIZE constant with function
tst_max_lfs_filesize(). Given that we must use MAX_OFF in a function,
move dst from tcase struct to verify_copy_file_range().
Credits for 32 bit MAX_LFS_FILESIZE algorithm: Cyril Hrubis.
+ replace spaces with tabs in struct tcase.
Fixes: bc514b224 ("syscalls/copy_file_range02: increase coverage and remove EXDEV test")
Fixes: #555
Reported-by: alexchu-cpe
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
include/lapi/fs.h | 31 ++++++++++++----
.../copy_file_range/copy_file_range.h | 3 --
.../copy_file_range/copy_file_range02.c | 35 +++++++++++--------
3 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/include/lapi/fs.h b/include/lapi/fs.h
index 1af55628c..430d21f27 100644
--- a/include/lapi/fs.h
+++ b/include/lapi/fs.h
@@ -1,14 +1,18 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * Referred from linux kernel -github/torvalds/linux/include/uapi/linux/fs.h
+ * Referred from linux kernel include/uapi/linux/fs.h
+ * Copyright (c) 2019 Petr Vorel <pvorel@suse.cz>
* Copyright (c) Zilogic Systems Pvt. Ltd., 2018
* Email: code@zilogic.com
*/
+
#ifdef HAVE_LINUX_FS_H
# include <linux/fs.h>
#endif
-# include <sys/user.h>
-# include "lapi/abisize.h"
+
+#include <sys/user.h>
+#include <limits.h>
+#include "lapi/abisize.h"
#ifndef LAPI_FS_H
#define LAPI_FS_H
@@ -37,11 +41,26 @@
#define FS_NODUMP_FL 0x00000040 /* do not dump file */
#endif
-/* Referred form linux kernel include/linux/fs.h */
+/*
+ * Helper function to get MAX_LFS_FILESIZE.
+ * Missing PAGE_SHIFT on some libc prevents defining MAX_LFS_FILESIZE.
+ *
+ * 64 bit: macro taken from kernel from include/linux/fs.h
+ * 32 bit: own implementation
+ */
+static inline loff_t tst_max_lfs_filesize(void)
+{
#ifdef TST_ABI64
- #define MAX_LFS_FILESIZE ((loff_t)LLONG_MAX)
+ return (loff_t)LLONG_MAX;
#else
- #define MAX_LFS_FILESIZE ((loff_t)ULONG_MAX << PAGE_SHIFT)
+ long page_size = getpagesize();
+ loff_t ret = ULONG_MAX;
+
+ while (page_size >>= 1)
+ ret <<= 1;
+
+ return ret;
#endif
+}
#endif
diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range.h b/testcases/kernel/syscalls/copy_file_range/copy_file_range.h
index 24e08e390..40d05d653 100644
--- a/testcases/kernel/syscalls/copy_file_range/copy_file_range.h
+++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range.h
@@ -10,7 +10,6 @@
#include <stdbool.h>
#include <unistd.h>
#include <sys/sysmacros.h>
-#include <limits.h>
#include "lapi/syscalls.h"
#include "lapi/fs.h"
@@ -30,9 +29,7 @@
#define CONTENT "ABCDEFGHIJKLMNOPQRSTUVWXYZ12345\n"
#define CONTSIZE (sizeof(CONTENT) - 1)
-#define MAX_LEN MAX_LFS_FILESIZE
#define MIN_OFF 65537
-#define MAX_OFF (MAX_LEN - MIN_OFF)
static void syscall_info(void)
{
diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
index 9004c4a40..a08df9bdb 100644
--- a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
+++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
@@ -58,23 +58,22 @@ static struct tcase {
int *copy_to_fd;
int flags;
int exp_err;
- loff_t dst;
loff_t len;
const char *tname;
} tcases[] = {
- {&fd_rdonly, 0, EBADF, 0, CONTSIZE, "readonly file"},
- {&fd_dir, 0, EISDIR, 0, CONTSIZE, "directory"},
- {&fd_append, 0, EBADF, 0, CONTSIZE, "append to file"},
- {&fd_closed, 0, EBADF, 0, CONTSIZE, "closed file descriptor"},
- {&fd_dest, -1, EINVAL, 0, CONTSIZE, "invalid flags"},
- {&fd_immutable, 0, EPERM, 0, CONTSIZE, "immutable file"},
- {&fd_swapfile, 0, ETXTBSY, 0, CONTSIZE, "swap file"},
- {&fd_dup, 0, EINVAL, 0, CONTSIZE/2, "overlaping range"},
- {&fd_blkdev, 0, EINVAL, 0, CONTSIZE, "block device"},
- {&fd_chrdev, 0, EINVAL, 0, CONTSIZE, "char device"},
- {&fd_fifo, 0, EINVAL, 0, CONTSIZE, "fifo"},
- {&fd_copy, 0, EOVERFLOW, MAX_OFF, ULLONG_MAX, "max length lenght"},
- {&fd_copy, 0, EFBIG, MAX_OFF, MIN_OFF, "max file size"},
+ {&fd_rdonly, 0, EBADF, CONTSIZE, "readonly file"},
+ {&fd_dir, 0, EISDIR, CONTSIZE, "directory"},
+ {&fd_append, 0, EBADF, CONTSIZE, "append to file"},
+ {&fd_closed, 0, EBADF, CONTSIZE, "closed file descriptor"},
+ {&fd_dest, -1, EINVAL, CONTSIZE, "invalid flags"},
+ {&fd_immutable, 0, EPERM, CONTSIZE, "immutable file"},
+ {&fd_swapfile, 0, ETXTBSY, CONTSIZE, "swap file"},
+ {&fd_dup, 0, EINVAL, CONTSIZE/2, "overlaping range"},
+ {&fd_blkdev, 0, EINVAL, CONTSIZE, "block device"},
+ {&fd_chrdev, 0, EINVAL, CONTSIZE, "char device"},
+ {&fd_fifo, 0, EINVAL, CONTSIZE, "fifo"},
+ {&fd_copy, 0, EOVERFLOW, ULLONG_MAX, "max length lenght"},
+ {&fd_copy, 0, EFBIG, MIN_OFF, "max file size"},
};
static int run_command(char *command, char *option, char *file)
@@ -98,6 +97,8 @@ static int run_command(char *command, char *option, char *file)
static void verify_copy_file_range(unsigned int n)
{
struct tcase *tc = &tcases[n];
+ loff_t dst = 0;
+
tst_res(TINFO, "Test #%d: %s", n, tc->tname);
if (tc->copy_to_fd == &fd_immutable && chattr_i_nsup) {
@@ -112,8 +113,12 @@ static void verify_copy_file_range(unsigned int n)
tst_res(TCONF, "filesystem doesn't have free loopdev, skip it");
return;
}
+
+ if (tc->copy_to_fd == &fd_copy)
+ dst = tst_max_lfs_filesize() - MIN_OFF;
+
TEST(sys_copy_file_range(fd_src, 0, *tc->copy_to_fd,
- &tc->dst, tc->len, tc->flags));
+ &dst, tc->len, tc->flags));
if (TST_RET == -1) {
if (tc->exp_err == TST_ERR) {
--
2.22.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-15 8:36 [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation Petr Vorel
@ 2019-08-16 8:53 ` Murphy Zhou
2019-08-19 8:16 ` Li Wang
2019-08-23 7:23 ` Li Wang
1 sibling, 1 reply; 9+ messages in thread
From: Murphy Zhou @ 2019-08-16 8:53 UTC (permalink / raw)
To: ltp
On Thu, Aug 15, 2019 at 10:36:30AM +0200, Petr Vorel wrote:
> Some libc implementations on arm (at least AArch32 target with hard
> float (arm-linux-gnueabihf)) or some libc (musl, bionic) does not
> export PAGE_SHIFT. Replace it with own inline function.
>
> This required to replace MAX_LFS_FILESIZE constant with function
> tst_max_lfs_filesize(). Given that we must use MAX_OFF in a function,
> move dst from tcase struct to verify_copy_file_range().
>
> Credits for 32 bit MAX_LFS_FILESIZE algorithm: Cyril Hrubis.
I got the same results:
copy_file_range02.c:120: INFO: dst 9223372036854710270 len 65537
copy_file_range02.c:136: FAIL: copy_file_range returned wrong value: 32
THanks,
M
>
> + replace spaces with tabs in struct tcase.
>
> Fixes: bc514b224 ("syscalls/copy_file_range02: increase coverage and remove EXDEV test")
> Fixes: #555
>
> Reported-by: alexchu-cpe
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> include/lapi/fs.h | 31 ++++++++++++----
> .../copy_file_range/copy_file_range.h | 3 --
> .../copy_file_range/copy_file_range02.c | 35 +++++++++++--------
> 3 files changed, 45 insertions(+), 24 deletions(-)
>
> diff --git a/include/lapi/fs.h b/include/lapi/fs.h
> index 1af55628c..430d21f27 100644
> --- a/include/lapi/fs.h
> +++ b/include/lapi/fs.h
> @@ -1,14 +1,18 @@
> // SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - * Referred from linux kernel -github/torvalds/linux/include/uapi/linux/fs.h
> + * Referred from linux kernel include/uapi/linux/fs.h
> + * Copyright (c) 2019 Petr Vorel <pvorel@suse.cz>
> * Copyright (c) Zilogic Systems Pvt. Ltd., 2018
> * Email: code@zilogic.com
> */
> +
> #ifdef HAVE_LINUX_FS_H
> # include <linux/fs.h>
> #endif
> -# include <sys/user.h>
> -# include "lapi/abisize.h"
> +
> +#include <sys/user.h>
> +#include <limits.h>
> +#include "lapi/abisize.h"
>
> #ifndef LAPI_FS_H
> #define LAPI_FS_H
> @@ -37,11 +41,26 @@
> #define FS_NODUMP_FL 0x00000040 /* do not dump file */
> #endif
>
> -/* Referred form linux kernel include/linux/fs.h */
> +/*
> + * Helper function to get MAX_LFS_FILESIZE.
> + * Missing PAGE_SHIFT on some libc prevents defining MAX_LFS_FILESIZE.
> + *
> + * 64 bit: macro taken from kernel from include/linux/fs.h
> + * 32 bit: own implementation
> + */
> +static inline loff_t tst_max_lfs_filesize(void)
> +{
> #ifdef TST_ABI64
> - #define MAX_LFS_FILESIZE ((loff_t)LLONG_MAX)
> + return (loff_t)LLONG_MAX;
> #else
> - #define MAX_LFS_FILESIZE ((loff_t)ULONG_MAX << PAGE_SHIFT)
> + long page_size = getpagesize();
> + loff_t ret = ULONG_MAX;
> +
> + while (page_size >>= 1)
> + ret <<= 1;
> +
> + return ret;
> #endif
> +}
>
> #endif
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range.h b/testcases/kernel/syscalls/copy_file_range/copy_file_range.h
> index 24e08e390..40d05d653 100644
> --- a/testcases/kernel/syscalls/copy_file_range/copy_file_range.h
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range.h
> @@ -10,7 +10,6 @@
> #include <stdbool.h>
> #include <unistd.h>
> #include <sys/sysmacros.h>
> -#include <limits.h>
> #include "lapi/syscalls.h"
> #include "lapi/fs.h"
>
> @@ -30,9 +29,7 @@
>
> #define CONTENT "ABCDEFGHIJKLMNOPQRSTUVWXYZ12345\n"
> #define CONTSIZE (sizeof(CONTENT) - 1)
> -#define MAX_LEN MAX_LFS_FILESIZE
> #define MIN_OFF 65537
> -#define MAX_OFF (MAX_LEN - MIN_OFF)
>
> static void syscall_info(void)
> {
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
> index 9004c4a40..a08df9bdb 100644
> --- a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
> @@ -58,23 +58,22 @@ static struct tcase {
> int *copy_to_fd;
> int flags;
> int exp_err;
> - loff_t dst;
> loff_t len;
> const char *tname;
> } tcases[] = {
> - {&fd_rdonly, 0, EBADF, 0, CONTSIZE, "readonly file"},
> - {&fd_dir, 0, EISDIR, 0, CONTSIZE, "directory"},
> - {&fd_append, 0, EBADF, 0, CONTSIZE, "append to file"},
> - {&fd_closed, 0, EBADF, 0, CONTSIZE, "closed file descriptor"},
> - {&fd_dest, -1, EINVAL, 0, CONTSIZE, "invalid flags"},
> - {&fd_immutable, 0, EPERM, 0, CONTSIZE, "immutable file"},
> - {&fd_swapfile, 0, ETXTBSY, 0, CONTSIZE, "swap file"},
> - {&fd_dup, 0, EINVAL, 0, CONTSIZE/2, "overlaping range"},
> - {&fd_blkdev, 0, EINVAL, 0, CONTSIZE, "block device"},
> - {&fd_chrdev, 0, EINVAL, 0, CONTSIZE, "char device"},
> - {&fd_fifo, 0, EINVAL, 0, CONTSIZE, "fifo"},
> - {&fd_copy, 0, EOVERFLOW, MAX_OFF, ULLONG_MAX, "max length lenght"},
> - {&fd_copy, 0, EFBIG, MAX_OFF, MIN_OFF, "max file size"},
> + {&fd_rdonly, 0, EBADF, CONTSIZE, "readonly file"},
> + {&fd_dir, 0, EISDIR, CONTSIZE, "directory"},
> + {&fd_append, 0, EBADF, CONTSIZE, "append to file"},
> + {&fd_closed, 0, EBADF, CONTSIZE, "closed file descriptor"},
> + {&fd_dest, -1, EINVAL, CONTSIZE, "invalid flags"},
> + {&fd_immutable, 0, EPERM, CONTSIZE, "immutable file"},
> + {&fd_swapfile, 0, ETXTBSY, CONTSIZE, "swap file"},
> + {&fd_dup, 0, EINVAL, CONTSIZE/2, "overlaping range"},
> + {&fd_blkdev, 0, EINVAL, CONTSIZE, "block device"},
> + {&fd_chrdev, 0, EINVAL, CONTSIZE, "char device"},
> + {&fd_fifo, 0, EINVAL, CONTSIZE, "fifo"},
> + {&fd_copy, 0, EOVERFLOW, ULLONG_MAX, "max length lenght"},
> + {&fd_copy, 0, EFBIG, MIN_OFF, "max file size"},
> };
>
> static int run_command(char *command, char *option, char *file)
> @@ -98,6 +97,8 @@ static int run_command(char *command, char *option, char *file)
> static void verify_copy_file_range(unsigned int n)
> {
> struct tcase *tc = &tcases[n];
> + loff_t dst = 0;
> +
> tst_res(TINFO, "Test #%d: %s", n, tc->tname);
>
> if (tc->copy_to_fd == &fd_immutable && chattr_i_nsup) {
> @@ -112,8 +113,12 @@ static void verify_copy_file_range(unsigned int n)
> tst_res(TCONF, "filesystem doesn't have free loopdev, skip it");
> return;
> }
> +
> + if (tc->copy_to_fd == &fd_copy)
> + dst = tst_max_lfs_filesize() - MIN_OFF;
> +
> TEST(sys_copy_file_range(fd_src, 0, *tc->copy_to_fd,
> - &tc->dst, tc->len, tc->flags));
> + &dst, tc->len, tc->flags));
>
> if (TST_RET == -1) {
> if (tc->exp_err == TST_ERR) {
> --
> 2.22.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-16 8:53 ` Murphy Zhou
@ 2019-08-19 8:16 ` Li Wang
2019-08-20 9:09 ` Yang Xu
0 siblings, 1 reply; 9+ messages in thread
From: Li Wang @ 2019-08-19 8:16 UTC (permalink / raw)
To: ltp
On Fri, Aug 16, 2019 at 4:53 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> On Thu, Aug 15, 2019 at 10:36:30AM +0200, Petr Vorel wrote:
> > Some libc implementations on arm (at least AArch32 target with hard
> > float (arm-linux-gnueabihf)) or some libc (musl, bionic) does not
> > export PAGE_SHIFT. Replace it with own inline function.
> >
> > This required to replace MAX_LFS_FILESIZE constant with function
> > tst_max_lfs_filesize(). Given that we must use MAX_OFF in a function,
> > move dst from tcase struct to verify_copy_file_range().
> >
> > Credits for 32 bit MAX_LFS_FILESIZE algorithm: Cyril Hrubis.
>
> I got the same results:
>
> copy_file_range02.c:120: INFO: dst 9223372036854710270 len 65537
> copy_file_range02.c:136: FAIL: copy_file_range returned wrong value: 32
I'm not chanllenge the tst_max_lfs_filesize().
But I don't understand why to define MAX_OFF as (MAX_LEN - MIN_OFF),
the failure indicates that not to write at a position past the maximum
allowed offset. Shouldn't we give a dst_off large than
MAX_LFS_FILESIZE?
if I change the code as below, then it could be passed.
--- a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
+++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
@@ -115,7 +115,7 @@ static void verify_copy_file_range(unsigned int n)
}
if (tc->copy_to_fd == &fd_copy)
- dst = tst_max_lfs_filesize() - MIN_OFF;
+ dst = tst_max_lfs_filesize();
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-19 8:16 ` Li Wang
@ 2019-08-20 9:09 ` Yang Xu
2019-08-21 7:27 ` Li Wang
0 siblings, 1 reply; 9+ messages in thread
From: Yang Xu @ 2019-08-20 9:09 UTC (permalink / raw)
To: ltp
on 2019/08/19 16:16, Li Wang wrote:
> On Fri, Aug 16, 2019 at 4:53 PM Murphy Zhou<jencce.kernel@gmail.com> wrote:
>> On Thu, Aug 15, 2019 at 10:36:30AM +0200, Petr Vorel wrote:
>>> Some libc implementations on arm (at least AArch32 target with hard
>>> float (arm-linux-gnueabihf)) or some libc (musl, bionic) does not
>>> export PAGE_SHIFT. Replace it with own inline function.
>>>
>>> This required to replace MAX_LFS_FILESIZE constant with function
>>> tst_max_lfs_filesize(). Given that we must use MAX_OFF in a function,
>>> move dst from tcase struct to verify_copy_file_range().
>>>
>>> Credits for 32 bit MAX_LFS_FILESIZE algorithm: Cyril Hrubis.
>> I got the same results:
>>
>> copy_file_range02.c:120: INFO: dst 9223372036854710270 len 65537
>> copy_file_range02.c:136: FAIL: copy_file_range returned wrong value: 32
> I'm not chanllenge the tst_max_lfs_filesize().
>
> But I don't understand why to define MAX_OFF as (MAX_LEN - MIN_OFF),
> the failure indicates that not to write at a position past the maximum
> allowed offset. Shouldn't we give a dst_off large than
> MAX_LFS_FILESIZE?
Yes, we should use a dst_off large than MAX_LFS_FILESIZE because it used pos to compare
in kernel code as below:
mm/filemap.c
static int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
...
if (unlikely(pos>= max_size))
return -EFBIG;
...
I strace xfstest generic/564 code( I follow this test code to ltp), as below:
#max_off=$((8 * 2**60 - 65536 - 1))
#min_off=65537
#xfs_io -f -c "pwrite -S 0x61 0 128k" file
#touch copy
#strace xfs_io -c "copy_range -l $min_off -s 0 -d $max_off file" copy
....
openat(AT_FDCWD, "file", O_RDONLY) = 4
copy_file_range(4, [0], 3, [9223372036854710271], 65537, 0) = 65536
copy_file_range(4, [65536], 3, [9223372036854775807], 1, 0) = -1 EFBIG (File too large)
....
xfsprogs used a loop to call copy_file_range, and get EFBIG when pos greater than LLONG_MAX.
I think we should use tst_max_lfs_filesize instead of (tst_max_lfs_filesize -MIN_OFF)
and this case will pass whether xfs,btrfs and ext4.
Thanks for pointing out this.
> if I change the code as below, then it could be passed.
>
> --- a/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range02.c
> @@ -115,7 +115,7 @@ static void verify_copy_file_range(unsigned int n)
> }
>
> if (tc->copy_to_fd ==&fd_copy)
> - dst = tst_max_lfs_filesize() - MIN_OFF;
> + dst = tst_max_lfs_filesize();
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-20 9:09 ` Yang Xu
@ 2019-08-21 7:27 ` Li Wang
2019-08-21 8:14 ` Yang Xu
0 siblings, 1 reply; 9+ messages in thread
From: Li Wang @ 2019-08-21 7:27 UTC (permalink / raw)
To: ltp
On Tue, Aug 20, 2019 at 5:10 PM Yang Xu <xuyang2018.jy@cn.fujitsu.com> wrote:
...
> >
> > But I don't understand why to define MAX_OFF as (MAX_LEN - MIN_OFF),
> > the failure indicates that not to write at a position past the maximum
> > allowed offset. Shouldn't we give a dst_off large than
> > MAX_LFS_FILESIZE?
> Yes, we should use a dst_off large than MAX_LFS_FILESIZE because it used pos to compare
> in kernel code as below:
>
> mm/filemap.c
> static int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
> ...
> if (unlikely(pos>= max_size))
> return -EFBIG;
> ...
>
> I strace xfstest generic/564 code( I follow this test code to ltp), as below:
> #max_off=$((8 * 2**60 - 65536 - 1))
> #min_off=65537
> #xfs_io -f -c "pwrite -S 0x61 0 128k" file
> #touch copy
> #strace xfs_io -c "copy_range -l $min_off -s 0 -d $max_off file" copy
> ....
> openat(AT_FDCWD, "file", O_RDONLY) = 4
> copy_file_range(4, [0], 3, [9223372036854710271], 65537, 0) = 65536
> copy_file_range(4, [65536], 3, [9223372036854775807], 1, 0) = -1 EFBIG (File too large)
> ....
>
> xfsprogs used a loop to call copy_file_range, and get EFBIG when pos greater than LLONG_MAX.
>
> I think we should use tst_max_lfs_filesize instead of (tst_max_lfs_filesize -MIN_OFF)
> and this case will pass whether xfs,btrfs and ext4.
Good job, Xu. I think you can format a new patch to fix this problem.
Because Petr's patch is used for solving the cross-compiling issue and
looks good.
@Petr Vorel Hi Petr, what do you think? any more comments?
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-21 7:27 ` Li Wang
@ 2019-08-21 8:14 ` Yang Xu
2019-08-23 3:10 ` Murphy Zhou
0 siblings, 1 reply; 9+ messages in thread
From: Yang Xu @ 2019-08-21 8:14 UTC (permalink / raw)
To: ltp
on 2019/08/21 15:27, Li Wang wrote:
> On Tue, Aug 20, 2019 at 5:10 PM Yang Xu<xuyang2018.jy@cn.fujitsu.com> wrote:
> ...
>>> But I don't understand why to define MAX_OFF as (MAX_LEN - MIN_OFF),
>>> the failure indicates that not to write at a position past the maximum
>>> allowed offset. Shouldn't we give a dst_off large than
>>> MAX_LFS_FILESIZE?
>> Yes, we should use a dst_off large than MAX_LFS_FILESIZE because it used pos to compare
>> in kernel code as below:
>>
>> mm/filemap.c
>> static int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
>> ...
>> if (unlikely(pos>= max_size))
>> return -EFBIG;
>> ...
>>
>> I strace xfstest generic/564 code( I follow this test code to ltp), as below:
>> #max_off=$((8 * 2**60 - 65536 - 1))
>> #min_off=65537
>> #xfs_io -f -c "pwrite -S 0x61 0 128k" file
>> #touch copy
>> #strace xfs_io -c "copy_range -l $min_off -s 0 -d $max_off file" copy
>> ....
>> openat(AT_FDCWD, "file", O_RDONLY) = 4
>> copy_file_range(4, [0], 3, [9223372036854710271], 65537, 0) = 65536
>> copy_file_range(4, [65536], 3, [9223372036854775807], 1, 0) = -1 EFBIG (File too large)
>> ....
>>
>> xfsprogs used a loop to call copy_file_range, and get EFBIG when pos greater than LLONG_MAX.
>>
>> I think we should use tst_max_lfs_filesize instead of (tst_max_lfs_filesize -MIN_OFF)
>> and this case will pass whether xfs,btrfs and ext4.
> Good job, Xu. I think you can format a new patch to fix this problem.
> Because Petr's patch is used for solving the cross-compiling issue and
> looks good.
Hi Li
OK. I will send a new patch to fix this problem. But copy_file_range02.c still has a problem as Murphy found,
we set all_filesystem but use the same tmpdir instead of mntpoint. I think we can remove all_filesystem and mountpoint.
@Murphy Zhou Hi Murphy, would you like to send a new patch to remove useless all_filesystem or I do it in my
new patch by adding your signed-off-by tag? What do you think about it?
> @Petr Vorel Hi Petr, what do you think? any more comments?
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-21 8:14 ` Yang Xu
@ 2019-08-23 3:10 ` Murphy Zhou
0 siblings, 0 replies; 9+ messages in thread
From: Murphy Zhou @ 2019-08-23 3:10 UTC (permalink / raw)
To: ltp
On Wed, Aug 21, 2019 at 04:14:45PM +0800, Yang Xu wrote:
> on 2019/08/21 15:27, Li Wang wrote:
>
> > On Tue, Aug 20, 2019 at 5:10 PM Yang Xu<xuyang2018.jy@cn.fujitsu.com> wrote:
> > ...
> > > > But I don't understand why to define MAX_OFF as (MAX_LEN - MIN_OFF),
> > > > the failure indicates that not to write at a position past the maximum
> > > > allowed offset. Shouldn't we give a dst_off large than
> > > > MAX_LFS_FILESIZE?
> > > Yes, we should use a dst_off large than MAX_LFS_FILESIZE because it used pos to compare
> > > in kernel code as below:
> > >
> > > mm/filemap.c
> > > static int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
> > > ...
> > > if (unlikely(pos>= max_size))
> > > return -EFBIG;
> > > ...
> > >
> > > I strace xfstest generic/564 code( I follow this test code to ltp), as below:
> > > #max_off=$((8 * 2**60 - 65536 - 1))
> > > #min_off=65537
> > > #xfs_io -f -c "pwrite -S 0x61 0 128k" file
> > > #touch copy
> > > #strace xfs_io -c "copy_range -l $min_off -s 0 -d $max_off file" copy
> > > ....
> > > openat(AT_FDCWD, "file", O_RDONLY) = 4
> > > copy_file_range(4, [0], 3, [9223372036854710271], 65537, 0) = 65536
> > > copy_file_range(4, [65536], 3, [9223372036854775807], 1, 0) = -1 EFBIG (File too large)
> > > ....
> > >
> > > xfsprogs used a loop to call copy_file_range, and get EFBIG when pos greater than LLONG_MAX.
> > >
> > > I think we should use tst_max_lfs_filesize instead of (tst_max_lfs_filesize -MIN_OFF)
> > > and this case will pass whether xfs,btrfs and ext4.
> > Good job, Xu. I think you can format a new patch to fix this problem.
> > Because Petr's patch is used for solving the cross-compiling issue and
> > looks good.
> Hi Li
>
> OK. I will send a new patch to fix this problem. But copy_file_range02.c still has a problem as Murphy found,
> we set all_filesystem but use the same tmpdir instead of mntpoint. I think we can remove all_filesystem and mountpoint.
>
> @Murphy Zhou Hi Murphy, would you like to send a new patch to remove useless all_filesystem or I do it in my
> new patch by adding your signed-off-by tag? What do you think about it?
You can go ahead and do it. Thank you!
M
>
> > @Petr Vorel Hi Petr, what do you think? any more comments?
> >
>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-15 8:36 [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation Petr Vorel
2019-08-16 8:53 ` Murphy Zhou
@ 2019-08-23 7:23 ` Li Wang
2019-08-26 8:50 ` Petr Vorel
1 sibling, 1 reply; 9+ messages in thread
From: Li Wang @ 2019-08-23 7:23 UTC (permalink / raw)
To: ltp
Pushed. Thanks for the fix.
--
Regards,
Li Wang
^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation
2019-08-23 7:23 ` Li Wang
@ 2019-08-26 8:50 ` Petr Vorel
0 siblings, 0 replies; 9+ messages in thread
From: Petr Vorel @ 2019-08-26 8:50 UTC (permalink / raw)
To: ltp
Hi Li,
> Pushed. Thanks for the fix.
Thanks for pushing it and sorry for delay in a reply
(vacation without my laptop).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-26 8:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15 8:36 [LTP] [PATCH] lapi/fs.h: Replace MAX_LFS_FILESIZE constant with own implementation Petr Vorel
2019-08-16 8:53 ` Murphy Zhou
2019-08-19 8:16 ` Li Wang
2019-08-20 9:09 ` Yang Xu
2019-08-21 7:27 ` Li Wang
2019-08-21 8:14 ` Yang Xu
2019-08-23 3:10 ` Murphy Zhou
2019-08-23 7:23 ` Li Wang
2019-08-26 8:50 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.