Re: [PATCH 02/13] libmultipath: Add basic gendisk support

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

On 03/03/2026 12:39, Nilay Shroff wrote:
>>
>>> Yes we got a reference to mpath_head while
>>> allocating it but then these are two (alloc mpath_disk and
>>> alloc mpath_head) disjoint operations. In that case, can't
>>> we have both mpath_disk and mpath_head allocated under one
>>> libmultipath API?
>>
>> I would like to have something simpler (like mainline NVMe code), but 
>> I have it this way because of SCSI, as above.
>>
> I understand the intended lifetime model due to SCSI, but the current 
> flow is somewhat confusing.
> 
> In nvme_mpath_alloc_disk(), mpath_disk and mpath_head are allocated
> separately. However, during teardown, both objects are ultimately
> released through mpath_free_disk(), which drops the reference to
> mpath_head via mpath_put_head().
> 
> Since the allocation of mpath_disk and mpath_head happens independently,
> it is not immediately obvious why their lifetime is tied together and
> why they are not freed independently when the NVMe head node is removed.
> This coupling makes the ownership and reference flow harder to reason
> about.

Yes, and also having 2x separate structures bloats the code, as we are 
continuously looking up one from another and so on. Only having a 
mpath_head for a nvme_ns_head would be nice - I'll look at this approach 
(again).

> 
> Additionally, I noticed that nvme_remove_head() has been removed in the
> NVMe code that integrates with libmultipath. IMO, It might be clearer to
> retain this function and make the teardown sequence explicit (after
> removing mpath_put_head() from mpath_free_disk()).
> For example:
> 
> nvme_remove_head():
>      mpath_unregister_disk();  /* removes mpath_disk and drops its ref */
>      mpath_put_head();         /* drops mpath_head reference */
>      nvme_put_ns_head();       /* drops NVMe namespace head reference */
> 
> Does the above example makes sense?

Yeah, something like that would be simpler. I just need to make it work 
for scsi :)

My current implementation has it that the scsi_device manages the 
mpath_head and the scsi_disk manages the mpath_disk.

Supporting a single structure for scsi is more complicated, as the 
lifetime of the scsi_disk is not tied to that of the scsi_device, i.e. 
we can do something like this:

echo "8:0:0:0" > /sys/bus/scsi/drivers/sd/unbind

Which removes the scsi_disk, but the scsi_device remains. So, for 
multipath support, doing this would mean the that mpath_disk would be 
removed (if the last path), but not the mpath_head.

Furthermore, the scsi_disk info is private to sd.c, so this would mean 
that the mpath_disk could/should also be private. My point is that scsi 
makes things more complicated.

Thanks for checking!