On 04/06/2012 11:39 AM, Bart Van Assche wrote: > On 04/05/12 13:58, Tomas Henzl wrote: > >> When the async scan is protected this way a further protection via ref. count is no more needed >> so remove it >> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c >> index 29c4c04..be9e6fe 100644 >> --- a/drivers/scsi/scsi_scan.c >> +++ b/drivers/scsi/scsi_scan.c >> @@ -1743,10 +1743,9 @@ static struct async_scan_data *scsi_prep_async_scan(struct Scsi_Host *shost) >> >> data = kmalloc(sizeof(*data), GFP_KERNEL); >> if (!data) >> - goto err; >> - data->shost = scsi_host_get(shost); >> - if (!data->shost) >> - goto err; >> + return NULL; >> + >> + data->shost = shost; >> init_completion(&data->prev_finished); >> >> mutex_lock(&shost->scan_mutex); > > Maybe it's better to leave the scsi_host_get() / scsi_host_put() pair in > scsi_prep_async_scan() and scsi_finish_async_scan(). Let's have a look > at the following code in scsi_finish_async_scan(): > > spin_lock_irqsave(shost->host_lock, flags); > shost->async_scan = 0; > spin_unlock_irqrestore(shost->host_lock, flags); > > If the context that is waiting for shost->async_scan wouldn't lock > shost->host_lock after having observed that shost->async_scan became > zero then shost->host_lock could already have been freed before > scsi_finish_async_scan() has unlocked that spin lock. Personally I > prefer to avoid having such subtle dependencies between the async > scanning code and the context waiting for it to finish. It's also my preference It's not built in for purpose, I only haven't noticed it :) Thanks for the advice. Tomas > > Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
