aacraid: Patchset for aacraid driver version 41052

From: Raghava Aditya Renukunta <[email protected]>

The method to allocate and free FIB's in the present code utilizes spinlocks.Multiple IO's have to wait on the spinlock to acquire or free fibs creating a performance bottleneck.

An alternative solution would be to use block layer tags to keep track of the fibs allocated and freed. To this end 2 functions aac_fib_alloc_tag and aac_fib_free_tag were created which utilize the blk layer tags to plug into the Fib pool.These functions are used exclusively in the IO path. 8 fibs are reserved for the use of AIF management software and utilize the previous spinlock based implementations.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aachba.c  | 35 +++++++++++++++++------------- drivers/scsi/aacraid/aacraid.h |  2 ++ drivers/scsi/aacraid/commsup.c | 49 +++++++++++++++++++++++++++++++++++++++- --
 drivers/scsi/aacraid/dpcsup.c  |  4 ++-- drivers/scsi/aacraid/linit.c   |  2 ++

 5 files changed, 72 insertions(+), 20 deletions(-)

drivers/scsi/aacraid/aachba.c +0 -0

index e4c2437..06cbab8 100644
@@ -323,7 +323,7 @@ static inline int aac_valid_context(struct scsi_cmnd 

drivers/scsi/aacraid/aachba.c +29 -0

   *	@dev: Adapter to allocate the fib for
   *
+ *	Allocate a fib from the adapter fib pool using tags
+ *	from the blk layer.
+ */
+
+struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd)
+{
+	struct fib *fibptr;
+
+	fibptr = &dev->fibs[scmd->request->tag];
+	/*
+	 *	Set the proper node type code and node byte size
+	 */
+	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
+	fibptr->size = sizeof(struct fib);
+	/*
+	 *	Null out fields that depend on being zero at the start of
+	 *	each I/O
+	 */
+	fibptr->hw_fib_va->header.XferState = 0;
+	fibptr->flags = 0;
+	fibptr->callback = NULL;
+	fibptr->callback_data = NULL;
+
+	return fibptr;
+}
+
+/**
+ *	aac_fib_free_tag	free a fib
+ *	@fibptr: fib to free up

From: Raghava Aditya Renukunta <[email protected]>

The method to allocate and free FIB's in the present code utilizes spinlocks.Multiple IO's have to wait on the spinlock to acquire or free fibs creating a performance bottleneck.

An alternative solution would be to use block layer tags to keep track of the fibs allocated and freed. To this end 2 functions aac_fib_alloc_tag and aac_fib_free_tag were created which utilize the blk layer tags to plug into the Fib pool.These functions are used exclusively in the IO path. 8 fibs are reserved for the use of AIF management software and utilize the previous spinlock based implementations.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aachba.c  | 35 +++++++++++++++++-------------
 drivers/scsi/aacraid/aacraid.h |  2 ++
 drivers/scsi/aacraid/commsup.c | 49 +++++++++++++++++++++++++++++++++++++++---
 drivers/scsi/aacraid/dpcsup.c  |  4 ++--
 drivers/scsi/aacraid/linit.c   |  2 ++
 5 files changed, 72 insertions(+), 20 deletions(-)

drivers/scsi/aacraid/aachba.c +20 -15

index e4c2437..06cbab8 100644
@@ -323,7 +323,7 @@ static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
 	if (unlikely(!scsicmd || !scsicmd->scsi_done)) {
 		dprintk((KERN_WARNING "aac_valid_context: scsi command corrupt\n"));
 		aac_fib_complete(fibptr);
-		aac_fib_free(fibptr);
+		aac_fib_free_tag(fibptr);
 		return 0;
 	}
 	scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;

@@ -331,7 +331,7 @@ static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
 	if (unlikely(!device || !scsi_device_online(device))) {
 		dprintk((KERN_WARNING "aac_valid_context: scsi device corrupt\n"));
 		aac_fib_complete(fibptr);
-		aac_fib_free(fibptr);
+		aac_fib_free_tag(fibptr);
 		return 0;
 	}
 	return 1;

@@ -541,7 +541,7 @@ static void get_container_name_callback(void *context, struct fib * fibptr)
 	scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
 
 	aac_fib_complete(fibptr);
-	aac_fib_free(fibptr);
+	aac_fib_free_tag(fibptr);
 	scsicmd->scsi_done(scsicmd);
 }
 

@@ -557,7 +557,8 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
 
 	dev = (struct aac_dev *)scsicmd->device->host->hostdata;
 
-	if (!(cmd_fibcontext = aac_fib_alloc(dev)))
+	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+	if (!cmd_fibcontext)
 		return -ENOMEM;
 
 	aac_fib_init(cmd_fibcontext);

@@ -586,7 +587,7 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
 
 	printk(KERN_WARNING "aac_get_container_name: aac_fib_send failed with status: %d.\n", status);
 	aac_fib_complete(cmd_fibcontext);
-	aac_fib_free(cmd_fibcontext);
+	aac_fib_free_tag(cmd_fibcontext);
 	return -1;
 }
 

@@ -1024,7 +1025,7 @@ static void get_container_serial_callback(void *context, struct fib * fibptr)
 	scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8 | SAM_STAT_GOOD;
 
 	aac_fib_complete(fibptr);
-	aac_fib_free(fibptr);
+	aac_fib_free_tag(fibptr);
 	scsicmd->scsi_done(scsicmd);
 }
 

@@ -1040,7 +1041,8 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
 
 	dev = (struct aac_dev *)scsicmd->device->host->hostdata;
 
-	if (!(cmd_fibcontext = aac_fib_alloc(dev)))
+	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+	if (!cmd_fibcontext)
 		return -ENOMEM;
 
 	aac_fib_init(cmd_fibcontext);

@@ -1068,7 +1070,7 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
 
 	printk(KERN_WARNING "aac_get_container_serial: aac_fib_send failed with status: %d.\n", status);
 	aac_fib_complete(cmd_fibcontext);
-	aac_fib_free(cmd_fibcontext);
+	aac_fib_free_tag(cmd_fibcontext);
 	return -1;
 }
 

@@ -1869,7 +1871,7 @@ static void io_callback(void *context, struct fib * fibptr)
 		break;
 	}
 	aac_fib_complete(fibptr);
-	aac_fib_free(fibptr);
+	aac_fib_free_tag(fibptr);
 
 	scsicmd->scsi_done(scsicmd);
 }

@@ -1954,7 +1956,8 @@ static int aac_read(struct scsi_cmnd * scsicmd)
 	/*
 	 *	Alocate and initialize a Fib
 	 */
-	if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+	if (!cmd_fibcontext) {
 		printk(KERN_WARNING "aac_read: fib allocation failed\n");
 		return -1;
 	}

@@ -2051,7 +2054,8 @@ static int aac_write(struct scsi_cmnd * scsicmd)
 	/*
 	 *	Allocate and initialize a Fib then setup a BlockWrite command
 	 */
-	if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+	if (!cmd_fibcontext) {
 		/* FIB temporarily unavailable,not catastrophic failure */
 
 		/* scsicmd->result = DID_ERROR << 16;

@@ -2285,7 +2289,7 @@ static int aac_start_stop(struct scsi_cmnd *scsicmd)
 	/*
 	 *	Allocate and initialize a Fib
 	 */
-	cmd_fibcontext = aac_fib_alloc(aac);
+	cmd_fibcontext = aac_fib_alloc_tag(aac, scsicmd);
 	if (!cmd_fibcontext)
 		return SCSI_MLQUEUE_HOST_BUSY;
 

@@ -3157,7 +3161,7 @@ static void aac_srb_callback(void *context, struct fib * fibptr)
 	scsicmd->result |= le32_to_cpu(srbreply->scsi_status);
 
 	aac_fib_complete(fibptr);
-	aac_fib_free(fibptr);
+	aac_fib_free_tag(fibptr);
 	scsicmd->scsi_done(scsicmd);
 }
 

@@ -3187,9 +3191,10 @@ static int aac_send_srb_fib(struct scsi_cmnd* scsicmd)
 	/*
 	 *	Allocate and initialize a Fib then setup a BlockWrite command
 	 */
-	if (!(cmd_fibcontext = aac_fib_alloc(dev))) {
+	cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);
+	if (!cmd_fibcontext)
 		return -1;
-	}
+
 	status = aac_adapter_scsi(cmd_fibcontext, scsicmd);
 
 	/*

drivers/scsi/aacraid/aacraid.h +2 -0

index 074878b..da227e8 100644
@@ -2114,9 +2114,11 @@ int aac_acquire_irq(struct aac_dev *dev);
 void aac_free_irq(struct aac_dev *dev);
 const char *aac_driverinfo(struct Scsi_Host *);
 struct fib *aac_fib_alloc(struct aac_dev *dev);
+struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd *scmd);
 int aac_fib_setup(struct aac_dev *dev);
 void aac_fib_map_free(struct aac_dev *dev);
 void aac_fib_free(struct fib * context);
+void aac_fib_free_tag(struct fib *context);
 void aac_fib_init(struct fib * context);
 void aac_printf(struct aac_dev *dev, u32 val);
 int aac_fib_send(u16 command, struct fib * context, unsigned long size, int priority, int wait, int reply, fib_callback callback, void *ctxt);

drivers/scsi/aacraid/commsup.c +5 -3

index a1f90fe..b5b653c 100644
@@ -156,9 +156,9 @@ int aac_fib_setup(struct aac_dev * dev)
 	 */
 	dev->fibs[dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1].next = NULL;
 	/*
-	 *	Enable this to debug out of queue space
-	 */
-	dev->free_fib = &dev->fibs[0];
+	*	Set 8 fibs aside for management tools
+	*/
+	dev->free_fib = &dev->fibs[dev->scsi_host_ptr->can_queue];
 	return 0;
 }
 

@@ -166,6 +166,49 @@ int aac_fib_setup(struct aac_dev * dev)
  *	aac_fib_alloc	-	allocate a fib
  *	@dev: Adapter to allocate the fib for
  *
+ *	Allocate a fib from the adapter fib pool using tags
+ *	from the blk layer.

drivers/scsi/aacraid/commsup.c +8 -0

+	fibptr->callback = NULL;
+	fibptr->callback_data = NULL;
+
+	return fibptr;
+}
+
+/**
+ *	aac_fib_free_tag	free a fib

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, December 3, 2015 7:52 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 01/10] aacraid: SCSI blk tag support

Quoted 14 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

The method to allocate and free FIB's in the present code utilizes spinlocks.Multiple IO's have to wait on the spinlock to acquire or free fibs creating a performance bottleneck.

An alternative solution would be to use block layer tags to keep track of the fibs allocated and freed. To this end 2 functions aac_fib_alloc_tag and aac_fib_free_tag were created which utilize the blk layer tags to plug into the Fib pool.These functions are used exclusively in the IO path. 8 fibs are reserved for the use of AIF management software and utilize the previous spinlock based implementations.

drivers/scsi/aacraid/commsup.c int aac_fib_setup(struct aac_dev * dev) · line 173

Quoted 10 lines

drivers/scsi/aacraid/commsup.c +5 -0

@@ -166,6 +166,49 @@ int aac_fib_setup(struct aac_dev * dev)
  *	aac_fib_alloc	-	allocate a fib
  *	@dev: Adapter to allocate the fib for
  *
+ *	Allocate a fib from the adapter fib pool using tags
+ *	from the blk layer.
+ */
+
+struct fib *aac_fib_alloc_tag(struct aac_dev *dev, struct scsi_cmnd

*scmd)

drivers/scsi/aacraid/commsup.c int aac_fib_setup(struct aac_dev * dev) · line 180

Quoted 7 lines

drivers/scsi/aacraid/commsup.c +7 -0

+{
+	struct fib *fibptr;
+
+	fibptr = &dev->fibs[scmd->request->tag];
+	/*
+	 *	Set the proper node type code and node byte size
+	 */

Can't you just make most of the initialisation below just once when the driver starts?

drivers/scsi/aacraid/commsup.c int aac_fib_setup(struct aac_dev * dev) · line 188

Quoted 8 lines

drivers/scsi/aacraid/commsup.c +8 -0

+	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
+	fibptr->size = sizeof(struct fib);
+	/*
+	 *	Null out fields that depend on being zero at the start of
+	 *	each I/O
+	 */
+	fibptr->hw_fib_va->header.XferState = 0;
+	fibptr->flags = 0;

For example the flags field is initialised here to '0', then
in aac_fib_send again to zero and later is 'fibptr->flags = FIB_CONTEXT_FLAG;'
Removing part of it would help I think.
The code here is not new so, if I am right, please plan it for a next series.

Quoted 14 lines

drivers/scsi/aacraid/commsup.c +13 -0

+	fibptr->callback = NULL;
+	fibptr->callback_data = NULL;
+
+	return fibptr;
+}
+
+/**
+ *	aac_fib_free_tag	free a fib
+ *	@fibptr: fib to free up
+ *
+ *	Placeholder to free tag allocated fibs
+ *	Does not do anything
+ */

drivers/scsi/aacraid/commsup.c int aac_fib_setup(struct aac_dev * dev) · line 206

Quoted 5 lines

drivers/scsi/aacraid/commsup.c +5 -0

+
+void aac_fib_free_tag(struct fib *fibptr)
+{
+	(void)fibptr;
+}

I agree with Johannes, I also don't like the aac_fib_free_tag as it is. With the aac_fib_free_tag you may add -

Reviewed-by Tomas Henzl <[email protected]>

From: Raghava Aditya Renukunta <[email protected]>

The driver utilizes an array of atomic variables to keep track of IO submissions to each vector. To submit an IO multiple threads iterate through the array to find a vector which has empty slots to send an IO. The reading and updating of the variable is not atomic, causing race conditions when a thread uses a full vector to submit an IO.

Fixed by mapping each FIB to a vector, the submission path then uses said vector to submit IO thereby removing the possibly of a race condition.The vector assignment is started from 1 since vector 0 is reserved for the use of AIF management FIBS.If the number of MSIx vectors is 1 (MSI or INTx mode) then all the fibs are allocated to vector 0.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aacraid.h |  1 + drivers/scsi/aacraid/commsup.c | 12 ++++++++++++ drivers/scsi/aacraid/src.c     | 30 +++++++-----------------------

 3 files changed, 20 insertions(+), 23 deletions(-)

drivers/scsi/aacraid/aacraid.h +1 -0

index da227e8..d133c4a 100644
@@ -944,6 +944,7 @@ struct fib {
  	 */
  	struct list_head	fiblink;
  	void			*data;
+	u32			vector_no;

  struct hw_fib *hw_fib_va; /* Actual
shared object */
  dma_addr_t hw_fib_pa; /* physical
address of hw_fib*/
 };

drivers/scsi/aacraid/commsup.c +1 -0

index b5b653c..b257d3b 100644
@@ -104,6 +104,7 @@ int aac_fib_setup(struct aac_dev * dev)
  	struct hw_fib *hw_fib;
  	dma_addr_t hw_fib_pa;
  	int i;
+	u32 vector = 1;

  while (((i = fib_map_alloc(dev)) == -ENOMEM)
   && (dev->scsi_host_ptr->can_queue > (64 - AAC_NUM_MGT_FIB))) {

drivers/scsi/aacraid/commsup.c +0 -0

@@ -150,6 +151,17 @@ int aac_fib_setup(struct aac_dev * dev)

  dev->max_fib_size + sizeof(struct aac_fib_xporthdr));
  hw_fib_pa = hw_fib_pa +
  dev->max_fib_size + sizeof(struct aac_fib_xporthdr);

drivers/scsi/aacraid/commsup.c +3 -0

+
+		if ((dev->max_msix == 1) ||
+		  (i > ((dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB -

1)

drivers/scsi/aacraid/commsup.c +8 -0

+			- dev->vector_cap))) {
+			fibptr->vector_no = 0;
+		} else {
+			fibptr->vector_no = vector;
+			vector++;
+			if (vector == dev->max_msix)
+				vector = 1;
+		}

  }
  /*
   * Add the fib chain to the free list

drivers/scsi/aacraid/src.c +0 -0

index 2aa34ea..bc0203f 100644
@@ -156,8 +156,8 @@ static irqreturn_t aac_src_intr_message(int irq, void
 *dev_id)
  				break;
  			if (dev->msi_enabled && dev->max_msix > 1)

  atomic_dec(&dev-

Quoted 1 line

rrq_outstanding[vector_no]);

drivers/scsi/aacraid/src.c +0 -1

-			aac_intr_normal(dev, handle-1, 0, isFastResponse,

NULL);
  dev->host_rrq[index++] = 0;

drivers/scsi/aacraid/src.c +1 -0

+			aac_intr_normal(dev, handle-1, 0, isFastResponse,

NULL);
  if (index == (vector_no + 1) * dev->vector_cap) index = vector_no * dev->vector_cap; dev->host_rrq_idx[vector_no] = index;

drivers/scsi/aacraid/src.c +0 -0

@@ -452,36 +452,20 @@ static int aac_src_deliver_message(struct fib *fib)
  #endif

  u16 hdr_size = le16_to_cpu(fib->hw_fib_va->header.Size);

drivers/scsi/aacraid/src.c +1 -0

+	u16 vector_no;

  atomic_inc(&q->numpending);

  if (dev->msi_enabled && fib->hw_fib_va->header.Command != AifRequest &&
      dev->max_msix > 1) {

drivers/scsi/aacraid/src.c +1 -22

-		u_int16_t vector_no, first_choice = 0xffff;
-
-		vector_no = dev->fibs_pushed_no % dev->max_msix;
-		do {
-			vector_no += 1;
-			if (vector_no == dev->max_msix)
-				vector_no = 1;
-			if (atomic_read(&dev->rrq_outstanding[vector_no]) <
-			    dev->vector_cap)
-				break;
-			if (0xffff == first_choice)
-				first_choice = vector_no;
-			else if (vector_no == first_choice)
-				break;
-		} while (1);
-		if (vector_no == first_choice)
-			vector_no = 0;
-		atomic_inc(&dev->rrq_outstanding[vector_no]);
-		if (dev->fibs_pushed_no == 0xffffffff)
-			dev->fibs_pushed_no = 0;
-		else
-			dev->fibs_pushed_no++;
+		vector_no = fib->vector_no;

  fib->hw_fib_va->header.Handle += (vector_no << 16);

drivers/scsi/aacraid/src.c +2 -0

+	} else {
+		vector_no = 0;

  }

drivers/scsi/aacraid/src.c +2 -0

+	atomic_inc(&dev->rrq_outstanding[vector_no]);
+

  if (dev->comm_interface == AAC_COMM_MESSAGE_TYPE2) { /* Calculate the amount to the fibsize bits */ fibsize = (hdr_size + 127) / 128 - 1;

From: Raghava Aditya Renukunta <[email protected]>

The driver utilizes an array of atomic variables to keep track of IO submissions to each vector. To submit an IO multiple threads iterate through the array to find a vector which has empty slots to send an IO. The reading and updating of the variable is not atomic, causing race conditions when a thread uses a full vector to submit an IO.

Fixed by mapping each FIB to a vector, the submission path then uses said vector to submit IO thereby removing the possibly of a race condition.The vector assignment is started from 1 since vector 0 is reserved for the use of AIF management FIBS.If the number of MSIx vectors is 1 (MSI or INTx mode) then all the fibs are allocated to vector 0.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

From: Raghava Aditya Renukunta <[email protected]>

Added support for PCI EEH(extended error handling).

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aacraid.h |   1 +
 drivers/scsi/aacraid/linit.c   | 138
+++++++++++++++++++++++++++++++++++++++++

 2 files changed, 139 insertions(+)

drivers/scsi/aacraid/aacraid.h +0 -0

index d133c4a..594de5f 100644
@@ -1235,6 +1235,7 @@ struct aac_dev
  	struct msix_entry	msixentry[AAC_MAX_MSIX];

  struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */
  u8 adapter_shutdown;

drivers/scsi/aacraid/aacraid.h +1 -0

+	u32			handle_pci_error;

 };

 #define aac_adapter_interrupt(dev) \

drivers/scsi/aacraid/linit.c +1 -0

index fa0fc44..0147210 100644
@@ -38,6 +38,7 @@
  #include <linux/module.h>
  #include <linux/moduleparam.h>
  #include <linux/pci.h>
+#include <linux/aer.h>
  #include <linux/pci-aspm.h>
  #include <linux/slab.h>
  #include <linux/mutex.h>

@@ -1298,6 +1299,9 @@ static int aac_probe_one(struct pci_dev *pdev, const
 struct pci_device_id *id)
  		goto out_deinit;
  	scsi_scan_host(shost);

drivers/scsi/aacraid/linit.c +3 -0

+	pci_enable_pcie_error_reporting(pdev);
+	pci_save_state(pdev);
+

  return 0;

  out_deinit:

drivers/scsi/aacraid/linit.c +0 -0

@@ -1501,6 +1505,139 @@ static void aac_remove_one(struct pci_dev *pdev)
  	}
  }

drivers/scsi/aacraid/linit.c +133 -0

+void aac_flush_ios(struct aac_dev *aac)
+{
+	int i;
+	struct scsi_cmnd *cmd;
+
+	for (i = 0; i < aac->scsi_host_ptr->can_queue; i++) {
+		cmd = (struct scsi_cmnd *)aac->fibs[i].callback_data;
+		if (cmd && (cmd->SCp.phase == AAC_OWNER_FIRMWARE)) {
+			scsi_dma_unmap(cmd);
+			aac_fib_free_tag(&aac->fibs[i]);
+
+			if (aac->handle_pci_error)
+				cmd->result = DID_NO_CONNECT << 16;
+			else
+				cmd->result = DID_RESET << 16;
+
+			cmd->scsi_done(cmd);
+		}
+	}
+}
+
+pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
+					enum pci_channel_state error)
+{
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct aac_dev *aac = shost_priv(shost);
+
+	dev_err(&pdev->dev, "aacraid: PCI error detected %x\n", error);
+
+	switch (error) {
+	case pci_channel_io_normal:
+		return PCI_ERS_RESULT_CAN_RECOVER;
+	case pci_channel_io_frozen:
+
+		aac->handle_pci_error = 1;
+
+		scsi_block_requests(aac->scsi_host_ptr);
+		aac_flush_ios(aac);
+		aac_release_resources(aac);
+
+		pci_disable_pcie_error_reporting(pdev);
+		aac_adapter_ioremap(aac, 0);
+
+		return PCI_ERS_RESULT_NEED_RESET;
+	case pci_channel_io_perm_failure:
+		aac->handle_pci_error = 1;
+		aac_flush_ios(aac);
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+pci_ers_result_t aac_pci_mmio_enabled(struct pci_dev *pdev)
+{
+	dev_err(&pdev->dev, "aacraid: PCI error - mmio enabled\n");
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+pci_ers_result_t aac_pci_slot_reset(struct pci_dev *pdev)
+{
+	dev_err(&pdev->dev, "aacraid: PCI error - slot reset\n");
+	pci_restore_state(pdev);
+	if (pci_enable_device(pdev)) {
+		dev_warn(&pdev->dev,
+			"aacraid: failed to enable slave\n");
+		goto fail_device;
+	}
+
+	pci_set_master(pdev);
+
+	if (pci_enable_device_mem(pdev)) {
+		dev_err(&pdev->dev, "pci_enable_device_mem failed\n");
+		goto fail_device;
+	}
+
+	return PCI_ERS_RESULT_RECOVERED;
+
+fail_device:
+	dev_err(&pdev->dev, "aacraid: PCI error - slot reset failed\n");
+	return PCI_ERS_RESULT_DISCONNECT;
+}
+
+
+void aac_pci_resume(struct pci_dev *pdev)
+{
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct scsi_device *sdev = NULL;
+	struct aac_dev *aac = (struct aac_dev *)shost_priv(shost);
+
+	pci_cleanup_aer_uncorrect_error_status(pdev);
+
+	if (aac_adapter_ioremap(aac, aac->base_size)) {
+
+		dev_err(&pdev->dev, "aacraid: ioremap failed\n");
+		/* remap failed, go back ... */
+		aac->comm_interface = AAC_COMM_PRODUCER;
+		if (aac_adapter_ioremap(aac, AAC_MIN_FOOTPRINT_SIZE)) {
+			dev_warn(&pdev->dev,
+				"aacraid: unable to map adapter.\n");
+
+			return;
+		}
+	}
+
+	msleep(10000);
+
+	aac_acquire_resources(aac);
+
+	/*
+	 * reset this flag to unblock ioctl() as it was set
+	 * at aac_send_shutdown() to block ioctls from upperlayer
+	 */
+	aac->adapter_shutdown = 0;
+	aac->handle_pci_error = 0;
+
+	shost_for_each_device(sdev, shost)
+		if (sdev->sdev_state == SDEV_OFFLINE)
+			sdev->sdev_state = SDEV_RUNNING;
+	scsi_unblock_requests(aac->scsi_host_ptr);
+	scsi_scan_host(aac->scsi_host_ptr);
+	pci_save_state(pdev);
+
+	dev_err(&pdev->dev, "aacraid: PCI error - resume\n");
+}
+
+static struct pci_error_handlers aac_pci_err_handler = {
+	.error_detected		= aac_pci_error_detected,
+	.mmio_enabled		= aac_pci_mmio_enabled,
+	.slot_reset		= aac_pci_slot_reset,
+	.resume			= aac_pci_resume,
+};
+

 static struct pci_driver aac_pci_driver = {
  .name = AAC_DRIVERNAME,
  .id_table = aac_pci_tbl,

drivers/scsi/aacraid/linit.c +1 -0

@@ -1511,6 +1648,7 @@ static struct pci_driver aac_pci_driver = {
  	.resume		= aac_resume,
  #endif
  	.shutdown	= aac_shutdown,
+	.err_handler    = &aac_pci_err_handler,
  };

 static int __init aac_init(void)

-----Original Message-----
From: Johannes Thumshirn [mailto:[email protected]]
Sent: Wednesday, December 2, 2015 1:42 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 03/10] aacraid: Added EEH support

drivers/scsi/aacraid/linit.c static void aac_remove_one(struct pci_dev

Quoted 48 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

Added support for PCI EEH(extended error handling).

Signed-off-by: Raghava Aditya Renukunta
<[email protected]>
---
 drivers/scsi/aacraid/aacraid.h |   1 +
 drivers/scsi/aacraid/linit.c   | 138
+++++++++++++++++++++++++++++++++++++++++

 2 files changed, 139 insertions(+)

(quoted diff) +0 -0

b/drivers/scsi/aacraid/aacraid.h index d133c4a..594de5f 100644

drivers/scsi/aacraid/aacraid.h +0 -0

@@ -1235,6 +1235,7 @@ struct aac_dev
  	struct msix_entry	msixentry[AAC_MAX_MSIX];

  struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */
  u8 adapter_shutdown;

drivers/scsi/aacraid/aacraid.h +1 -0

+	u32			handle_pci_error;

 };

 #define aac_adapter_interrupt(dev) \

(quoted diff) +0 -0

b/drivers/scsi/aacraid/linit.c index fa0fc44..0147210 100644

drivers/scsi/aacraid/linit.c +4 -0

@@ -38,6 +38,7 @@
  #include <linux/module.h>
  #include <linux/moduleparam.h>
  #include <linux/pci.h>
+#include <linux/aer.h>
  #include <linux/pci-aspm.h>
  #include <linux/slab.h>
  #include <linux/mutex.h>

@@ -1298,6 +1299,9 @@ static int aac_probe_one(struct pci_dev *pdev,
 const struct pci_device_id *id)
  		goto out_deinit;
  	scsi_scan_host(shost);
 
+	pci_enable_pcie_error_reporting(pdev);
+	pci_save_state(pdev);
+
  	return 0;
 

  out_deinit:

drivers/scsi/aacraid/linit.c +0 -0

@@ -1501,6 +1505,139 @@ static void aac_remove_one(struct pci_dev

*pdev)

drivers/scsi/aacraid/linit.c static void aac_remove_one(struct pci_dev *pdev)

Quoted 10 lines

drivers/scsi/aacraid/linit.c +7 -0

  	}
  }
 
+void aac_flush_ios(struct aac_dev *aac) {
+	int i;
+	struct scsi_cmnd *cmd;
+
+	for (i = 0; i < aac->scsi_host_ptr->can_queue; i++) {
+		cmd = (struct scsi_cmnd *)aac->fibs[i].callback_data;
+		if (cmd && (cmd->SCp.phase == AAC_OWNER_FIRMWARE))

{

Quoted 131 lines

drivers/scsi/aacraid/linit.c +122 -0

+			scsi_dma_unmap(cmd);
+			aac_fib_free_tag(&aac->fibs[i]);
+
+			if (aac->handle_pci_error)
+				cmd->result = DID_NO_CONNECT << 16;
+			else
+				cmd->result = DID_RESET << 16;
+
+			cmd->scsi_done(cmd);
+		}
+	}
+}
+
+pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
+					enum pci_channel_state error)
+{
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct aac_dev *aac = shost_priv(shost);
+
+	dev_err(&pdev->dev, "aacraid: PCI error detected %x\n", error);
+
+	switch (error) {
+	case pci_channel_io_normal:
+		return PCI_ERS_RESULT_CAN_RECOVER;
+	case pci_channel_io_frozen:
+
+		aac->handle_pci_error = 1;
+
+		scsi_block_requests(aac->scsi_host_ptr);
+		aac_flush_ios(aac);
+		aac_release_resources(aac);
+
+		pci_disable_pcie_error_reporting(pdev);
+		aac_adapter_ioremap(aac, 0);
+
+		return PCI_ERS_RESULT_NEED_RESET;
+	case pci_channel_io_perm_failure:
+		aac->handle_pci_error = 1;
+		aac_flush_ios(aac);
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+pci_ers_result_t aac_pci_mmio_enabled(struct pci_dev *pdev) {
+	dev_err(&pdev->dev, "aacraid: PCI error - mmio enabled\n");
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+pci_ers_result_t aac_pci_slot_reset(struct pci_dev *pdev) {
+	dev_err(&pdev->dev, "aacraid: PCI error - slot reset\n");
+	pci_restore_state(pdev);
+	if (pci_enable_device(pdev)) {
+		dev_warn(&pdev->dev,
+			"aacraid: failed to enable slave\n");
+		goto fail_device;
+	}
+
+	pci_set_master(pdev);
+
+	if (pci_enable_device_mem(pdev)) {
+		dev_err(&pdev->dev, "pci_enable_device_mem failed\n");
+		goto fail_device;
+	}
+
+	return PCI_ERS_RESULT_RECOVERED;
+
+fail_device:
+	dev_err(&pdev->dev, "aacraid: PCI error - slot reset failed\n");
+	return PCI_ERS_RESULT_DISCONNECT;
+}
+
+
+void aac_pci_resume(struct pci_dev *pdev) {
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct scsi_device *sdev = NULL;
+	struct aac_dev *aac = (struct aac_dev *)shost_priv(shost);
+
+	pci_cleanup_aer_uncorrect_error_status(pdev);
+
+	if (aac_adapter_ioremap(aac, aac->base_size)) {
+
+		dev_err(&pdev->dev, "aacraid: ioremap failed\n");
+		/* remap failed, go back ... */
+		aac->comm_interface = AAC_COMM_PRODUCER;
+		if (aac_adapter_ioremap(aac, AAC_MIN_FOOTPRINT_SIZE)) {
+			dev_warn(&pdev->dev,
+				"aacraid: unable to map adapter.\n");
+
+			return;
+		}
+	}
+
+	msleep(10000);
+
+	aac_acquire_resources(aac);
+
+	/*
+	 * reset this flag to unblock ioctl() as it was set
+	 * at aac_send_shutdown() to block ioctls from upperlayer
+	 */
+	aac->adapter_shutdown = 0;
+	aac->handle_pci_error = 0;
+
+	shost_for_each_device(sdev, shost)
+		if (sdev->sdev_state == SDEV_OFFLINE)
+			sdev->sdev_state = SDEV_RUNNING;
+	scsi_unblock_requests(aac->scsi_host_ptr);
+	scsi_scan_host(aac->scsi_host_ptr);
+	pci_save_state(pdev);
+
+	dev_err(&pdev->dev, "aacraid: PCI error - resume\n"); }
+
+static struct pci_error_handlers aac_pci_err_handler = {
+	.error_detected		= aac_pci_error_detected,
+	.mmio_enabled		= aac_pci_mmio_enabled,
+	.slot_reset		= aac_pci_slot_reset,
+	.resume			= aac_pci_resume,
+};
+
  static struct pci_driver aac_pci_driver = {
  	.name		= AAC_DRIVERNAME,
  	.id_table	= aac_pci_tbl,

@@ -1511,6 +1648,7 @@ static struct pci_driver aac_pci_driver = {
  	.resume		= aac_resume,
  #endif
  	.shutdown	= aac_shutdown,
+	.err_handler    = &aac_pci_err_handler,
  };
 

drivers/scsi/aacraid/linit.c static struct pci_driver aac_pci_driver = { · line 1654

Quoted 1 line

drivers/scsi/aacraid/linit.c +0 -0

  static int __init aac_init(void)

I really think the new aac_flush_io() and pci error hanler methods should be static as they're nowhere used outside of this file's scope.

From: Raghava Aditya Renukunta <[email protected]>

Added support for PCI EEH(extended error handling).

Signed-off-by Raghava Aditya Renukunta <[email protected]>

From: Raghava Aditya Renukunta <[email protected]>

aac_fib_map_free() calls pci_free_consistent() without checking that dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they are indeed NULL/0, this will result in a hang as pci_free_consistent() will attempt to invalidate cache for the entire 64-bit address space (which would take a very long time).

Fixed by adding a check to make sure that dev->hw_fib_va and dev->max_fib_size are not NULL and 0 respectively.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/commsup.c | 9 ++++++---

 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/scsi/aacraid/commsup.c +0 -0

index b257d3b..9533f47 100644
@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)

 void aac_fib_map_free(struct aac_dev *dev)
 {

drivers/scsi/aacraid/commsup.c +0 -2

-	pci_free_consistent(dev->pdev,
-	  dev->max_fib_size * (dev->scsi_host_ptr->can_queue +

AAC_NUM_MGT_FIB),

drivers/scsi/aacraid/commsup.c +6 -1

-	  dev->hw_fib_va, dev->hw_fib_pa);
+	if (dev->hw_fib_va && dev->max_fib_size) {
+		pci_free_consistent(dev->pdev,
+		(dev->max_fib_size *
+		(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+		dev->hw_fib_va, dev->hw_fib_pa);
+	}

  dev->hw_fib_va = NULL;
  dev->hw_fib_pa = 0;
 }

From: Raghava Aditya Renukunta <[email protected]>

aac_fib_map_free() calls pci_free_consistent() without checking that dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they are indeed NULL/0, this will result in a hang as pci_free_consistent() will attempt to invalidate cache for the entire 64-bit address space (which would take a very long time).

Fixed by adding a check to make sure that dev->hw_fib_va and dev->max_fib_size are not NULL and 0 respectively.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/commsup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

drivers/scsi/aacraid/commsup.c +6 -3

index b257d3b..9533f47 100644
@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
 
 void aac_fib_map_free(struct aac_dev *dev)
 {
-	pci_free_consistent(dev->pdev,
-	  dev->max_fib_size * (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB),
-	  dev->hw_fib_va, dev->hw_fib_pa);
+	if (dev->hw_fib_va && dev->max_fib_size) {
+		pci_free_consistent(dev->pdev,
+		(dev->max_fib_size *
+		(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+		dev->hw_fib_va, dev->hw_fib_pa);
+	}
 	dev->hw_fib_va = NULL;
 	dev->hw_fib_pa = 0;
 }

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, December 4, 2015 6:35 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 04/10] aacraid: Fix memory leak in aac_fib_map_free

Quoted 8 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

aac_fib_map_free() calls pci_free_consistent() without checking that dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they are indeed NULL/0, this will result in a hang as pci_free_consistent() will attempt to invalidate cache for the entire 64-bit address space (which would take a very long time).

Quoted 4 lines

Fixed by adding a check to make sure that dev->hw_fib_va and dev->max_fib_size are not NULL and 0 respectively.

Signed-off-by Raghava Aditya Renukunta

<[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Is the can_queue constant during the driver's life, or is it possible to manipulate it (aac_change_queue_depth)?

Tomas

Review

Quoted 5 lines

---
 drivers/scsi/aacraid/commsup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/drivers/scsi/aacraid/commsup.c

drivers/scsi/aacraid/commsup.c static int fib_map_alloc(struct aac_dev *dev) · line 87

Quoted 9 lines

index b257d3b..9533f47 100644

drivers/scsi/aacraid/commsup.c +0 -2

@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
 
 void aac_fib_map_free(struct aac_dev *dev)
 {
-	pci_free_consistent(dev->pdev,
-	  dev->max_fib_size * (dev->scsi_host_ptr->can_queue +

AAC_NUM_MGT_FIB),

Quoted 10 lines

drivers/scsi/aacraid/commsup.c +6 -1

-	  dev->hw_fib_va, dev->hw_fib_pa);
+	if (dev->hw_fib_va && dev->max_fib_size) {
+		pci_free_consistent(dev->pdev,
+		(dev->max_fib_size *
+		(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+		dev->hw_fib_va, dev->hw_fib_pa);
+	}
 	dev->hw_fib_va = NULL;
 	dev->hw_fib_pa = 0;
 }

Hello Tomas,

Quoted 26 lines

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, December 4, 2015 6:35 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 04/10] aacraid: Fix memory leak in aac_fib_map_free

Quoted 8 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

aac_fib_map_free() calls pci_free_consistent() without checking that dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they are indeed NULL/0, this will result in a hang as pci_free_consistent() will attempt to invalidate cache for the entire 64-bit address space (which would take a very long time).

Quoted 4 lines

Fixed by adding a check to make sure that dev->hw_fib_va and dev->max_fib_size are not NULL and 0 respectively.

Signed-off-by Raghava Aditya Renukunta

<[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Quoted 4 lines

Is the can_queue constant during the driver's life, or is it possible to manipulate it (aac_change_queue_depth)?

Tomas

can_queue is only changed in aac_init_adapter. Do you want to save
(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) in a variable
So that the whole can_queue dereference need not be used?

Regards,
Raghava Aditya

Quoted 16 lines

Review

Quoted 5 lines

---
 drivers/scsi/aacraid/commsup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/drivers/scsi/aacraid/commsup.c

drivers/scsi/aacraid/commsup.c static int fib_map_alloc(struct aac_dev *dev) · line 87

Quoted 9 lines

index b257d3b..9533f47 100644

drivers/scsi/aacraid/commsup.c +0 -2

@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
 
 void aac_fib_map_free(struct aac_dev *dev)
 {
-	pci_free_consistent(dev->pdev,
-	  dev->max_fib_size * (dev->scsi_host_ptr->can_queue +

AAC_NUM_MGT_FIB),

Quoted 10 lines

drivers/scsi/aacraid/commsup.c static int fib_map_alloc(struct aac_dev *dev) · line 94

Quoted 10 lines

drivers/scsi/aacraid/commsup.c +6 -1

-	  dev->hw_fib_va, dev->hw_fib_pa);
+	if (dev->hw_fib_va && dev->max_fib_size) {
+		pci_free_consistent(dev->pdev,
+		(dev->max_fib_size *
+		(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+		dev->hw_fib_va, dev->hw_fib_pa);
+	}
 	dev->hw_fib_va = NULL;
 	dev->hw_fib_pa = 0;
 }

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

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Monday, December 7, 2015 6:06 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 04/10] aacraid: Fix memory leak in aac_fib_map_free

Quoted 14 lines · Raghava Aditya Renukunta

Hello Tomas,

Quoted 11 lines

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, December 4, 2015 6:35 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 04/10] aacraid: Fix memory leak in aac_fib_map_free

Quoted 1 line · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta

<[email protected]>

Quoted 19 lines

Quoted 19 lines

Quoted 11 lines

aac_fib_map_free() calls pci_free_consistent() without checking that dev->hw_fib_va is not NULL and dev->max_fib_size is not zero.If they are indeed NULL/0, this will result in a hang as pci_free_consistent() will attempt to invalidate cache for the entire 64-bit address space (which would take a very long time).

Fixed by adding a check to make sure that dev->hw_fib_va and dev->max_fib_size are not NULL and 0 respectively.

Signed-off-by Raghava Aditya Renukunta

<[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Is the can_queue constant during the driver's life, or is it possible to manipulate it (aac_change_queue_depth)?

Tomas

Quoted 3 lines

can_queue is only changed in aac_init_adapter. Do you want to save
(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB) in a variable
So that the whole can_queue dereference need not be used?

It's fine as it is, (I thought it may change elsewhere in your code but now I think that I was wrong).

--tm

Quoted 35 lines

Regards,
Raghava Aditya

Quoted 16 lines

Review

Quoted 5 lines

---
 drivers/scsi/aacraid/commsup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

(quoted diff) +0 -0

b/drivers/scsi/aacraid/commsup.c

drivers/scsi/aacraid/commsup.c static int fib_map_alloc(struct aac_dev *dev) · line 87

Quoted 9 lines

index b257d3b..9533f47 100644

drivers/scsi/aacraid/commsup.c +0 -2

@@ -83,9 +83,12 @@ static int fib_map_alloc(struct aac_dev *dev)
 
 void aac_fib_map_free(struct aac_dev *dev)
 {
-	pci_free_consistent(dev->pdev,
-	  dev->max_fib_size * (dev->scsi_host_ptr->can_queue +

AAC_NUM_MGT_FIB),

Quoted 10 lines

drivers/scsi/aacraid/commsup.c static int fib_map_alloc(struct aac_dev *dev) · line 94

Quoted 10 lines

drivers/scsi/aacraid/commsup.c +6 -1

-	  dev->hw_fib_va, dev->hw_fib_pa);
+	if (dev->hw_fib_va && dev->max_fib_size) {
+		pci_free_consistent(dev->pdev,
+		(dev->max_fib_size *
+		(dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB)),
+		dev->hw_fib_va, dev->hw_fib_pa);
+	}
 	dev->hw_fib_va = NULL;
 	dev->hw_fib_pa = 0;
 }

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

From: Raghava Aditya Renukunta <[email protected]>

During EEH recovery number of online CPU's might change thereby changing the number of MSIx vectors. Since each fib is allocated to a vector, changes in the number of vectors causes fib to be sent thru invalid vectors.In addition the correct number of MSIx vectors is not updated in the INIT struct sent to the controller, when it is reinitialized.

Fixed by reassigning vectors to fibs based on the updated number of MSIx vectors and updating the INIT structure before sending to controller.

drivers/scsi/aacraid/linit.c +4 -1

-	if (!dev->sync_mode)
+	if (!dev->sync_mode) {
+		/* After EEH recovery or suspend resume, max_msix count
+		 * may change, therfore updating in init as well.
+		 */

  aac_adapter_start(dev);

drivers/scsi/aacraid/linit.c +2 -0

+		dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix);
+	}

  return 0;

 error_iounmap:

-----Original Message-----
From: Johannes Thumshirn [mailto:[email protected]]
Sent: Wednesday, December 2, 2015 2:27 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 05/10] aacraid: Set correct msix count for EEH recovery

Hi Raghava,

Quoted 12 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

During EEH recovery number of online CPU's might change thereby changing the number of MSIx vectors. Since each fib is allocated to a vector, changes in the number of vectors causes fib to be sent thru invalid vectors.In addition the correct number of MSIx vectors is not updated in the INIT struct sent to the controller, when it is reinitialized.

Fixed by reassigning vectors to fibs based on the updated number of MSIx vectors and updating the INIT structure before sending to controller.

[...]

Quoted 10 lines

drivers/scsi/aacraid/linit.c +4 -1

-	if (!dev->sync_mode)
+	if (!dev->sync_mode) {
+		/* After EEH recovery or suspend resume, max_msix count
+		 * may change, therfore updating in init as well.
+		 */

  aac_adapter_start(dev);

drivers/scsi/aacraid/linit.c +2 -0

+		dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix);
+	}

  return 0;

Quoted 1 line

 error_iounmap:

This is for EEH and suspend/resume isn't it? If it fixes MSI-X vector calculation for suspend/resume as well you probably should tag it for inclusion into stable as well.

From: Raghava Aditya Renukunta <[email protected]>

During EEH recovery number of online CPU's might change thereby changing the number of MSIx vectors. Since each fib is allocated to a vector, changes in the number of vectors causes fib to be sent thru invalid vectors.In addition the correct number of MSIx vectors is not updated in the INIT struct sent to the controller, when it is reinitialized.

Fixed by reassigning vectors to fibs based on the updated number of MSIx vectors and updating the INIT structure before sending to controller.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/linit.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

drivers/scsi/aacraid/linit.c +8 -0

index 0147210..f88f1132 100644
@@ -1355,6 +1355,7 @@ static int aac_acquire_resources(struct aac_dev *dev)
 	int i, j;
 	int instance = dev->id;
 	const char *name = dev->name;
+	int vector = 0;
 	unsigned long status;
 	/*
 	 *	First clear out all interrupts.  Then enable the one's that we

@@ -1409,9 +1410,31 @@ static int aac_acquire_resources(struct aac_dev *dev)
 	}
 
 	aac_adapter_enable_int(dev);
+	/*max msix may change  after EEH
+	 * Re-assign vectors to fibs
+	 */
+	 for (i = 0;
+		i < (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB);
+		i++) {
+		if ((dev->max_msix == 1) ||

drivers/scsi/aacraid/linit.c +6 -1

-	if (!dev->sync_mode)
+	if (!dev->sync_mode) {
+		/* After EEH recovery or suspend resume, max_msix count
+		 * may change, therfore updating in init as well.
+		 */
 		aac_adapter_start(dev);
+		dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix);
+	}
 	return 0;
 
 error_iounmap:

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Friday, December 4, 2015 6:10 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 05/10] aacraid: Set correct msix count for EEH recovery

Quoted 3 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

During EEH recovery number of online CPU's might change thereby

changing

Quoted 10 lines

the number of MSIx vectors. Since each fib is allocated to a vector, changes in the number of vectors causes fib to be sent thru invalid vectors.In addition the correct number of MSIx vectors is not updated in the INIT struct sent to the controller, when it is reinitialized.

Fixed by reassigning vectors to fibs based on the updated number of MSIx vectors and updating the INIT structure before sending to controller.

Signed-off-by Raghava Aditya Renukunta

<[email protected]>

drivers/scsi/aacraid/linit.c static int aac_acquire_resources(struct aac_dev

Quoted 9 lines

---
 drivers/scsi/aacraid/linit.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

drivers/scsi/aacraid/linit.c +0 -0

index 0147210..f88f1132 100644
@@ -1355,6 +1355,7 @@ static int aac_acquire_resources(struct aac_dev

*dev)

drivers/scsi/aacraid/linit.c static int aac_acquire_resources(struct aac_dev

Quoted 8 lines

drivers/scsi/aacraid/linit.c +1 -0

 	int i, j;
 	int instance = dev->id;
 	const char *name = dev->name;
+	int vector = 0;
 	unsigned long status;
 	/*
 	 *	First clear out all interrupts.  Then enable the one's that we

@@ -1409,9 +1410,31 @@ static int aac_acquire_resources(struct aac_dev

*dev)

drivers/scsi/aacraid/linit.c static int aac_acquire_resources(struct aac_dev *dev)

Quoted 8 lines

drivers/scsi/aacraid/linit.c +5 -0

 	}
 
 	aac_adapter_enable_int(dev);
+	/*max msix may change  after EEH
+	 * Re-assign vectors to fibs
+	 */
+	 for (i = 0;
+		i < (dev->scsi_host_ptr->can_queue +

AAC_NUM_MGT_FIB);

drivers/scsi/aacraid/linit.c static int aac_acquire_resources(struct aac_dev *dev)

Quoted 3 lines

drivers/scsi/aacraid/linit.c +3 -0

+		i++) {
+		if ((dev->max_msix == 1) ||
+		   (i > ((dev->scsi_host_ptr->can_queue +

AAC_NUM_MGT_FIB - 1)

drivers/scsi/aacraid/linit.c static int aac_acquire_resources(struct aac_dev *dev) · line 1429

Quoted 9 lines

drivers/scsi/aacraid/linit.c +9 -0

+			- dev->vector_cap))) {
+			dev->fibs[i].vector_no = 0;
+		} else {
+			dev->fibs[i].vector_no = vector;
+			vector++;
+			if (vector == dev->max_msix)
+				vector = 1;
+		}
+	}

The above hunk added code looks identical to the part you have just added in 02/10 "aacraid: Fix RRQ overload" could you make this to function ? Thanks, tomash

Quoted 11 lines

drivers/scsi/aacraid/linit.c +6 -1

-	if (!dev->sync_mode)
+	if (!dev->sync_mode) {
+		/* After EEH recovery or suspend resume, max_msix count
+		 * may change, therfore updating in init as well.
+		 */
 		aac_adapter_start(dev);
+		dev->init->Sa_MSIXVectors = cpu_to_le32(dev->max_msix);
+	}
 	return 0;
 

drivers/scsi/aacraid/linit.c static int aac_acquire_resources(struct aac_dev *dev) · line 1440

Quoted 1 line

drivers/scsi/aacraid/linit.c +0 -0

 error_iounmap:

From: Raghava Aditya Renukunta <[email protected]>

Series 7 does not support PCI hot reset used by EEH.

Enabled fundamental reset only for Series 7

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/linit.c | 6 ++++++

 1 file changed, 6 insertions(+)

drivers/scsi/aacraid/linit.c +0 -0

index f88f1132..6912efd 100644
@@ -1135,6 +1135,12 @@ static int aac_probe_one(struct pci_dev *pdev, const
 struct pci_device_id *id)
  	u64 dmamask;
  	extern int aac_sync_mode;

drivers/scsi/aacraid/linit.c +6 -0

+	/*
+	 * Only series 7 needs freset.
+	 */
+	 if (pdev->device == PMC_DEVICE_S7)
+		pdev->needs_freset = 1;
+

  list_for_each_entry(aac, &aac_devices, entry) { if (aac->id > unique_id) break;

From: Raghava Aditya Renukunta <[email protected]>

while driver removal is in progress or PCI shutdown is invoked, driver kills AIF aacraid thread, but IOCTL requests from the management tools re-start AIF thread leading to IOP_RESET.

Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/linit.c | 1 +

 1 file changed, 1 insertion(+)

drivers/scsi/aacraid/linit.c +0 -0

index 6912efd..3a4dbe7 100644
@@ -1454,6 +1454,7 @@ static int aac_suspend(struct pci_dev *pdev,
 pm_message_t state)
  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;

  scsi_block_requests(shost);

drivers/scsi/aacraid/linit.c +1 -0

+	aac->adapter_shutdown = 1;

  aac_send_shutdown(aac);

  aac_release_resources(aac);

-----Original Message-----
From: Johannes Thumshirn [mailto:[email protected]]
Sent: Wednesday, December 2, 2015 2:01 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 07/10] aacraid: Fix AIF triggered IOP_RESET

Hi Raghava,

Quoted 26 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

while driver removal is in progress or PCI shutdown is invoked, driver kills AIF aacraid thread, but IOCTL requests from the management tools re-start AIF thread leading to IOP_RESET.

Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.

Signed-off-by: Raghava Aditya Renukunta
<[email protected]>
---
 drivers/scsi/aacraid/linit.c | 1 +

 1 file changed, 1 insertion(+)

(quoted diff) +0 -0

b/drivers/scsi/aacraid/linit.c index 6912efd..3a4dbe7 100644

drivers/scsi/aacraid/linit.c +1 -0

@@ -1454,6 +1454,7 @@ static int aac_suspend(struct pci_dev *pdev,
 pm_message_t state)
  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
 
  	scsi_block_requests(shost);
+	aac->adapter_shutdown = 1;
  	aac_send_shutdown(aac);
 

Quoted 1 line

  aac_release_resources(aac);

I don't quite undestand that, the following is from aac_send_shutdown():

229         /* FIB should be freed only after getting the response from the F/W */
230         if (status != -ERESTARTSYS)
231                 aac_fib_free(fibctx);
232         dev->adapter_shutdown = 1;
233         if ((dev->pdev->device == PMC_DEVICE_S7 ||
234              dev->pdev->device == PMC_DEVICE_S8 ||

in line 232 you're already setting the adapter shutdown flag, why do you need to pre-set it before calling aac_send_shutdown()?

Thanks,
Johannes

Hello Johannes,

Quoted 1 line

-----Original Message-----

Originally the problem was that dev->adapter_shutdown was set after the Shutdown command was sent to the controller, leading to a race condition  If after the controller was shut down and before dev->adapter_shutdown  was set , the driver received  a AIF command from management tool , it  would still be sent to the controller in aac_cfg_ioctl() leading to a  IOP_RESET.

A fix would  be to set dev->adapter_shutdown at the very beginning of aac_send_shutdown().

From: Raghava Aditya Renukunta <[email protected]>

Added module parameter that disables device ID wild card binding.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aachba.c  | 5 +++++
 drivers/scsi/aacraid/aacraid.h | 1 +
 drivers/scsi/aacraid/linit.c   | 5 +++++

 3 files changed, 11 insertions(+)

drivers/scsi/aacraid/aachba.c +0 -0

index 06cbab8..87f4f21 100644
@@ -315,6 +315,11 @@ MODULE_PARM_DESC(wwn, "Select a WWN type for the

arrays:\n"
  "\t1 - Array Meta Data Signature (default)\n" "\t2 - Adapter Serial Number");

drivers/scsi/aacraid/aachba.c +5 -0

+int aac_disable_device_id_wildcards;
+module_param_named(disable_device_id_wildcards,
+	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_device_id_wildcards,
+	"Disable device ID wildcards");

 static inline int aac_valid_context(struct scsi_cmnd *scsicmd, struct fib *fibptr) {

drivers/scsi/aacraid/aacraid.h +1 -0

index 594de5f..7708a2c 100644
@@ -2177,3 +2177,4 @@ extern int aac_commit;
  extern int update_interval;
  extern int check_interval;
  extern int aac_check_reset;
+extern int aac_disable_device_id_wildcards;

drivers/scsi/aacraid/linit.c +0 -0

index 3a4dbe7..2094842 100644
@@ -1135,6 +1135,11 @@ static int aac_probe_one(struct pci_dev *pdev, const
 struct pci_device_id *id)
  	u64 dmamask;
  	extern int aac_sync_mode;

drivers/scsi/aacraid/linit.c +5 -0

+	if (aac_disable_device_id_wildcards &&
+		id->subvendor == PCI_ANY_ID &&
+		id->subdevice == PCI_ANY_ID)
+		return -ENODEV;
+

  /*
   * Only series 7 needs freset.
   */

From: Raghava Aditya Renukunta <[email protected]>

Added module parameter that disables device ID wild card binding.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aachba.c  | 5 +++++
 drivers/scsi/aacraid/aacraid.h | 1 +
 drivers/scsi/aacraid/linit.c   | 5 +++++
 3 files changed, 11 insertions(+)

drivers/scsi/aacraid/aachba.c +5 -0

index 06cbab8..87f4f21 100644
@@ -315,6 +315,11 @@ MODULE_PARM_DESC(wwn, "Select a WWN type for the arrays:\n"
 	"\t1 - Array Meta Data Signature (default)\n"
 	"\t2 - Adapter Serial Number");
 
+int aac_disable_device_id_wildcards;
+module_param_named(disable_device_id_wildcards,
+	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_device_id_wildcards,
+	"Disable device ID wildcards");
 
 static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
 		struct fib *fibptr) {

drivers/scsi/aacraid/aacraid.h +1 -0

index 594de5f..7708a2c 100644
@@ -2177,3 +2177,4 @@ extern int aac_commit;
 extern int update_interval;
 extern int check_interval;
 extern int aac_check_reset;
+extern int aac_disable_device_id_wildcards;

drivers/scsi/aacraid/linit.c +5 -0

index 3a4dbe7..2094842 100644
@@ -1135,6 +1135,11 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	u64 dmamask;
 	extern int aac_sync_mode;
 
+	if (aac_disable_device_id_wildcards &&
+		id->subvendor == PCI_ANY_ID &&
+		id->subdevice == PCI_ANY_ID)
+		return -ENODEV;
+
 	/*
 	 * Only series 7 needs freset.
 	 */

-----Original Message-----
From: Tomas Henzl [mailto:[email protected]]
Sent: Thursday, December 3, 2015 7:55 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 08/10] aacraid: Disable device ID wildcard

On 1.12.2015 13:39, Raghava Aditya Renukunta wrote:

Quoted 3 lines

From: Raghava Aditya Renukunta <[email protected]>

Added module parameter that disables device ID wild card binding.

What is the use case for this?

It looks like something which could be solved in the modprobe config, without a module option.

Cheers,
Tomas

Quoted 2 lines

Signed-off-by Raghava Aditya Renukunta

<[email protected]>

drivers/scsi/aacraid/aachba.c MODULE_PARM_DESC(wwn, "Select a WWN type

Quoted 11 lines

---
 drivers/scsi/aacraid/aachba.c  | 5 +++++
 drivers/scsi/aacraid/aacraid.h | 1 +
 drivers/scsi/aacraid/linit.c   | 5 +++++
 3 files changed, 11 insertions(+)

drivers/scsi/aacraid/aachba.c +0 -0

index 06cbab8..87f4f21 100644
@@ -315,6 +315,11 @@ MODULE_PARM_DESC(wwn, "Select a WWN type

for the arrays:\n"

drivers/scsi/aacraid/linit.c static int aac_probe_one(struct pci_dev *pdev,

Quoted 25 lines

drivers/scsi/aacraid/aachba.c +5 -0

 	"\t1 - Array Meta Data Signature (default)\n"
 	"\t2 - Adapter Serial Number");
 
+int aac_disable_device_id_wildcards;
+module_param_named(disable_device_id_wildcards,
+	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_device_id_wildcards,
+	"Disable device ID wildcards");
 
 static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
 		struct fib *fibptr) {

drivers/scsi/aacraid/aacraid.h +1 -0

index 594de5f..7708a2c 100644
@@ -2177,3 +2177,4 @@ extern int aac_commit;
 extern int update_interval;
 extern int check_interval;
 extern int aac_check_reset;
+extern int aac_disable_device_id_wildcards;

drivers/scsi/aacraid/linit.c +0 -0

index 3a4dbe7..2094842 100644
@@ -1135,6 +1135,11 @@ static int aac_probe_one(struct pci_dev *pdev,

const struct pci_device_id *id)

Quoted 3 lines

drivers/scsi/aacraid/linit.c +0 -0

 	u64 dmamask;
 	extern int aac_sync_mode;
 

drivers/scsi/aacraid/linit.c static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id) · line 1145

Quoted 8 lines

drivers/scsi/aacraid/linit.c +5 -0

+	if (aac_disable_device_id_wildcards &&
+		id->subvendor == PCI_ANY_ID &&
+		id->subdevice == PCI_ANY_ID)
+		return -ENODEV;
+
 	/*
 	 * Only series 7 needs freset.
 	 */

This will enable us to prevent aacraid from loading for PCI devices that match device ID wildcards. Enabling us to use say a new driver for future devices.

-----Original Message-----
From: Christoph Hellwig [mailto:[email protected]]
Sent: Friday, December 4, 2015 12:33 AM
To: Raghava Aditya Renukunta
Cc: Tomas Henzl; [email protected]; [email protected]; Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 08/10] aacraid: Disable device ID wildcard

Quoted 1 line · Raghava Aditya Renukunta

This will enable us to prevent aacraid from loading for PCI devices that

match

Quoted 1 line

device ID wildcards. Enabling us to use say a new driver for future devices.

This looks like a bogus reason. The same PCI ID should always be compatible and mathed by the same driver. Even if you add a new driver to expose additional feature and break these semantics there is no point to do a) reject them conditionally on a module option and b) do this before said driver is merged.

From: Raghava Aditya Renukunta <[email protected]>

During EEH PCI hotplug activity kernel unloads and loads the driver, causing character device to be unregistered(aac_remove_one).When the driver is loaded back using aac_probe_one the character device needs to be registered again for the AIF management tools to work.

Fixed by adding code to register character device in aac_probe_one if it is unregistered in aac_remove_one.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/linit.c | 19 +++++++++++++------

 1 file changed, 13 insertions(+), 6 deletions(-)

drivers/scsi/aacraid/linit.c +7 -0

index 2094842..7142578 100644
@@ -1123,6 +1123,13 @@ static void __aac_shutdown(struct aac_dev * aac)
  	else if (aac->max_msix > 1)
  		pci_disable_msix(aac->pdev);
  }
+static void aac_init_char(void)
+{
+	aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops);
+	if (aac_cfg_major < 0) {
+		pr_err("aacraid: unable to register \"aac\" device.\n");
+	}
+}

 static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 {

drivers/scsi/aacraid/linit.c +0 -0

@@ -1185,6 +1192,9 @@ static int aac_probe_one(struct pci_dev *pdev, const
 struct pci_device_id *id)
  	shost->max_cmd_len = 16;
  	shost->use_cmd_list = 1;

drivers/scsi/aacraid/linit.c +3 -0

+	if (aac_cfg_major == -2)
+		aac_init_char();
+

  aac = (struct aac_dev *)shost->hostdata; aac->base_start = pci_resource_start(pdev, 0); aac->scsi_host_ptr = shost;

drivers/scsi/aacraid/linit.c +0 -0

  	}
  }

drivers/scsi/aacraid/linit.c +0 -0

@@ -1697,11 +1707,8 @@ static int __init aac_init(void)
  	if (error < 0)
  		return error;

drivers/scsi/aacraid/linit.c +2 -5

-	aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops);
-	if (aac_cfg_major < 0) {
-		printk(KERN_WARNING
-			"aacraid: unable to register \"aac\" device.\n");
-	}
+	aac_init_char();
+

  return 0;
 }

-----Original Message-----
From: Johannes Thumshirn [mailto:[email protected]]
Sent: Wednesday, December 2, 2015 2:14 AM
To: Raghava Aditya Renukunta; [email protected]; linux- [email protected]
Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran; [email protected]; Rich Bono
Subject: Re: [PATCH 09/10] aacraid: Fix character device re-initialization

Hi Raghava,

drivers/scsi/aacraid/linit.c static void __aac_shutdown(struct aac_dev *

Quoted 21 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

During EEH PCI hotplug activity kernel unloads and loads the driver, causing character device to be unregistered(aac_remove_one).When the driver is loaded back using aac_probe_one the character device needs to be registered again for the AIF management tools to work.

Fixed by adding code to register character device in aac_probe_one if it is unregistered in aac_remove_one.

Signed-off-by: Raghava Aditya Renukunta <[email protected]>
---
 drivers/scsi/aacraid/linit.c | 19 +++++++++++++------

 1 file changed, 13 insertions(+), 6 deletions(-)

(quoted diff) +0 -0

b/drivers/scsi/aacraid/linit.c index 2094842..7142578 100644

drivers/scsi/aacraid/linit.c +0 -0

@@ -1123,6 +1123,13 @@ static void __aac_shutdown(struct aac_dev *

aac)

drivers/scsi/aacraid/linit.c static void aac_remove_one(struct pci_dev

Quoted 27 lines

drivers/scsi/aacraid/linit.c +7 -0

  	else if (aac->max_msix > 1)
  		pci_disable_msix(aac->pdev);
  }
+static void aac_init_char(void)
+{
+	aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops);
+	if (aac_cfg_major < 0) {
+		pr_err("aacraid: unable to register \"aac\" device.\n");
+	}
+}
 

 static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id
*id)
 {

drivers/scsi/aacraid/linit.c +3 -0

@@ -1185,6 +1192,9 @@ static int aac_probe_one(struct pci_dev *pdev,
 const struct pci_device_id *id)
  	shost->max_cmd_len = 16;
  	shost->use_cmd_list = 1;
 
+	if (aac_cfg_major == -2)
+		aac_init_char();
+
  	aac = (struct aac_dev *)shost->hostdata;
  	aac->base_start = pci_resource_start(pdev, 0);

  aac->scsi_host_ptr = shost;

drivers/scsi/aacraid/linit.c +0 -0

@@ -1536,7 +1546,7 @@ static void aac_remove_one(struct pci_dev

*pdev)

drivers/scsi/aacraid/linit.c static void aac_remove_one(struct pci_dev *pdev) · line 1549

Quoted 5 lines

drivers/scsi/aacraid/linit.c +1 -1

  	pci_disable_device(pdev);
  	if (list_empty(&aac_devices)) {
  		unregister_chrdev(aac_cfg_major, "aac");
-		aac_cfg_major = -1;
+		aac_cfg_major = -2;

Please add something like
#define AAC_CHARDEV_UNREGISTERED -1
#define AAC_CHARDEV_NEEDS_REINIT -2

so it's obvious what you're doing.

Quoted 16 lines

drivers/scsi/aacraid/linit.c +2 -5

  	}
  }
 

@@ -1697,11 +1707,8 @@ static int __init aac_init(void)
  	if (error < 0)
  		return error;
 
-	aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops);
-	if (aac_cfg_major < 0) {
-		printk(KERN_WARNING
-			"aacraid: unable to register \"aac\" device.\n");
-	}
+	aac_init_char();
+
 

drivers/scsi/aacraid/linit.c static int __init aac_init(void) · line 1714

Quoted 2 lines

drivers/scsi/aacraid/linit.c +0 -0

  	return 0;
  }

From: Raghava Aditya Renukunta <[email protected]>

During EEH PCI hotplug activity kernel unloads and loads the driver, causing character device to be unregistered(aac_remove_one).When the driver is loaded back using aac_probe_one the character device needs to be registered again for the AIF management tools to work.

Fixed by adding code to register character device in aac_probe_one if it is unregistered in aac_remove_one.

Signed-off-by Raghava Aditya Renukunta <[email protected]>

Hi Raghava,

Quoted 7 lines · Raghava Aditya Renukunta

From: Raghava Aditya Renukunta <[email protected]>

During EEH PCI hotplug activity kernel unloads and loads the driver, causing character device to be unregistered(aac_remove_one).When the driver is loaded back using aac_probe_one the character device needs to be registered again for the AIF management tools to work.

Quoted 5 lines

Fixed by adding code to register character device in aac_probe_one if it is unregistered in aac_remove_one.

Signed-off-by: Raghava Aditya Renukunta
<[email protected]>

This one is duplicate, isn't it?

From: Raghava Aditya Renukunta <[email protected]>

Updated diver version to 41052

Signed-off-by Raghava Aditya Renukunta <[email protected]>

---
 drivers/scsi/aacraid/aacraid.h | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/scsi/aacraid/aacraid.h +0 -0

index 7708a2c..bbb94ec 100644
@@ -62,7 +62,7 @@ enum {
  #define	PMC_GLOBAL_INT_BIT0		0x00000001

 #ifndef AAC_DRIVER_BUILD

drivers/scsi/aacraid/aacraid.h +1 -1

-# define AAC_DRIVER_BUILD 41010
+# define AAC_DRIVER_BUILD 41052

 # define AAC_DRIVER_BRANCH "-ms"
 #endif
 #define MAXIMUM_NUM_CONTAINERS	32