* [PATCH v2] fsx: add more check for copy_file_range
@ 2019-09-27 1:53 Su Yanjun
2019-09-27 1:53 ` Su Yanjun
2019-10-05 19:07 ` Eryu Guan
0 siblings, 2 replies; 3+ messages in thread
From: Su Yanjun @ 2019-09-27 1:53 UTC (permalink / raw)
To: guaneryu; +Cc: fstests, suyj.fnst
On some linux distros(RHEL7, centos 7) copy_file_range uses
general implementation (splice interface). splice interace
uses pipe_to_file. pipe_to_file only work for different page.
The userspace cant's be aware of such error because copy_file_range
returns ok too.
So for such case when copy_file_range return we read back data
then check it.
Also add check for do_clone_range following Eryu's advice.
Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
---
v2:
- Use check_buffers instead memcmp code
- Add check for do_clone_range
- Fix return unique error number for the check code
---
ltp/fsx.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/ltp/fsx.c b/ltp/fsx.c
index 06d08e4..714bebc 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1378,6 +1378,7 @@ test_clone_range(void)
void
do_clone_range(unsigned offset, unsigned length, unsigned dest)
{
+ int ret = 0;
struct file_clone_range fcr = {
.src_fd = fd,
.src_offset = offset,
@@ -1429,6 +1430,23 @@ do_clone_range(unsigned offset, unsigned length, unsigned dest)
memset(good_buf + file_size, '\0', dest - file_size);
if (dest + length > file_size)
file_size = dest + length;
+
+ ret = lseek(fd, (off_t)dest, SEEK_SET);
+ if (ret == (off_t)-1) {
+ prterr("do_clone_range: lseek");
+ report_failure(162);
+ }
+ ret = fsxread(fd, temp_buf, length, dest);
+ if (ret != length) {
+ if (ret == -1)
+ prterr("do_clone_range: read");
+ else
+ prt("short read: 0x%x bytes instead of 0x%x\n",
+ ret, length);
+ report_failure(163);
+ }
+ check_buffers(temp_buf, dest, length);
+
}
#else
@@ -1602,6 +1620,7 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
size_t olen;
ssize_t nr;
int tries = 0;
+ int ret = 0;
if (length == 0) {
if (!quiet && testcalls > simulatedopcount)
@@ -1665,6 +1684,22 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
memset(good_buf + file_size, '\0', dest - file_size);
if (dest + length > file_size)
file_size = dest + length;
+
+ ret = lseek(fd, (off_t)dest, SEEK_SET);
+ if (ret == (off_t)-1) {
+ prterr("do_copy_range: lseek");
+ report_failure(162);
+ }
+ ret = fsxread(fd, temp_buf, length, dest);
+ if (ret != length) {
+ if (ret == -1)
+ prterr("do_copy_range: read");
+ else
+ prt("short read: 0x%x bytes instead of 0x%x\n",
+ ret, length);
+ report_failure(163);
+ }
+ check_buffers(temp_buf, dest, length);
}
#else
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v2] fsx: add more check for copy_file_range
2019-09-27 1:53 [PATCH v2] fsx: add more check for copy_file_range Su Yanjun
@ 2019-09-27 1:53 ` Su Yanjun
2019-10-05 19:07 ` Eryu Guan
1 sibling, 0 replies; 3+ messages in thread
From: Su Yanjun @ 2019-09-27 1:53 UTC (permalink / raw)
To: guaneryu; +Cc: fstests, suyj.fnst
On some linux distros(RHEL7, centos 7) copy_file_range uses
general implementation (splice interface). splice interace
uses pipe_to_file. pipe_to_file only work for different page.
The userspace cant's be aware of such error because copy_file_range
returns ok too.
So for such case when copy_file_range return we read back data
then check it.
Also add check for do_clone_range following Eryu's advice.
Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
---
v2:
- Use check_buffers instead memcmp code
- Add check for do_clone_range
- Fix return unique error number for the check code
---
ltp/fsx.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/ltp/fsx.c b/ltp/fsx.c
index 06d08e4..714bebc 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1378,6 +1378,7 @@ test_clone_range(void)
void
do_clone_range(unsigned offset, unsigned length, unsigned dest)
{
+ int ret = 0;
struct file_clone_range fcr = {
.src_fd = fd,
.src_offset = offset,
@@ -1429,6 +1430,23 @@ do_clone_range(unsigned offset, unsigned length, unsigned dest)
memset(good_buf + file_size, '\0', dest - file_size);
if (dest + length > file_size)
file_size = dest + length;
+
+ ret = lseek(fd, (off_t)dest, SEEK_SET);
+ if (ret == (off_t)-1) {
+ prterr("do_clone_range: lseek");
+ report_failure(162);
+ }
+ ret = fsxread(fd, temp_buf, length, dest);
+ if (ret != length) {
+ if (ret == -1)
+ prterr("do_clone_range: read");
+ else
+ prt("short read: 0x%x bytes instead of 0x%x\n",
+ ret, length);
+ report_failure(163);
+ }
+ check_buffers(temp_buf, dest, length);
+
}
#else
@@ -1602,6 +1620,7 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
size_t olen;
ssize_t nr;
int tries = 0;
+ int ret = 0;
if (length == 0) {
if (!quiet && testcalls > simulatedopcount)
@@ -1665,6 +1684,22 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
memset(good_buf + file_size, '\0', dest - file_size);
if (dest + length > file_size)
file_size = dest + length;
+
+ ret = lseek(fd, (off_t)dest, SEEK_SET);
+ if (ret == (off_t)-1) {
+ prterr("do_copy_range: lseek");
+ report_failure(162);
+ }
+ ret = fsxread(fd, temp_buf, length, dest);
+ if (ret != length) {
+ if (ret == -1)
+ prterr("do_copy_range: read");
+ else
+ prt("short read: 0x%x bytes instead of 0x%x\n",
+ ret, length);
+ report_failure(163);
+ }
+ check_buffers(temp_buf, dest, length);
}
#else
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] fsx: add more check for copy_file_range
2019-09-27 1:53 [PATCH v2] fsx: add more check for copy_file_range Su Yanjun
2019-09-27 1:53 ` Su Yanjun
@ 2019-10-05 19:07 ` Eryu Guan
1 sibling, 0 replies; 3+ messages in thread
From: Eryu Guan @ 2019-10-05 19:07 UTC (permalink / raw)
To: Su Yanjun; +Cc: fstests, Darrick J. Wong
On Fri, Sep 27, 2019 at 09:53:54AM +0800, Su Yanjun wrote:
> On some linux distros(RHEL7, centos 7) copy_file_range uses
> general implementation (splice interface). splice interace
> uses pipe_to_file. pipe_to_file only work for different page.
> The userspace cant's be aware of such error because copy_file_range
> returns ok too.
> So for such case when copy_file_range return we read back data
> then check it.
>
> Also add check for do_clone_range following Eryu's advice.
>
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
> ---
> v2:
> - Use check_buffers instead memcmp code
> - Add check for do_clone_range
> - Fix return unique error number for the check code
I just noticed that fsx already has an option (-X) to control check file
content or not after each test, which calls check_buffers().
Would you please check if adding '-X' option to fsx could reproduce the
copy_file_range bug on RHEL7? If so, perhaps we should add a new test
which is similar to generic/522 but does content check. Or just add '-X'
to generic/52[12]?
/me copies Darrick and hopes he could share his thoughts.
Thanks,
Eryu
> ---
> ltp/fsx.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 06d08e4..714bebc 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -1378,6 +1378,7 @@ test_clone_range(void)
> void
> do_clone_range(unsigned offset, unsigned length, unsigned dest)
> {
> + int ret = 0;
> struct file_clone_range fcr = {
> .src_fd = fd,
> .src_offset = offset,
> @@ -1429,6 +1430,23 @@ do_clone_range(unsigned offset, unsigned length, unsigned dest)
> memset(good_buf + file_size, '\0', dest - file_size);
> if (dest + length > file_size)
> file_size = dest + length;
> +
> + ret = lseek(fd, (off_t)dest, SEEK_SET);
> + if (ret == (off_t)-1) {
> + prterr("do_clone_range: lseek");
> + report_failure(162);
> + }
> + ret = fsxread(fd, temp_buf, length, dest);
> + if (ret != length) {
> + if (ret == -1)
> + prterr("do_clone_range: read");
> + else
> + prt("short read: 0x%x bytes instead of 0x%x\n",
> + ret, length);
> + report_failure(163);
> + }
> + check_buffers(temp_buf, dest, length);
> +
> }
>
> #else
> @@ -1602,6 +1620,7 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
> size_t olen;
> ssize_t nr;
> int tries = 0;
> + int ret = 0;
>
> if (length == 0) {
> if (!quiet && testcalls > simulatedopcount)
> @@ -1665,6 +1684,22 @@ do_copy_range(unsigned offset, unsigned length, unsigned dest)
> memset(good_buf + file_size, '\0', dest - file_size);
> if (dest + length > file_size)
> file_size = dest + length;
> +
> + ret = lseek(fd, (off_t)dest, SEEK_SET);
> + if (ret == (off_t)-1) {
> + prterr("do_copy_range: lseek");
> + report_failure(162);
> + }
> + ret = fsxread(fd, temp_buf, length, dest);
> + if (ret != length) {
> + if (ret == -1)
> + prterr("do_copy_range: read");
> + else
> + prt("short read: 0x%x bytes instead of 0x%x\n",
> + ret, length);
> + report_failure(163);
> + }
> + check_buffers(temp_buf, dest, length);
> }
>
> #else
> --
> 2.7.4
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-05 19:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-27 1:53 [PATCH v2] fsx: add more check for copy_file_range Su Yanjun
2019-09-27 1:53 ` Su Yanjun
2019-10-05 19:07 ` Eryu Guan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox