| Message ID | 20200417173221.6380-1-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, Apr 17, 2020 at 06:32:21PM +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. > V3: Added destination offset align by writebdy when bdy_align is true. > > ltp/fsx.c | 94 ++++++++++++++++++++++++++------------------------------------- > 1 file changed, 39 insertions(+), 55 deletions(-) > > diff --git a/ltp/fsx.c b/ltp/fsx.c > index 89a5f60e..2e51169b 100644 > --- a/ltp/fsx.c > +++ b/ltp/fsx.c > @@ -1930,6 +1930,39 @@ range_overlaps( > return llabs((unsigned long long)off1 - off0) < size; > } > > +static void generate_dest_range(bool bdy_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 (bdy_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); > + if (bdy_align) > + *dst_offset = *dst_offset & writebdy; That still doesn't look right (and either way we might as well use consistent logic for readbdy and writebdy). Brian > + else > + *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 +2037,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: > -- > 2.11.0 >
On Mon, Apr 20, 2020 at 3:34 PM Brian Foster <bfoster@redhat.com> wrote: > > On Fri, Apr 17, 2020 at 06:32:21PM +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. > > V3: Added destination offset align by writebdy when bdy_align is true. > > > > ltp/fsx.c | 94 ++++++++++++++++++++++++++------------------------------------- > > 1 file changed, 39 insertions(+), 55 deletions(-) > > > > diff --git a/ltp/fsx.c b/ltp/fsx.c > > index 89a5f60e..2e51169b 100644 > > --- a/ltp/fsx.c > > +++ b/ltp/fsx.c > > @@ -1930,6 +1930,39 @@ range_overlaps( > > return llabs((unsigned long long)off1 - off0) < size; > > } > > > > +static void generate_dest_range(bool bdy_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 (bdy_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); > > + if (bdy_align) > > + *dst_offset = *dst_offset & writebdy; > > That still doesn't look right (and either way we might as well use > consistent logic for readbdy and writebdy). Right, I made the mistakes of using & instead of % and = instead of -=. They were not intentional, just a friday afternoon type of thing. Thanks, correct in the next version. > > Brian > > > + else > > + *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 +2037,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: > > -- > > 2.11.0 > > >
diff --git a/ltp/fsx.c b/ltp/fsx.c index 89a5f60e..2e51169b 100644 --- a/ltp/fsx.c +++ b/ltp/fsx.c @@ -1930,6 +1930,39 @@ range_overlaps( return llabs((unsigned long long)off1 - off0) < size; } +static void generate_dest_range(bool bdy_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 (bdy_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); + if (bdy_align) + *dst_offset = *dst_offset & writebdy; + else + *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 +2037,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: