diff mbox series

[3/4] fsx: fix infinite/too long loops when generating ranges for copy_file_range

Message ID 20200408103616.11458-1-fdmanana@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/4] fsx: add missing file size update on zero range operations | expand

Commit Message

Filipe Manana April 8, 2020, 10:36 a.m. UTC
From: Filipe Manana <fdmanana@suse.com>

While running generic/521 I've had fsx taking a lot of CPU time and not
making any progress for several hours. Attaching gdb to the fsx process
revealed that fsx was in the loop that generates the ranges for a
copy_file_range operation, in particular the loop seemed to never end
because the range defined by 'offset2' kept overlapping with the range
defined by 'offset'.
So far this happened one time only in one of my test VMs with generic/521.

Fix this by breaking out of the loop after trying 30 times, like we
currently do for dedupe operations, which results in logging the operation
as skipped.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 ltp/fsx.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

Comments

Brian Foster April 17, 2020, 5:10 p.m. UTC | #1
On Wed, Apr 08, 2020 at 11:36:16AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> While running generic/521 I've had fsx taking a lot of CPU time and not
> making any progress for several hours. Attaching gdb to the fsx process
> revealed that fsx was in the loop that generates the ranges for a
> copy_file_range operation, in particular the loop seemed to never end
> because the range defined by 'offset2' kept overlapping with the range
> defined by 'offset'.
> So far this happened one time only in one of my test VMs with generic/521.
> 
> Fix this by breaking out of the loop after trying 30 times, like we
> currently do for dedupe operations, which results in logging the operation
> as skipped.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  ltp/fsx.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 5949ebf0..89a5f60e 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -2042,17 +2042,25 @@ test(void)
>  			break;
>  		}
>  	case OP_COPY_RANGE:
> -		TRIM_OFF_LEN(offset, size, file_size);
> -		offset -= offset % readbdy;
> -		if (o_direct)
> -			size -= size % readbdy;
> -		do {
> -			offset2 = random();
> -			TRIM_OFF(offset2, maxfilelen);
> -			offset2 -= offset2 % writebdy;
> -		} while (range_overlaps(offset, offset2, size) ||
> -			 offset2 + size > maxfilelen);
> -		break;
> +		{
> +			int tries = 0;
> +
> +			TRIM_OFF_LEN(offset, size, file_size);
> +			offset -= offset % readbdy;
> +			if (o_direct)
> +				size -= size % readbdy;
> +			do {
> +				if (tries++ >= 30) {
> +					size = 0;
> +					break;
> +				}
> +				offset2 = random();
> +				TRIM_OFF(offset2, maxfilelen);
> +				offset2 -= offset2 % writebdy;
> +			} while (range_overlaps(offset, offset2, size) ||
> +				 offset2 + size > maxfilelen);
> +			break;
> +		}
>  	}
>  
>  have_op:
> -- 
> 2.11.0
>
diff mbox series

Patch

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 5949ebf0..89a5f60e 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -2042,17 +2042,25 @@  test(void)
 			break;
 		}
 	case OP_COPY_RANGE:
-		TRIM_OFF_LEN(offset, size, file_size);
-		offset -= offset % readbdy;
-		if (o_direct)
-			size -= size % readbdy;
-		do {
-			offset2 = random();
-			TRIM_OFF(offset2, maxfilelen);
-			offset2 -= offset2 % writebdy;
-		} while (range_overlaps(offset, offset2, size) ||
-			 offset2 + size > maxfilelen);
-		break;
+		{
+			int tries = 0;
+
+			TRIM_OFF_LEN(offset, size, file_size);
+			offset -= offset % readbdy;
+			if (o_direct)
+				size -= size % readbdy;
+			do {
+				if (tries++ >= 30) {
+					size = 0;
+					break;
+				}
+				offset2 = random();
+				TRIM_OFF(offset2, maxfilelen);
+				offset2 -= offset2 % writebdy;
+			} while (range_overlaps(offset, offset2, size) ||
+				 offset2 + size > maxfilelen);
+			break;
+		}
 	}
 
 have_op: