Re: [PATCH v3 07/14] block: Do not remove zone write plugs still in use

Hannes Reinecke
2024-05-02T06:09:39Z

On 5/1/24 13:09, Damien Le Moal wrote:
> Large write BIOs that span a zone boundary are split in
> blk_mq_submit_bio() before being passed to blk_zone_plug_bio() for zone
> write plugging. Such split BIO will be chained with one fragment
> targeting one zone and the remainder of the BIO targeting the next
> zone. The two BIOs can be executed in parallel, without a predetermine
> order relative to eachother and their completion may be reversed: the

each other

> remainder first completing and the first fragment then completing. In
> such case, bio_endio() will not immediately execute
> blk_zone_write_plug_bio_endio() for the parent BIO (the remainder of the
> split BIO) as the BIOs are chained. blk_zone_write_plug_bio_endio() for
> the parent BIO will be executed only once the first fragment completes.
>
> In the case of a device with small zones and very large BIOs, uch

such

> completion pattern can lead to disk_should_remove_zone_wplug() to return
> true for the zone of the parent BIO when the parent BIO request
> completes and blk_zone_write_plug_complete_request() is executed. This
> triggers the removal of the zone write plug from the hash table using
> disk_remove_zone_wplug(). With the zone write plug of the parent BIO
> missing, the call to disk_get_zone_wplug() in
> blk_zone_write_plug_bio_endio() returns NULL and triggers a warning.
>
> This patterns can be recreated fairly easily using a scsi_debug device
> with small zone and btrfs. E.g.
>
> modprobe scsi_debug delay=0 dev_size_mb=1024 sector_size=4096 \
> zbc=host-managed zone_cap_mb=3 zone_nr_conv=0 zone_size_mb=4
> mkfs.btrfs -f -O zoned /dev/sda
> mount -t btrfs /dev/sda /mnt
> fio --name=wrtest --rw=randwrite --direct=1 --ioengine=libaio \
> --bs=4k --iodepth=16 --size=1M --directory=/mnt --time_based \
> --runtime=10
> umount /dev/sda
>
> Will result in the warning:
>
> [ 29.035538] WARNING: CPU: 3 PID: 37 at block/blk-zoned.c:1207 blk_zone_write_plug_bio_endio+0xee/0x1e0
> ...
> [ 29.058682] Call Trace:
> [ 29.059095] <TASK>
> [ 29.059473] ? __warn+0x80/0x120
> [ 29.059983] ? blk_zone_write_plug_bio_endio+0xee/0x1e0
> [ 29.060728] ? report_bug+0x160/0x190
> [ 29.061283] ? handle_bug+0x36/0x70
> [ 29.061830] ? exc_invalid_op+0x17/0x60
> [ 29.062399] ? asm_exc_invalid_op+0x1a/0x20
> [ 29.063025] ? blk_zone_write_plug_bio_endio+0xee/0x1e0
> [ 29.063760] bio_endio+0xb7/0x150
> [ 29.064280] btrfs_clone_write_end_io+0x2b/0x60 [btrfs]
> [ 29.065049] blk_update_request+0x17c/0x500
> [ 29.065666] scsi_end_request+0x27/0x1a0 [scsi_mod]
> [ 29.066356] scsi_io_completion+0x5b/0x690 [scsi_mod]
> [ 29.067077] blk_complete_reqs+0x3a/0x50
> [ 29.067692] __do_softirq+0xcf/0x2b3
> [ 29.068248] ? sort_range+0x20/0x20
> [ 29.068791] run_ksoftirqd+0x1c/0x30
> [ 29.069339] smpboot_thread_fn+0xcc/0x1b0
> [ 29.069936] kthread+0xcf/0x100
> [ 29.070438] ? kthread_complete_and_exit+0x20/0x20
> [ 29.071314] ret_from_fork+0x31/0x50
> [ 29.071873] ? kthread_complete_and_exit+0x20/0x20
> [ 29.072563] ret_from_fork_asm+0x11/0x20
> [ 29.073146] </TASK>
>
> either when fio executes or when unmount is executed.
>
> Fix this by modifying disk_should_remove_zone_wplug() to check that the
> reference count to a zone write plug is not larger than 2, that is, that
> the only references left on the zone are the caller held reference
> (blk_zone_write_plug_complete_request()) and the initial extra reference
> for the zone write plug taken when it was initialized (and that is
> dropped when the zone write plug is removed from the hash table).
>
> To be consistent with this change, make sure to drop the request or BIO
> held reference to the zone write plug before calling
> disk_zone_wplug_unplug_bio(). All references are also dropped using
> disk_put_zone_wplug() instead of atomic_dec() to ensure that the zone
> write plug is freed if it needs to be.
>
> Comments are also improved to clarify zone write plugs reference
> handling.
>
> Reported-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
> block/blk-zoned.c | 39 +++++++++++++++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich