[v2] fsx: add more check for copy_file_range
diff mbox series

Message ID 1569549234-13096-1-git-send-email-suyj.fnst@cn.fujitsu.com
State New
Headers show
Series
  • [v2] fsx: add more check for copy_file_range
Related show

Commit Message

Su Yanjun Sept. 27, 2019, 1:53 a.m. UTC
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(+)

Comments

Eryu Guan Oct. 5, 2019, 7:07 p.m. UTC | #1
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
> 
> 
>

Patch
diff mbox series

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