On 8/07/21 3:31 pm, Rafael J. Wysocki wrote:
> On Wed, Jul 7, 2021 at 7:49 PM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>>
>> On 7/07/21 8:39 pm, Greg Kroah-Hartman wrote:
>>> On Wed, Jul 07, 2021 at 08:29:48PM +0300, Adrian Hunter wrote:
>>>> If a LUN fails to probe (e.g. absent BOOT WLUN), the device will not have
>>>> been registered but can still have a device link holding a reference to the
>>>> device. The unwanted device link will prevent runtime suspend indefinitely,
>>>> and cause some warnings if the supplier is ever deleted (e.g. by unbinding
>>>> the UFS host controller). Fix by explicitly deleting the device link when
>>>> SCSI destroys the SCSI device.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>>>> ---
>>>> drivers/scsi/ufs/ufshcd.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>>>> index 708b3b62fc4d..483aa74fe2c8 100644
>>>> --- a/drivers/scsi/ufs/ufshcd.c
>>>> +++ b/drivers/scsi/ufs/ufshcd.c
>>>> @@ -5029,6 +5029,13 @@ static void ufshcd_slave_destroy(struct scsi_device *sdev)
>>>> spin_lock_irqsave(hba->host->host_lock, flags);
>>>> hba->sdev_ufs_device = NULL;
>>>> spin_unlock_irqrestore(hba->host->host_lock, flags);
>>>> + } else {
>>>> + /*
>>>> + * If a LUN fails to probe (e.g. absent BOOT WLUN), the device
>>>> + * will not have been registered but can still have a device
>>>> + * link holding a reference to the device.
>>>> + */
>>>> + device_links_scrap(&sdev->sdev_gendev);
>>>
>>> What created that link? And why did it do that before probe happened
>>> successfully?
>>
>> The same driver created the link.
>>
>> The documentation seems to say it is allowed to, if it is the consumer.
>> From Documentation/driver-api/device_link.rst
>>
>> Usage
>> =====
>>
>> The earliest point in time when device links can be added is after
>> :c:func:`device_add()` has been called for the supplier and
>> :c:func:`device_initialize()` has been called for the consumer.
>
> Yes, this is allowed, but if you've added device links to a device
> object that is not going to be registered after all, you are
> responsible for doing the cleanup.
>
> Why can't you call device_link_del() directly on those links?
>
> Or device_link_remove() if you don't want to deal with link pointers?
>
Those only work for DL_FLAG_STATELESS device links, but we use only
DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE flags.
