On 2023/09/27 16:55, Bart Van Assche wrote:
> On 9/26/23 01:14, Damien Le Moal wrote:
>> @@ -3891,21 +3895,26 @@ static int sd_suspend_runtime(struct device *dev)
>> static int sd_resume(struct device *dev, bool runtime)
>> {
>> struct scsi_disk *sdkp = dev_get_drvdata(dev);
>> - int ret;
>> + int ret = 0;
>>
>> if (!sdkp) /* E.g.: runtime resume at the start of sd_probe() */
>> return 0;
>
> As far as I can tell there is nothing that prevents system wide
> suspend or resume after a SCSI disk has been discovered and before
> sd_probe() is called(). So I think that "sdkp->suspended = false;"
> has to be added in the above if-statement. This is the how SCSI
> disks are registered (synchronous case only):
The above if statement being "if (!sdkp)", if I add "sdkp->suspended = false;",
I will get a NULL pointer dereference. So I do not understand your point here...
>
> scsi_probe_and_add_lun(target, bflagsp, sdevp, rescan, hostdata)
> scsi_alloc_sdev(starget, lun, hostdata)
> __scsi_init_queue(&sdev->host)
> scsi_sysfs_device_initialize(sdev)
> shost->hostt->slave_alloc(sdev)
> scsi_probe_lun(sdev, ...)
> scsi_execute_req(sdev, INQUIRY)
> scsi_add_lun(sdev, ...)
> scsi_device_set_state(sdev, SDEV_RUNNING)
> sdev->host->hostt->slave_configure(sdev) /* may do I/O */
> scsi_sysfs_add_sdev(sdev) /* enables runtime PM */
> scsi_target_add(starget)
> device_add(&sdev->sdev_gendev)
> kobject_add(&dev->kobj, ...)
> bus_add_device(dev)
> bus_probe_device(dev)
> device_initial_probe(dev)
> __device_attach(dev, /*allow_async=*/true)
> __device_attach_driver(drv, dev, ...)
> driver_probe_device(drv, dev)
> really_probe(dev, drv)
> dev->bus->probe(dev) = sd_probe(dev)
> gd = blk_mq_alloc_disk_for_queue()
> device_add(&sdkp->dev)
> sd_revalidate_disk(gd)
> device_add_disk(dev, gd, NULL)
> device_add(&sdev->sdev_dev)
> bsg_scsi_register_queue(rq, &sdev->sdev_gendev)
>
> Thanks,
>
> Bart.
>
--
Damien Le Moal
Western Digital Research
