On Sat, Mar 05, 2022 at 06:03:39PM -0800, Bart Van Assche wrote:
>> - scsi_disk_put(sdkp);
>> + scsi_device_put(sdkp->device);
>> return retval;
>> }
>
> Hmm ... why is the above scsi_device_put() call passed sdkp->device?
> Wouldn't it be more symmetric to pass 'sdev' to that function?
>
>> @@ -1502,7 +1468,7 @@ static void sd_release(struct gendisk *disk, fmode_t mode)
>> scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
>> }
>> - scsi_disk_put(sdkp);
>> + scsi_device_put(sdkp->device);
>> }
>
> Same question here - why to pass sdkp->device instead of sdev?
Yes, we can just pass sdev in both cases as that is a bit cleaner.
>
>> +static void scsi_disk_free_disk(struct gendisk *disk)
>> +{
>> + struct scsi_disk *sdkp = disk->private_data;
>> +
>> + put_device(&sdkp->disk_dev);
>> +}
>
> Can the body of the above function be written as
> put_device(&scsi_disk(disk)->disk_dev) ? I'm asking this because other
> parts of this patch use scsi_disk() instead of using disk->private_data
> directly.
The scsi_disk() helper is a bit pointless now, but I could use it
here for now. In the long run we should probably just remove
scsi_disk() entirely.
