On 10/29/2015 03:59 PM, Matthew R. Ochs wrote:
On Oct 29, 2015, at 3:51 PM, Don Brace <brace77...@gmail.com> wrote:
On 10/29/2015 03:20 PM, Matthew R. Ochs wrote:
On Oct 28, 2015, at 5:06 PM, Don Brace <don.br...@pmcs.com>
  wrote:

From: Scott Teel
<scott.t...@pmcs.com>


There are problems with getting configuration change notification
in pass-through RAID environments.  So, activate flag
h->discovery_polling when one of these devices is detected in
update_scsi_devices.

After discovery_polling is set, execute a report luns from
rescan_controller_worker (every 30 seconds).

If the data from report_luns is different than last
time (binary compare), execute a full rescan via update_scsi_devices.

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 |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/scsi/hpsa.h |    2 ++
2 files changed, 70 insertions(+)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 8d67648..e521acd 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -276,6 +276,7 @@ static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info 
*h,
static void hpsa_command_resubmit_worker(struct work_struct *work);
static u32 lockup_detected(struct ctlr_info *h);
static int detect_controller_lockup(struct ctlr_info *h);
+static int hpsa_luns_changed(struct ctlr_info *h);

static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev)
{
@@ -3904,6 +3905,18 @@ static void hpsa_update_scsi_devices(struct ctlr_info 
*h, int hostno)
                hpsa_update_device_supports_aborts(h, tmpdevice, lunaddrbytes);
                this_device = currentsd[ncurrent];

+               /* Turn on discovery_polling if there are ext target devices.
+                * Event-based change notification is unreliable for those.
+                */
+               if (!h->discovery_polling) {
+                       if (tmpdevice->external) {
+                               h->discovery_polling = 1;
+                               dev_info(&h->pdev->dev,
+                                       "External target, activate discovery 
polling.\n");
+                       }
+               }
+
+
                *this_device = *tmpdevice;
                this_device->physical_device = physical_device;

@@ -8022,6 +8035,41 @@ static int hpsa_offline_devices_ready(struct ctlr_info 
*h)
        return 0;
}

+static int hpsa_luns_changed(struct ctlr_info *h)
+{
+       int rc = 1; /* assume there are changes */
+       struct ReportLUNdata *logdev = NULL;
+
+       /* if we can't find out if lun data has changed,
+        * assume that it has.
+        */
+
+       if (!h->lastlogicals)
+               goto out;
+
+       logdev = kzalloc(sizeof(*logdev), GFP_KERNEL);
+       if (!logdev) {
+               dev_warn(&h->pdev->dev,
+                       "Out of memory, can't track lun changes.\n");
+               goto out;
+       }
+       if (hpsa_scsi_do_report_luns(h, 1, logdev, sizeof(*logdev), 0)) {
+               dev_warn(&h->pdev->dev,
+                       "report luns failed, can't track lun changes.\n");
+               goto out;
+       }
+       if (memcmp(logdev, h->lastlogicals, sizeof(*logdev))) {
+               dev_info(&h->pdev->dev,
+                       "Lun changes detected.\n");
+               memcpy(h->lastlogicals, logdev, sizeof(*logdev));
+               goto out;
+       } else
+               rc = 0; /* no changes detected. */
+out:
+       kfree(logdev);
+       return rc;
+}
+
static void hpsa_rescan_ctlr_worker(struct work_struct *work)
{
        unsigned long flags;
@@ -8037,6 +8085,18 @@ static void hpsa_rescan_ctlr_worker(struct work_struct 
*work)
                hpsa_ack_ctlr_events(h);
                hpsa_scan_start(h->scsi_host);
                scsi_host_put(h->scsi_host);
+       } else if (h->discovery_polling) {
+               if (hpsa_luns_changed(h)) {
+                       struct Scsi_Host *sh = NULL;
+
+                       dev_info(&h->pdev->dev,
+                               "driver discovery polling rescan.\n");
+                       sh = scsi_host_get(h->scsi_host);
+                       if (sh != NULL) {
+                               hpsa_scan_start(sh);
+                               scsi_host_put(sh);
+                       }
+               }
        }
        spin_lock_irqsave(&h->lock, flags);
        if (!h->remove_in_progress)
@@ -8277,6 +8337,8 @@ reinit_after_soft_reset:

        /* Enable Accelerated IO path at driver layer */
        h->acciopath_status = 1;
+       /* Disable discovery polling.*/
+       h->discovery_polling = 0;


        /* Turn the interrupts on so we can service requests */
@@ -8284,6 +8346,11 @@ reinit_after_soft_reset:

        hpsa_hba_inquiry(h);

+       h->lastlogicals = kzalloc(sizeof(*(h->lastlogicals)), GFP_KERNEL);
+       if (!h->lastlogicals)
+               dev_info(&h->pdev->dev,
+                       "Can't track change to report lun data\n");
+
        /* Monitor the controller for firmware lockups */
        h->heartbeat_sample_interval = HEARTBEAT_SAMPLE_INTERVAL;
        INIT_DELAYED_WORK(&h->monitor_ctlr_work, hpsa_monitor_ctlr_worker);
@@ -8368,6 +8435,7 @@ static void hpsa_shutdown(struct pci_dev *pdev)
        hpsa_flush_cache(h);
        h->access.set_intr_mask(h, HPSA_INTR_OFF);
        hpsa_free_irqs(h);                      /* init_one 4 */
+       kfree(h->lastlogicals);

Is this the best place to free this memory? If your rescan worker is running
concurrently you might run into trouble.

Since hpsa_shutdown is called from hpsa_remove_one, at a point after
cancel_delayed_work_sync(&h->rescan_ctlr_work) has already been called,
I  think that the rescan worker won’t be running at this point.
My concern wasn't about the remove path but rather the shutdown notification 
path.
]
You are correct. I moved this to hpsa_remove_one.
Thanks


--
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