On Wed, Apr 08, 2020 at 11:36:27AM +0100, fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> ltp/fsx.c | 91 +++++++++++++++++++++++++--------------------------------------
> 1 file changed, 36 insertions(+), 55 deletions(-)
>
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 89a5f60e..8873cd01 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(unsigned long op,
> + 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 (op == OP_COPY_RANGE) {
It feels funny that we pass in @op just to let COPY RANGE relax the
block size alignment requirements. How about passing a block_align
parameter (instead of op) which would be the subject of the if test?
This way we concentrate the alignment requirement knowlege of each type
of call in the "case OP_FOO" part of the code instead of scattering it?
--D
> + *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(op, 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(op, 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(op, maxfilelen, &offset, &size, &offset2);
> + break;
> }
>
> have_op:
> --
> 2.11.0
>
