Re: [PATCH V4] btrfs-progs: Make btrfs_prepare_device parallel during mkfs.btrfs

[David Sterba <dsterba@suse.cz>]

On Sun, Sep 04, 2022 at 04:24:23PM +0800, Li Zhang wrote:
> [enhancement]
> When a disk is formatted as btrfs, it calls
> btrfs_prepare_device for each device, which takes too much time.
> 
> [implementation]
> Put each btrfs_prepare_device into a thread,
> wait for the first thread to complete to mkfs.btrfs,
> and wait for other threads to complete before adding
> other devices to the file system.
> 
> [test]
> Using the btrfs-progs test case mkfs-tests, mkfs.btrfs works fine.
> 
> Use tcmu-runner emulation to simulate two devices for testing.
> Each device is 2000G (about 19.53 TiB), the region size is 4MB,
> Use the following parameters for targetcli
> create name=zbc0 size=20000G cfgstring=model-HM/zsize-4/conv-100@~/zbc0.raw
> 
> Call difftime to calculate the running time of the function btrfs_prepare_device.
> Calculate the time from thread creation to completion of all threads after
> patching (time calculation is not included in patch submission)
> The test results are as follows.
> 
> $ lsscsi -p
> [10:0:1:0]   (0x14)  LIO-ORG  TCMU ZBC device  0002  /dev/sdb   -          none
> [11:0:1:0]   (0x14)  LIO-ORG  TCMU ZBC device  0002  /dev/sdc   -          none
> 
> $ sudo mkfs.btrfs -d single -m single -O zoned /dev/sdc /dev/sdb -f
> ....
> time for prepare devices:4.000000.
> ....
> 
> $ sudo mkfs.btrfs -d single -m single -O zoned /dev/sdc /dev/sdb -f
> ...
> time for prepare devices:2.000000.
> ...
> 
> Signed-off-by: Li Zhang <zhanglikernel@gmail.com>
> ---
> Issue: 496

Thanks for implementing it. I did only some style fixups but the
parallel zone reset works (and trim/discard as well as it's the same
function).

I've tested it with the null blk devices (128M, 4M zone, 32 zones) and
added some more debugging to see how it works, with artificial delay
100ms to zone reset. The overall runtime is about the same when running
on devices:

 2 - 4s
 6 - 5s
 8 - 5s
10 - 6s

It's fast so there's some noise in the numbers but it's not linear in
the number of devices and the threads may also wait on the printing lock
as I've added some debugging messages.

Sample run with your patch reverted and same delay:

 2 - 7s
 6 - 21s
 8 - 27s
10 - 35s

> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -969,6 +984,31 @@ fail:
>  	return ret;
>  }
>  
> +static void *prepare_one_dev(void *ctx)
> +{
> +	struct prepare_device_progress *prepare_ctx = ctx;
> +	int fd;
> +
> +	fd = open(prepare_ctx->file, opt_oflags);
> +	if (fd < 0) {
> +		pthread_mutex_lock(&prepare_mutex);

Is this lock actually needed? printf is thread safe and there are other
messages printed from btrfs_prepare_device anyway, so using this lock
does not help.

> +		error("unable to open %s: %m", prepare_ctx->file);
> +		pthread_mutex_unlock(&prepare_mutex);
> +		prepare_ctx->ret = fd;
> +		return NULL;
> +	}
> +	prepare_ctx->ret = btrfs_prepare_device(fd,
> +			prepare_ctx->file,
> +			&prepare_ctx->dev_block_count,
> +			prepare_ctx->block_count,
> +			(bconf.verbose ? PREP_DEVICE_VERBOSE : 0) |
> +			(opt_zero_end ? PREP_DEVICE_ZERO_END : 0) |
> +			(opt_discard ? PREP_DEVICE_DISCARD : 0) |
> +			(opt_zoned ? PREP_DEVICE_ZONED : 0));
> +	close(fd);
> +	return NULL;
> +}