Re: [PATCH v16 01/26] blk-zoned: Fix a reference count leak

Bart Van Assche
2024-11-19T20:21:24Z

On 11/18/24 6:23 PM, Damien Le Moal wrote:
> On 11/19/24 9:27 AM, Bart Van Assche wrote:
>> Fix a reference count leak in disk_zone_wplug_handle_error()
>>
>> Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>> ---
>> block/blk-zoned.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>> index 70211751df16..3346b8c53605 100644
>> --- a/block/blk-zoned.c
>> +++ b/block/blk-zoned.c
>> @@ -1337,6 +1337,8 @@ static void disk_zone_wplug_handle_error(struct gendisk *disk,
>>
>> unlock:
>> spin_unlock_irqrestore(&zwplug->lock, flags);
>> +
>> + disk_put_zone_wplug(zwplug);
>
> The zone wplug put call is right after the single call site to
> disk_zone_wplug_handle_error(). The reason it is *not* in that function is that
> the reference on the wplug for handling an error is taken when the wplug is
> added to the error list. disk_zone_wplug_handle_error() does not itself take a
> reference on the wplug.
>
> So how did you come up with this ? What workload/operation did you run to find
> an issue ?

Thanks for the feedback. I can't reproduce the refcount leak without my
other patches. I will check with which other patch this patch has to be
combined. This is the script that triggered the leak:

#!/bin/bash

set -eu

qd=${1:-64}
if modprobe -r scsi_debug; then :; fi
params=(
delay=0
dev_size_mb=256
every_nth=$((2 * qd))
max_queue=${qd}
ndelay=100000 # 100 us
opts=0x8000 # SDEBUG_OPT_HOST_BUSY
preserves_write_order=1
sector_size=4096
zbc=host-managed
zone_nr_conv=0
zone_size_mb=4
)
modprobe scsi_debug "${params[@]}"
while true; do
bdev=$(cd
/sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/*/block &&
echo *) 2>/dev/null
if [ -e /dev/"${bdev}" ]; then break; fi
sleep .1
done
dev=/dev/"${bdev}"
[ -b "${dev}" ]
for rw in write randwrite; do
params=(
--direct=1
--filename="${dev}"
--iodepth="${qd}"
--ioengine=io_uring
--ioscheduler=none
--gtod_reduce=1
--hipri=0
--name="$(basename "${dev}")"
--runtime=30
--rw="$rw"
--time_based=1
--zonemode=zbd
)
fio "${params[@]}"
rc=$?
if grep -avH " ref 1 " /sys/kernel/debug/block/sda/zone_wplugs;
then :; fi
echo ''
[ $rc = 0 ] || break
done