On 02/16/2017 05:09 PM, Bart Van Assche wrote:
> On Thu, 2017-02-16 at 16:12 +0100, Hannes Reinecke wrote:
>> +struct scsi_device *scsi_device_from_queue(struct request_queue *q)
>> +{
>> + struct scsi_device *sdev = NULL;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(q->queue_lock, flags);
>> + if (q->mq_ops) {
>> + if (q->mq_ops == &scsi_mq_ops)
>> + sdev = q->queuedata;
>> + } else if (q->request_fn == scsi_request_fn)
>> + sdev = q->queuedata;
>> + if (!sdev || !get_device(&sdev->sdev_gendev))
>> + sdev = NULL;
>> + spin_unlock_irqrestore(q->queue_lock, flags);
>> +
>> + return sdev;
>> +}
>
> Hello Hannes,
>
> Do we need to take the queue lock? Neither q->mq_ops nor q->request_fn are
> modified after a block device has been created. q->queuedata is not modified
> by any SCSI driver after it has been set. And since the caller of
> scsi_device_from_queue() has to guarantee that the queue does not disappear
> while this function is in progress, the queue lock does not have to be held
> around the get_device() call either. Otherwise this patch looks fine to me.
>
Well, we did for the original code path, so I assumed that it'd be
prudent to do so.
And it's hardly time-critical anyway.
But yes, I guess we could do without the lock.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
