quoted

@@ -612,7 +1942,25 @@ static void mpi3mr_target_destroy(struct

scsi_target *starget)

quoted

  */
 static int mpi3mr_slave_configure(struct scsi_device *sdev)  {
+	struct scsi_target *starget;
+	struct Scsi_Host *shost;
+	struct mpi3mr_ioc *mrioc;
+	struct mpi3mr_tgt_dev *tgt_dev;
+	unsigned long flags;
 	int retval = 0;
+
+	starget = scsi_target(sdev);
+	shost = dev_to_shost(&starget->dev);
+	mrioc = shost_priv(shost);
+
+	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
+	tgt_dev = __mpi3mr_get_tgtdev_by_perst_id(mrioc, starget->id);
+	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
+	if (!tgt_dev)
+		return retval;
+

Return '0' on unknown SCSI devices? Really?

Hannes  - I will fix this in V4. I am planning to send V4 today. Please
review.

quoted

+	mpi3mr_tgtdev_put(tgt_dev);
+
 	return retval;
 }

@@ -626,7 +1974,37 @@ static int mpi3mr_slave_configure(struct

scsi_device *sdev)

quoted

  */
 static int mpi3mr_slave_alloc(struct scsi_device *sdev)  {
+	struct Scsi_Host *shost;
+	struct mpi3mr_ioc *mrioc;
+	struct mpi3mr_stgt_priv_data *scsi_tgt_priv_data;
+	struct mpi3mr_tgt_dev *tgt_dev;
+	struct mpi3mr_sdev_priv_data *scsi_dev_priv_data;
+	unsigned long flags;
+	struct scsi_target *starget;
 	int retval = 0;
+
+	starget = scsi_target(sdev);
+	shost = dev_to_shost(&starget->dev);
+	mrioc = shost_priv(shost);
+	scsi_tgt_priv_data = starget->hostdata;
+
+	scsi_dev_priv_data = kzalloc(sizeof(*scsi_dev_priv_data),

GFP_KERNEL);

quoted

+	if (!scsi_dev_priv_data)
+		return -ENOMEM;
+
+	scsi_dev_priv_data->lun_id = sdev->lun;
+	scsi_dev_priv_data->tgt_priv_data = scsi_tgt_priv_data;
+	sdev->hostdata = scsi_dev_priv_data;
+
+	scsi_tgt_priv_data->num_luns++;
+
+	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
+	tgt_dev = __mpi3mr_get_tgtdev_by_perst_id(mrioc, starget->id);
+	if (tgt_dev && (tgt_dev->starget == NULL))
+		tgt_dev->starget = starget;
+	if (tgt_dev)
+		mpi3mr_tgtdev_put(tgt_dev);
+	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
 	return retval;
 }

Same here. I would have expected -ENXIO to be returned fi the tgt_dev is
not
found.
And you can fold the two 'if' clauses into one eg like:

This is fixed in  in V4.

if (tgt_dev) {
  if (tgt_dev->starget == NULL)
    tgt_dev = starget;
  mpi3mr_tgtdev_put(tgt_dev);
  retval = 0;
}

quoted

@@ -640,7 +2018,33 @@ static int mpi3mr_slave_alloc(struct scsi_device

*sdev)

quoted

  */
 static int mpi3mr_target_alloc(struct scsi_target *starget)  {
+	struct Scsi_Host *shost = dev_to_shost(&starget->dev);
+	struct mpi3mr_ioc *mrioc = shost_priv(shost);
+	struct mpi3mr_stgt_priv_data *scsi_tgt_priv_data;
+	struct mpi3mr_tgt_dev *tgt_dev;
+	unsigned long flags;
 	int retval = -ENODEV;
+
+	scsi_tgt_priv_data = kzalloc(sizeof(*scsi_tgt_priv_data),

GFP_KERNEL);

quoted

+	if (!scsi_tgt_priv_data)
+		return -ENOMEM;
+
+	starget->hostdata = scsi_tgt_priv_data;
+	scsi_tgt_priv_data->starget = starget;
+	scsi_tgt_priv_data->dev_handle = MPI3MR_INVALID_DEV_HANDLE;
+
+	spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
+	tgt_dev = __mpi3mr_get_tgtdev_by_perst_id(mrioc, starget->id);
+	if (tgt_dev && !tgt_dev->is_hidden) {
+		scsi_tgt_priv_data->dev_handle = tgt_dev->dev_handle;
+		scsi_tgt_priv_data->perst_id = tgt_dev->perst_id;
+		scsi_tgt_priv_data->dev_type = tgt_dev->dev_type;
+		scsi_tgt_priv_data->tgt_dev = tgt_dev;
+		tgt_dev->starget = starget;
+		atomic_set(&scsi_tgt_priv_data->block_io, 0);
+		retval = 0;
+	}
+	spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
 	return retval;
 }

Ah, here is the correct value set.
(But wasn't it ENXIO which should've been returned for unknown targets?)

This is fixed in V4.

quoted

@@ -836,7 +2240,7 @@ mpi3mr_probe(struct pci_dev *pdev, const struct

pci_device_id *id)  {
 	struct mpi3mr_ioc *mrioc = NULL;
 	struct Scsi_Host *shost = NULL;
-	int retval = 0;
+	int retval = 0, i;

 	shost = scsi_host_alloc(&mpi3mr_driver_template,
 	    sizeof(struct mpi3mr_ioc));

@@ -857,11 +2261,21 @@ mpi3mr_probe(struct pci_dev *pdev, const

struct pci_device_id *id)

quoted

 	spin_lock_init(&mrioc->admin_req_lock);
 	spin_lock_init(&mrioc->reply_free_queue_lock);
 	spin_lock_init(&mrioc->sbq_lock);
+	spin_lock_init(&mrioc->fwevt_lock);
+	spin_lock_init(&mrioc->tgtdev_lock);
 	spin_lock_init(&mrioc->watchdog_lock);
 	spin_lock_init(&mrioc->chain_buf_lock);

+	INIT_LIST_HEAD(&mrioc->fwevt_list);
+	INIT_LIST_HEAD(&mrioc->tgtdev_list);
+	INIT_LIST_HEAD(&mrioc->delayed_rmhs_list);
+
 	mpi3mr_init_drv_cmd(&mrioc->init_cmds,

MPI3MR_HOSTTAG_INITCMDS);

quoted

+	for (i = 0; i < MPI3MR_NUM_DEVRMCMD; i++)
+		mpi3mr_init_drv_cmd(&mrioc->dev_rmhs_cmds[i],
+		    MPI3MR_HOSTTAG_DEVRMCMD_MIN + i);
+
 	if (pdev->revision)
 		mrioc->enable_segqueue = true;

@@ -877,6 +2291,17 @@ mpi3mr_probe(struct pci_dev *pdev, const struct

pci_device_id *id)

quoted

 	shost->max_channel = 1;
 	shost->max_id = 0xFFFFFFFF;

+	snprintf(mrioc->fwevt_worker_name, sizeof(mrioc-
fwevt_worker_name),
+	    "%s%d_fwevt_wrkr", mrioc->driver_name, mrioc->id);
+	mrioc->fwevt_worker_thread = alloc_ordered_workqueue(
+	    mrioc->fwevt_worker_name, WQ_MEM_RECLAIM);
+	if (!mrioc->fwevt_worker_thread) {
+		ioc_err(mrioc, "failure at %s:%d/%s()!\n",
+		    __FILE__, __LINE__, __func__);
+		retval = -ENODEV;
+		goto out_fwevtthread_failed;
+	}
+
 	mrioc->is_driver_loading = 1;
 	if (mpi3mr_init_ioc(mrioc)) {
 		ioc_err(mrioc, "failure at %s:%d/%s()!\n", @@ -903,6 +2328,8

@@

quoted

mpi3mr_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 addhost_failed:
 	mpi3mr_cleanup_ioc(mrioc);
 out_iocinit_failed:
+	destroy_workqueue(mrioc->fwevt_worker_thread);
+out_fwevtthread_failed:
 	spin_lock(&mrioc_list_lock);
 	list_del(&mrioc->list);
 	spin_unlock(&mrioc_list_lock);

@@ -924,14 +2351,30 @@ static void mpi3mr_remove(struct pci_dev

*pdev)

quoted

{
 	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 	struct mpi3mr_ioc *mrioc;
+	struct workqueue_struct	*wq;
+	unsigned long flags;
+	struct mpi3mr_tgt_dev *tgtdev, *tgtdev_next;

 	mrioc = shost_priv(shost);
 	while (mrioc->reset_in_progress || mrioc->is_driver_loading)
 		ssleep(1);

 	mrioc->stop_drv_processing = 1;
+	mpi3mr_cleanup_fwevt_list(mrioc);
+	spin_lock_irqsave(&mrioc->fwevt_lock, flags);
+	wq = mrioc->fwevt_worker_thread;
+	mrioc->fwevt_worker_thread = NULL;
+	spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
+	if (wq)
+		destroy_workqueue(wq);

 	scsi_remove_host(shost);
+	list_for_each_entry_safe(tgtdev, tgtdev_next, &mrioc->tgtdev_list,
+	    list) {
+		mpi3mr_remove_tgtdev_from_host(mrioc, tgtdev);
+		mpi3mr_tgtdev_del_from_list(mrioc, tgtdev);
+		mpi3mr_tgtdev_put(tgtdev);
+	}

 	mpi3mr_cleanup_ioc(mrioc);

@@ -955,6 +2398,8 @@ static void mpi3mr_shutdown(struct pci_dev

*pdev)

quoted

{
 	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 	struct mpi3mr_ioc *mrioc;
+	struct workqueue_struct	*wq;
+	unsigned long flags;

 	if (!shost)
 		return;

@@ -963,6 +2408,13 @@ static void mpi3mr_shutdown(struct pci_dev

*pdev)

quoted

 	while (mrioc->reset_in_progress || mrioc->is_driver_loading)
 		ssleep(1);
 	mrioc->stop_drv_processing = 1;
+	mpi3mr_cleanup_fwevt_list(mrioc);
+	spin_lock_irqsave(&mrioc->fwevt_lock, flags);
+	wq = mrioc->fwevt_worker_thread;
+	mrioc->fwevt_worker_thread = NULL;
+	spin_unlock_irqrestore(&mrioc->fwevt_lock, flags);
+	if (wq)
+		destroy_workqueue(wq);

 	mpi3mr_cleanup_ioc(mrioc);

Cheers,

Hannes
--
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)