diff mbox series

[v2,4/4] fsx: move range generation logic into a common helper

Message ID 20200408181208.12054-1-fdmanana@kernel.org (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Filipe Manana April 8, 2020, 6:12 p.m. UTC
From: Filipe Manana <fdmanana@suse.com>

We have very similar code that generates the destination range for clone,
dedupe and copy_file_range operations, so avoid duplicating the code three
times and move it into a helper function.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: Turned the first parameter of the helper into a boolean as Darrick suggested.

 ltp/fsx.c | 91 +++++++++++++++++++++++++--------------------------------------
 1 file changed, 36 insertions(+), 55 deletions(-)

Comments

Brian Foster April 17, 2020, 5:10 p.m. UTC | #1
On Wed, Apr 08, 2020 at 07:12:08PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> We have very similar code that generates the destination range for clone,
> dedupe and copy_file_range operations, so avoid duplicating the code three
> times and move it into a helper function.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> V2: Turned the first parameter of the helper into a boolean as Darrick suggested.
> 
>  ltp/fsx.c | 91 +++++++++++++++++++++++++--------------------------------------
>  1 file changed, 36 insertions(+), 55 deletions(-)
> 
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 89a5f60e..9bfc98e0 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
...
> @@ -2004,63 +2034,14 @@ test(void)
>  			keep_size = random() % 2;
>  		break;
...
>  	case OP_COPY_RANGE:
> -		{
> -			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;

It looks like this writebdy bit is lost in the new helper...

Brian

> -			} while (range_overlaps(offset, offset2, size) ||
> -				 offset2 + size > maxfilelen);
> -			break;
> -		}
> +		generate_dest_range(true, maxfilelen, &offset, &size, &offset2);
> +		break;
>  	}
>  
>  have_op:
> -- 
> 2.11.0
>
Filipe Manana April 17, 2020, 5:20 p.m. UTC | #2
On Fri, Apr 17, 2020 at 6:10 PM Brian Foster <bfoster@redhat.com> wrote:
>
> On Wed, Apr 08, 2020 at 07:12:08PM +0100, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > We have very similar code that generates the destination range for clone,
> > dedupe and copy_file_range operations, so avoid duplicating the code three
> > times and move it into a helper function.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >
> > V2: Turned the first parameter of the helper into a boolean as Darrick suggested.
> >
> >  ltp/fsx.c | 91 +++++++++++++++++++++++++--------------------------------------
> >  1 file changed, 36 insertions(+), 55 deletions(-)
> >
> > diff --git a/ltp/fsx.c b/ltp/fsx.c
> > index 89a5f60e..9bfc98e0 100644
> > --- a/ltp/fsx.c
> > +++ b/ltp/fsx.c
> ...
> > @@ -2004,63 +2034,14 @@ test(void)
> >                       keep_size = random() % 2;
> >               break;
> ...
> >       case OP_COPY_RANGE:
> > -             {
> > -                     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;
>
> It looks like this writebdy bit is lost in the new helper...

Yes, thanks for pointing out. I'll send a v3.

>
> Brian
>
> > -                     } while (range_overlaps(offset, offset2, size) ||
> > -                              offset2 + size > maxfilelen);
> > -                     break;
> > -             }
> > +             generate_dest_range(true, maxfilelen, &offset, &size, &offset2);
> > +             break;
> >       }
> >
> >  have_op:
> > --
> > 2.11.0
> >
>
diff mbox series

Patch

diff --git a/ltp/fsx.c b/ltp/fsx.c
index 89a5f60e..9bfc98e0 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1930,6 +1930,36 @@  range_overlaps(
 	return llabs((unsigned long long)off1 - off0) < size;
 }
 
+static void generate_dest_range(bool readbdy_align,
+				unsigned long max_range_end,
+				unsigned long *src_offset,
+				unsigned long *size,
+				unsigned long *dst_offset)
+{
+	int tries = 0;
+
+	TRIM_OFF_LEN(*src_offset, *size, file_size);
+	if (readbdy_align) {
+		*src_offset -= *src_offset % readbdy;
+		if (o_direct)
+			*size -= *size % readbdy;
+	} else {
+		*src_offset = *src_offset & ~(block_size - 1);
+		*size = *size & ~(block_size - 1);
+	}
+
+	do {
+		if (tries++ >= 30) {
+			*size = 0;
+			break;
+		}
+		*dst_offset = random();
+		TRIM_OFF(*dst_offset, max_range_end);
+		*dst_offset = *dst_offset & ~(block_size - 1);
+	} while (range_overlaps(*src_offset, *dst_offset, *size) ||
+		 *dst_offset + *size > max_range_end);
+}
+
 int
 test(void)
 {
@@ -2004,63 +2034,14 @@  test(void)
 			keep_size = random() % 2;
 		break;
 	case OP_CLONE_RANGE:
-		{
-			int tries = 0;
-
-			TRIM_OFF_LEN(offset, size, file_size);
-			offset = offset & ~(block_size - 1);
-			size = size & ~(block_size - 1);
-			do {
-				if (tries++ >= 30) {
-					size = 0;
-					break;
-				}
-				offset2 = random();
-				TRIM_OFF(offset2, maxfilelen);
-				offset2 = offset2 & ~(block_size - 1);
-			} while (range_overlaps(offset, offset2, size) ||
-				 offset2 + size > maxfilelen);
-			break;
-		}
+		generate_dest_range(false, maxfilelen, &offset, &size, &offset2);
+		break;
 	case OP_DEDUPE_RANGE:
-		{
-			int tries = 0;
-
-			TRIM_OFF_LEN(offset, size, file_size);
-			offset = offset & ~(block_size - 1);
-			size = size & ~(block_size - 1);
-			do {
-				if (tries++ >= 30) {
-					size = 0;
-					break;
-				}
-				offset2 = random();
-				TRIM_OFF(offset2, file_size);
-				offset2 = offset2 & ~(block_size - 1);
-			} while (range_overlaps(offset, offset2, size) ||
-				 offset2 + size > file_size);
-			break;
-		}
+		generate_dest_range(false, file_size, &offset, &size, &offset2);
+		break;
 	case OP_COPY_RANGE:
-		{
-			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;
-		}
+		generate_dest_range(true, maxfilelen, &offset, &size, &offset2);
+		break;
 	}
 
 have_op: