On 27/05/2020 16:14, Hannes Reinecke wrote:
[...]
> @@ -670,8 +683,8 @@ EXPORT_SYMBOL(__scsi_device_lookup_by_target);
> struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *starget,
> u64 lun)
> {
> - struct scsi_device *sdev;
> struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
> + struct scsi_device *sdev;
> unsigned long flags;
This looks unrelated.
>
> spin_lock_irqsave(shost->host_lock, flags);
> @@ -701,19 +714,19 @@ EXPORT_SYMBOL(scsi_device_lookup_by_target);
> * really want to use scsi_device_lookup instead.
> **/
> struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost,
> - uint channel, uint id, u64 lun)
> + u16 channel, u16 id, u64 lun)
> {
> + struct scsi_target *starget;
> struct scsi_device *sdev;
>
> - list_for_each_entry(sdev, &shost->__devices, siblings) {
> - if (sdev->sdev_state == SDEV_DEL)
> - continue;
> - if (sdev->channel == channel && sdev->id == id &&
> - sdev->lun ==lun)
> - return sdev;
> - }
> + starget = __scsi_target_lookup(shost, channel, id);
> + if (!starget)
> + return NULL;
> + sdev = __scsi_device_lookup_by_target(starget, lun);
> + if (sdev && sdev->sdev_state == SDEV_DEL)
> + sdev = NULL;
>
I think the above if is unneeded as __scsi_device_lookup_by_target() does:
list_for_each_entry(sdev, &starget->devices, same_target_siblings) {
if (sdev->sdev_state == SDEV_DEL)
continue;
So 'sdev != NULL && sdev->sdev_state == SDEV_DEL' would never evaluate to true,
wouldn't it?
