> On Oct 29, 2015, at 3:30 PM, Don Brace <brace77...@gmail.com> wrote:
> 
> On 10/29/2015 12:21 PM, Matthew R. Ochs wrote:
>>> On Oct 28, 2015, at 5:06 PM, Don Brace <don.br...@pmcs.com> wrote:
>>> 
>>> From: Kevin Barnett <kevin.barn...@pmcs.com>
>>> 
>>> preparation for adding the sas transport class
>>> 
>>> Reviewed-by: Scott Teel <scott.t...@pmcs.com>
>>> Reviewed-by: Justin Lindley <justin.lind...@pmcs.com>
>>> Reviewed-by: Kevin Barnett <kevin.barn...@pmcs.com>
>>> Signed-off-by: Don Brace <don.br...@pmcs.com>
>>> ---
>>> drivers/scsi/hpsa.c |   65 
>>> +++++++++++++++++++++++++++++++--------------------
>>> 1 file changed, 39 insertions(+), 26 deletions(-)
>>> 
>>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>>> index 24b3c8c..06207e2 100644
>>> --- a/drivers/scsi/hpsa.c
>>> +++ b/drivers/scsi/hpsa.c
>>> @@ -1660,6 +1660,37 @@ static void 
>>> hpsa_update_log_drive_phys_drive_ptrs(struct ctlr_info *h,
>>>     }
>>> }
>>> 
>>> +static int hpsa_add_device(struct ctlr_info *h, struct hpsa_scsi_dev_t 
>>> *device)
>>> +{
>>> +   int rc = 0;
>>> +
>>> +   rc = scsi_add_device(h->scsi_host, device->bus,
>>> +                                   device->target, device->lun);
>>> +   return rc;
>>> +}
>>> +
>>> +static void hpsa_remove_device(struct ctlr_info *h,
>>> +                   struct hpsa_scsi_dev_t *device)
>>> +{
>>> +   struct scsi_device *sdev = NULL;
>>> +
>>> +   sdev = scsi_device_lookup(h->scsi_host, device->bus,
>>> +                                           device->target, device->lun);
>>> +
>>> +   if (sdev) {
>>> +           scsi_remove_device(sdev);
>>> +           scsi_device_put(sdev);
>>> +   } else {
>>> +           /*
>>> +            * We don't expect to get here.  Future commands
>>> +            * to this device will get a selection timeout as
>>> +            * if the device were gone.
>>> +            */
>>> +           hpsa_show_dev_msg(KERN_WARNING, h, device,
>>> +                                   "didn't find device for removal.");
>>> +   }
>>> +}
>>> +
>>> static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
>>>     struct hpsa_scsi_dev_t *sd[], int nsds)
>>> {
>>> @@ -1672,7 +1703,6 @@ static void adjust_hpsa_scsi_table(struct ctlr_info 
>>> *h, int hostno,
>>>     unsigned long flags;
>>>     struct hpsa_scsi_dev_t **added, **removed;
>>>     int nadded, nremoved;
>>> -   struct Scsi_Host *sh = NULL;
>>> 
>>>     /*
>>>      * A reset can cause a device status to change
>>> @@ -1792,46 +1822,29 @@ static void adjust_hpsa_scsi_table(struct ctlr_info 
>>> *h, int hostno,
>>>     if (hostno == -1 || !changes)
>>>             goto free_and_out;
>>> 
>>> -   sh = h->scsi_host;
>>> -   if (sh == NULL) {
>>> -           dev_warn(&h->pdev->dev, "%s: scsi_host is null\n", __func__);
>>> -           return;
>>> -   }
>> Are we guaranteed that h->scsi_host will never be NULL when running in here? 
>> Or when
>> the newly introduced hpsa_remove_device() is invoked elsewhere for that 
>> matter?
>> 
>> This commit loses this check and scsi_device_lookup() is not tolerant of a 
>> NULL
>> scsi_host * (first action is to grab the host_lock).
> Better to be safe. I'll add a check in both functions.

With these changes

Reviewed-by: Matthew R. Ochs <mro...@linux.vnet.ibm.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to