Re: [PATCH v3 04/13] libmultipath: Add bio handling

[John Garry <john.g.garry@oracle.com>]

On 03/07/2026 11:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] TOCTOU race in `mpath_remove_disk` can strand a bio on `requeue_list`, causing `del_gendisk` to deadlock.
> - [Medium] The `trace_block_bio_remap` tracepoint is called with the wrong device argument for `old_dev`, causing the trace event to emit identical devices for source and destination.
> --
> 
> commit 1f99dbf217347b66f1710f734eac617209a4e8c6
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add bio handling
> 
> This commit adds per-path bio submission support and failover requeuing
> logic for libmultipath. It introduces mechanisms similar to NVMe multipath
> to clone bios and submit them with the REQ_MPATH flag.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 21f7ffdb22d60..81e737c1ce469 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> 
> [ ... ]
> 
>> @@ -243,6 +242,81 @@ static struct mpath_device *mpath_find_path(struct mpath_head *mpath_head)
>> +static void mpath_bdev_submit_bio(struct bio *bio)
>> +{
> 
> [ ... ]
> 
>> +		bio_set_dev(bio, mpath_device->disk->part0);
>> +		/*
>> +		 * Use BIO_REMAPPED to skip bio_check_eod() when this bio
>> +		 * enters submit_bio_noacct() for the per-path device. The EOD
>> +		 * check already passed on the multipath head.
>> +		 */
>> +		bio_set_flag(bio, BIO_REMAPPED);
>> +		bio->bi_opf |= REQ_MPATH;
>> +		trace_block_bio_remap(bio, disk_devt(mpath_device->disk),
>> +				      bio->bi_iter.bi_sector);
> 
> [Severity: Medium]
> Does this tracepoint call incorrectly use the target path device instead of
> the original head device?
> 
> By passing disk_devt(mpath_device->disk), it seems the trace event will log
> the new path device as both the source and destination, which might break
> trace analysis. Should this be disk_devt(mpath_head->disk)?

But with bio_set_dev(), the new device is from mpath_device->disk->part0 
and old is the mpath_head disk in mpath_head->disk

> 
>> +		submit_bio_noacct(bio);
>> +	} else if (mpath_available_path(mpath_head)) {
>> +		dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
>> +
>> +		spin_lock_irq(&mpath_head->requeue_lock);
>> +		bio_list_add(&mpath_head->requeue_list, bio);
>> +		spin_unlock_irq(&mpath_head->requeue_lock);
>> +		atomic_long_inc(&mpath_head->requeue_no_usable_path_cnt);
>> +	} else {
> 
> [ ... ]
> 
>> @@ -314,11 +389,34 @@ static void multipath_partition_scan_work(struct work_struct *work)
>>   void mpath_remove_disk(struct mpath_head *mpath_head)
>>   {
>>   	if (test_and_clear_bit(MPATH_HEAD_DISK_LIVE, &mpath_head->flags)) {
>>   		struct gendisk *disk = mpath_head->disk;
>>   
>> +		/*
>> +		 * requeue I/O after MPATH_HEAD_DISK_LIVE has been cleared
>> +		 * to allow multipath to fail all I/O.
>> +		 */
>> +		mpath_schedule_requeue_work(mpath_head);
>> +
>>   		mpath_synchronize(mpath_head);
> 
> [Severity: High]
> Could scheduling the requeue work before synchronizing SRCU cause a deadlock
> during device removal?
> 
> If mpath_bdev_submit_bio() checks mpath_available_path() and sees the disk
> is live, but then gets preempted before taking the requeue_lock,
> mpath_remove_disk() might clear MPATH_HEAD_DISK_LIVE and schedule the
> requeue worker.
> 
> The worker would run and find the requeue_list empty. When the submitter
> wakes up and adds the bio, it will be stranded on the requeue_list.
> 
> Because the bio holds a q_usage_counter reference, will del_gendisk()
> hang forever waiting for the queue to freeze?

Yeah, maybe we should do that. This is same as mainline NVMe multipath 
code today.

> 
>>   		del_gendisk(disk);
>>   	}
>