Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/04/13 10:31, Hannes Reinecke wrote:
> On 4/12/26 21:40, Phil Pemberton wrote:
>> On 12/04/2026 08:38, Damien Le Moal wrote:
>>> On 4/9/26 23:05, Phil Pemberton wrote:
>>>>    - Raises max_lun from 1 to 8 (matching the SCSI host default).
>>>>      Sequential LUN scanning stops at the first non-responding LUN, so
>>>>      single-LUN devices are unaffected.
>>>
>>> If the only case that we can encounter with libata are these special 
>>> ATAPI
>>> devices with 2 LUNs, I would limit the maximum to 2.
>>
>> I'm inclined to agree, but there are devices with higher LUN counts: the 
>> Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI 
>> variant which exposed a LUN for each disc in the changer stack. There's 
>> a Cathode Ray Dude video demonstrating the latter.
>>
> Don't get too excited. This is ancient technology, with an extremely 
> narrow user-base :-)
> 'Were available' is not identical to 'in current use', and I'm somewhat
> disinclined to add support for technology with no users.
> 
>> I like the idea of the lower LUN cap for compatibility, but I think I'd 
>> hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) 
>> to override it. Default 2 seems like a good compromise.
>>
> Hmm. I really would like to restrict max lun to 1 (as it somewhat mimics 
> the master/slave thingie on IDE). But higher than that really is 
> arbitrary; the next sensible limit would be '7', as this is what SCSI-2
> could handle. I completely fail to see the motivation to have a limit
> somewhere in between.
> 
>> In any case, the BLIST_FORCELUN gate should limit things to one LUN for 
>> any device which isn't on the device list.
>>
>>
>>>>    - ata_scsi_dev_config() previously assigned dev->sdev = sdev for 
>>>> every
>>>>      LUN configured.  With multiple LUNs sharing one ata_device, this
>>>>      caused dev->sdev to be overwritten by each non-LUN-0 sdev.  
>>>> Restrict
>>>>      the assignment to LUN 0 so that dev->sdev always tracks the
>>>>      canonical scsi_device for the underlying ATA device.
>>>
>>> Special casing this does not seem nice at all. Why not simply 
>>> increasing the
>>> sdev reference count when it is assigned to a LUN that is not LUN 0 ? 
>>> And drop
>>> that reference when the LUN is torn down ? That will remove any 
>>> dependency on
>>> the order in which LUNs are torn down too.
>>
>> The if (sdev->lun == 0) guard isn't about teardown ordering; it's about 
>> which device dev->sdev points at.
>>
>> dev->sdev is a single pointer, but with multi-LUN ATAPI there are now 
>> multiple sdevs sharing one ata_device. Without the guard, each call to 
>> ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the 
>> last-configured LUN (likely the highest-numbered one).
>>
> And how would you address the 'sdev' of LUN 1?
> (and how of LUN 2, if we decide to have one?)
> 
> Please, don't. The correct way would be to convert 'dev->sdev' into
> a list.

Or an array sized to the max number of LUNs we allow (7 ?). That would be way
easier to handle than a list I think since we can index that by LUN number.
Unless LUNs can have any number ? I did not check...

And checking suspend/resume will be needed as we likely will need to keep track
of the number of LUNs already suspended before being able to suspend the port.
scsi layer should already be doing something like that for the scsi host though
(host == port here), so luckily, that should not too bad to handle.


-- 
Damien Le Moal
Western Digital Research