[PATCH V3 0/9] aacraid: Patchset for aacraid driver version 41052

[PATCH V3 0/9] aacraid: Patchset for aacraid driver version 41052

[Raghava Aditya Renukunta]

This patchset includes the following changes (bug fixes and new feature 
support) specific to aacraid driver.

V2:
Removed aac_fib_free_tag function
Setup relevant fib variables only once
Created aac_fib_vector_assign function
Made EEH functions static
Added character device status macros
changed location of aac->shutdown to prevent race condition
Withdrew patch that disables device ID wild card binding
Added Reviewed-by, Cc and Fixes tags from  mailing list

V3:
Moved aac_send_shutdown to top of __aac_shutdown
Converted aac->adapter_shutdown to atomic

Raghava Aditya Renukunta (9):
  [SCSI] aacraid: SCSI blk tag support
  [SCSI] aacraid: Fix RRQ overload
  [SCSI] aacraid: Added EEH support
  [SCSI] aacraid: Fix memory leak in aac_fib_map_free
  [SCSI] aacraid: Set correct msix count for EEH recovery
  [SCSI] aacraid: Fundamental reset support for Series 7
  [SCSI] aacraid: Fix AIF triggered IOP_RESET
  [SCSI] aacraid: Fix character device re-initialization
  [SCSI] aacraid: Update driver version

 drivers/scsi/aacraid/aachba.c   |  27 +++---
 drivers/scsi/aacraid/aacraid.h  |  15 +++-
 drivers/scsi/aacraid/comminit.c |   6 +-
 drivers/scsi/aacraid/commsup.c  |  69 +++++++++++++--
 drivers/scsi/aacraid/dpcsup.c   |   2 -
 drivers/scsi/aacraid/linit.c    | 188 +++++++++++++++++++++++++++++++++++++---
 drivers/scsi/aacraid/src.c      |  30 ++-----
 7 files changed, 275 insertions(+), 62 deletions(-)

-- 
1.9.1

[PATCH V3 1/9] aacraid: SCSI blk tag support

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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 aac_fib_alloc_tag was
created to 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.

Changes in V2:
Removed aac_fib_free_tag since it was a stub function
Moved population of fib fields that are constant to aac_fib_setup

Changes in V3:
None

Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
---
 drivers/scsi/aacraid/aachba.c  | 27 ++++++++++++---------------
 drivers/scsi/aacraid/aacraid.h |  1 +
 drivers/scsi/aacraid/commsup.c | 32 +++++++++++++++++++++++++++++---
 drivers/scsi/aacraid/dpcsup.c  |  2 --
 drivers/scsi/aacraid/linit.c   |  2 ++
 5 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index e4c2437..7dfd0fa 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -323,7 +323,6 @@ 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);
 		return 0;
 	}
 	scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;
@@ -331,7 +330,6 @@ 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);
 		return 0;
 	}
 	return 1;
@@ -541,7 +539,6 @@ 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);
 	scsicmd->scsi_done(scsicmd);
 }
 
@@ -557,7 +554,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 +584,6 @@ 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);
 	return -1;
 }
 
@@ -1024,7 +1021,6 @@ 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);
 	scsicmd->scsi_done(scsicmd);
 }
 
@@ -1040,7 +1036,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 +1065,6 @@ 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);
 	return -1;
 }
 
@@ -1869,7 +1865,6 @@ static void io_callback(void *context, struct fib * fibptr)
 		break;
 	}
 	aac_fib_complete(fibptr);
-	aac_fib_free(fibptr);
 
 	scsicmd->scsi_done(scsicmd);
 }
@@ -1954,7 +1949,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 +2047,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 +2282,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 +3154,6 @@ 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);
 	scsicmd->scsi_done(scsicmd);
 }
 
@@ -3187,9 +3183,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);
 
 	/*
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 074878b..f51f0a0 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2114,6 +2114,7 @@ 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);
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index a1f90fe..6b286b3 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -137,6 +137,7 @@ int aac_fib_setup(struct aac_dev * dev)
 		i++, fibptr++)
 	{
 		fibptr->flags = 0;
+		fibptr->size = sizeof(struct fib);
 		fibptr->dev = dev;
 		fibptr->hw_fib_va = hw_fib;
 		fibptr->data = (void *) fibptr->hw_fib_va->data;
@@ -156,13 +157,38 @@ 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;
 }
 
 /**
+ *	aac_fib_alloc_tag	-	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)
+{
+	struct fib *fibptr;
+
+	fibptr = &dev->fibs[scmd->request->tag];
+	/*
+	 *	Null out fields that depend on being zero at the start of
+	 *	each I/O
+	 */
+	fibptr->hw_fib_va->header.XferState = 0;
+	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
+	fibptr->callback_data = NULL;
+	fibptr->callback = NULL;
+
+	return fibptr;
+}
+
+/**
  *	aac_fib_alloc	-	allocate a fib
  *	@dev: Adapter to allocate the fib for
  *
diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
index da9d993..d677b52 100644
--- a/drivers/scsi/aacraid/dpcsup.c
+++ b/drivers/scsi/aacraid/dpcsup.c
@@ -394,7 +394,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
 				fib->callback(fib->callback_data, fib);
 			} else {
 				aac_fib_complete(fib);
-				aac_fib_free(fib);
 			}
 		} else {
 			unsigned long flagv;
@@ -416,7 +415,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
 				fib->done = 0;
 				spin_unlock_irqrestore(&fib->event_lock, flagv);
 				aac_fib_complete(fib);
-				aac_fib_free(fib);
 			}
 
 		}
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 76eaa38..129a515 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -454,6 +454,8 @@ static int aac_slave_configure(struct scsi_device *sdev)
 	} else
 		scsi_change_queue_depth(sdev, 1);
 
+		sdev->tagged_supported = 1;
+
 	return 0;
 }
 
-- 
1.9.1

RE: [PATCH V3 1/9] aacraid: SCSI blk tag support

[Seymour, Shane M]

Reviewed-by: Shane Seymour <shane.seymour@hpe.com>

Re: [PATCH V3 1/9] aacraid: SCSI blk tag support

[Johannes Thumshirn]

On Thu, Jan 14, 2016 at 11:16:14PM -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> 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 aac_fib_alloc_tag was
> created to 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.
> 
> Changes in V2:
> Removed aac_fib_free_tag since it was a stub function
> Moved population of fib fields that are constant to aac_fib_setup
> 
> Changes in V3:
> None
> 
> Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aachba.c  | 27 ++++++++++++---------------
>  drivers/scsi/aacraid/aacraid.h |  1 +
>  drivers/scsi/aacraid/commsup.c | 32 +++++++++++++++++++++++++++++---
>  drivers/scsi/aacraid/dpcsup.c  |  2 --
>  drivers/scsi/aacraid/linit.c   |  2 ++
>  5 files changed, 44 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index e4c2437..7dfd0fa 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -323,7 +323,6 @@ 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);
>  		return 0;
>  	}
>  	scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;
> @@ -331,7 +330,6 @@ 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);
>  		return 0;
>  	}
>  	return 1;
> @@ -541,7 +539,6 @@ 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);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -557,7 +554,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 +584,6 @@ 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);
>  	return -1;
>  }
>  
> @@ -1024,7 +1021,6 @@ 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);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -1040,7 +1036,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 +1065,6 @@ 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);
>  	return -1;
>  }
>  
> @@ -1869,7 +1865,6 @@ static void io_callback(void *context, struct fib * fibptr)
>  		break;
>  	}
>  	aac_fib_complete(fibptr);
> -	aac_fib_free(fibptr);
>  
>  	scsicmd->scsi_done(scsicmd);
>  }
> @@ -1954,7 +1949,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 +2047,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 +2282,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 +3154,6 @@ 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);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -3187,9 +3183,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);
>  
>  	/*
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 074878b..f51f0a0 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2114,6 +2114,7 @@ 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);
> diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
> index a1f90fe..6b286b3 100644
> --- a/drivers/scsi/aacraid/commsup.c
> +++ b/drivers/scsi/aacraid/commsup.c
> @@ -137,6 +137,7 @@ int aac_fib_setup(struct aac_dev * dev)
>  		i++, fibptr++)
>  	{
>  		fibptr->flags = 0;
> +		fibptr->size = sizeof(struct fib);
>  		fibptr->dev = dev;
>  		fibptr->hw_fib_va = hw_fib;
>  		fibptr->data = (void *) fibptr->hw_fib_va->data;
> @@ -156,13 +157,38 @@ 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;
>  }
>  
>  /**
> + *	aac_fib_alloc_tag	-	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)
> +{
> +	struct fib *fibptr;
> +
> +	fibptr = &dev->fibs[scmd->request->tag];
> +	/*
> +	 *	Null out fields that depend on being zero at the start of
> +	 *	each I/O
> +	 */
> +	fibptr->hw_fib_va->header.XferState = 0;
> +	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
> +	fibptr->callback_data = NULL;
> +	fibptr->callback = NULL;
> +
> +	return fibptr;
> +}
> +
> +/**
>   *	aac_fib_alloc	-	allocate a fib
>   *	@dev: Adapter to allocate the fib for
>   *
> diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
> index da9d993..d677b52 100644
> --- a/drivers/scsi/aacraid/dpcsup.c
> +++ b/drivers/scsi/aacraid/dpcsup.c
> @@ -394,7 +394,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
>  				fib->callback(fib->callback_data, fib);
>  			} else {
>  				aac_fib_complete(fib);
> -				aac_fib_free(fib);
>  			}
>  		} else {
>  			unsigned long flagv;
> @@ -416,7 +415,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
>  				fib->done = 0;
>  				spin_unlock_irqrestore(&fib->event_lock, flagv);
>  				aac_fib_complete(fib);
> -				aac_fib_free(fib);
>  			}
>  
>  		}
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 76eaa38..129a515 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -454,6 +454,8 @@ static int aac_slave_configure(struct scsi_device *sdev)
>  	} else
>  		scsi_change_queue_depth(sdev, 1);
>  
> +		sdev->tagged_supported = 1;
> +
>  	return 0;
>  }
>  
> -- 
> 1.9.1
> 

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/9] aacraid: SCSI blk tag support

[Tomas Henzl]

On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>
> 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 aac_fib_alloc_tag was
> created to 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.
>
> Changes in V2:
> Removed aac_fib_free_tag since it was a stub function
> Moved population of fib fields that are constant to aac_fib_setup
>
> Changes in V3:
> None
>
> Signed-off-by: Raghava Aditya Renukunta <RaghavaAditya.Renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aachba.c  | 27 ++++++++++++---------------
>  drivers/scsi/aacraid/aacraid.h |  1 +
>  drivers/scsi/aacraid/commsup.c | 32 +++++++++++++++++++++++++++++---
>  drivers/scsi/aacraid/dpcsup.c  |  2 --
>  drivers/scsi/aacraid/linit.c   |  2 ++
>  5 files changed, 44 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index e4c2437..7dfd0fa 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -323,7 +323,6 @@ 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);
>  		return 0;
>  	}
>  	scsicmd->SCp.phase = AAC_OWNER_MIDLEVEL;
> @@ -331,7 +330,6 @@ 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);
>  		return 0;
>  	}
>  	return 1;
> @@ -541,7 +539,6 @@ 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);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -557,7 +554,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_alloc_tag can't return NULL in the new implementation,
in some next series please remove all the needless tests

Reviewed-by: Tomas Henzl <thenzl@redhat.com>

Tomas

>  
>  	aac_fib_init(cmd_fibcontext);
> @@ -586,7 +584,6 @@ 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);
>  	return -1;
>  }
>  
> @@ -1024,7 +1021,6 @@ 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);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -1040,7 +1036,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 +1065,6 @@ 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);
>  	return -1;
>  }
>  
> @@ -1869,7 +1865,6 @@ static void io_callback(void *context, struct fib * fibptr)
>  		break;
>  	}
>  	aac_fib_complete(fibptr);
> -	aac_fib_free(fibptr);
>  
>  	scsicmd->scsi_done(scsicmd);
>  }
> @@ -1954,7 +1949,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 +2047,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 +2282,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 +3154,6 @@ 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);
>  	scsicmd->scsi_done(scsicmd);
>  }
>  
> @@ -3187,9 +3183,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);
>  
>  	/*
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 074878b..f51f0a0 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2114,6 +2114,7 @@ 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);
> diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
> index a1f90fe..6b286b3 100644
> --- a/drivers/scsi/aacraid/commsup.c
> +++ b/drivers/scsi/aacraid/commsup.c
> @@ -137,6 +137,7 @@ int aac_fib_setup(struct aac_dev * dev)
>  		i++, fibptr++)
>  	{
>  		fibptr->flags = 0;
> +		fibptr->size = sizeof(struct fib);
>  		fibptr->dev = dev;
>  		fibptr->hw_fib_va = hw_fib;
>  		fibptr->data = (void *) fibptr->hw_fib_va->data;
> @@ -156,13 +157,38 @@ 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;
>  }
>  
>  /**
> + *	aac_fib_alloc_tag	-	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)
> +{
> +	struct fib *fibptr;
> +
> +	fibptr = &dev->fibs[scmd->request->tag];
> +	/*
> +	 *	Null out fields that depend on being zero at the start of
> +	 *	each I/O
> +	 */
> +	fibptr->hw_fib_va->header.XferState = 0;
> +	fibptr->type = FSAFS_NTC_FIB_CONTEXT;
> +	fibptr->callback_data = NULL;
> +	fibptr->callback = NULL;
> +
> +	return fibptr;
> +}
> +
> +/**
>   *	aac_fib_alloc	-	allocate a fib
>   *	@dev: Adapter to allocate the fib for
>   *
> diff --git a/drivers/scsi/aacraid/dpcsup.c b/drivers/scsi/aacraid/dpcsup.c
> index da9d993..d677b52 100644
> --- a/drivers/scsi/aacraid/dpcsup.c
> +++ b/drivers/scsi/aacraid/dpcsup.c
> @@ -394,7 +394,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
>  				fib->callback(fib->callback_data, fib);
>  			} else {
>  				aac_fib_complete(fib);
> -				aac_fib_free(fib);
>  			}
>  		} else {
>  			unsigned long flagv;
> @@ -416,7 +415,6 @@ unsigned int aac_intr_normal(struct aac_dev *dev, u32 index,
>  				fib->done = 0;
>  				spin_unlock_irqrestore(&fib->event_lock, flagv);
>  				aac_fib_complete(fib);
> -				aac_fib_free(fib);
>  			}
>  
>  		}
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 76eaa38..129a515 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -454,6 +454,8 @@ static int aac_slave_configure(struct scsi_device *sdev)
>  	} else
>  		scsi_change_queue_depth(sdev, 1);
>  
> +		sdev->tagged_supported = 1;
> +
>  	return 0;
>  }
>  

[PATCH V3 2/9] aacraid: Fix RRQ overload

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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.

Changes in V2:
Created function aac_fib_vector_assign to assign vector numbers to
fib and to prevent code duplication

Changes in V3:
None

Fixes: 495c0217 "aacraid: MSI-x support"
Cc: stable@vger.kernel.org # v4.1

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
 drivers/scsi/aacraid/aacraid.h |  2 ++
 drivers/scsi/aacraid/commsup.c | 28 ++++++++++++++++++++++++++++
 drivers/scsi/aacraid/src.c     | 30 +++++++-----------------------
 3 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index f51f0a0..fff1306 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -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*/
 };
@@ -2113,6 +2114,7 @@ static inline unsigned int cap_to_cyls(sector_t capacity, unsigned divisor)
 int aac_acquire_irq(struct aac_dev *dev);
 void aac_free_irq(struct aac_dev *dev);
 const char *aac_driverinfo(struct Scsi_Host *);
+void aac_fib_vector_assign(struct aac_dev *dev);
 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);
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 6b286b3..e9f6e70 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -90,6 +90,28 @@ void aac_fib_map_free(struct aac_dev *dev)
 	dev->hw_fib_pa = 0;
 }
 
+void aac_fib_vector_assign(struct aac_dev *dev)
+{
+	u32 i = 0;
+	u32 vector = 1;
+	struct fib *fibptr = NULL;
+
+	for (i = 0, fibptr = &dev->fibs[i];
+		i < (dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB);
+		i++, fibptr++) {
+		if ((dev->max_msix == 1) ||
+		  (i > ((dev->scsi_host_ptr->can_queue + AAC_NUM_MGT_FIB - 1)
+			- dev->vector_cap))) {
+			fibptr->vector_no = 0;
+		} else {
+			fibptr->vector_no = vector;
+			vector++;
+			if (vector == dev->max_msix)
+				vector = 1;
+		}
+	}
+}
+
 /**
  *	aac_fib_setup	-	setup the fibs
  *	@dev: Adapter to set up
@@ -152,6 +174,12 @@ int aac_fib_setup(struct aac_dev * dev)
 		hw_fib_pa = hw_fib_pa +
 			dev->max_fib_size + sizeof(struct aac_fib_xporthdr);
 	}
+
+	/*
+	 *Assign vector numbers to fibs
+	 */
+	aac_fib_vector_assign(dev);
+
 	/*
 	 *	Add the fib chain to the free list
 	 */
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 2aa34ea..bc0203f 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -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->rrq_outstanding[vector_no]);
-			aac_intr_normal(dev, handle-1, 0, isFastResponse, NULL);
 			dev->host_rrq[index++] = 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;
@@ -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);
+	u16 vector_no;
 
 	atomic_inc(&q->numpending);
 
 	if (dev->msi_enabled && fib->hw_fib_va->header.Command != AifRequest &&
 	    dev->max_msix > 1) {
-		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);
+	} else {
+		vector_no = 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;
-- 
1.9.1

[PATCH V3 3/9] aacraid: Added EEH support

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

Added support for PCI EEH(extended error handling).

Changes in V2:
Made local functions static
Removed call to  aac_fib_free_tag
Set adapter_shutdown flag when PCI error detected

Changes in V3:
None

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
 drivers/scsi/aacraid/aacraid.h |   1 +
 drivers/scsi/aacraid/linit.c   | 140 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 insertions(+)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index fff1306..2916288 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -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;
+	u32			handle_pci_error;
 };
 
 #define aac_adapter_interrupt(dev) \
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 129a515..08c6835 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -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:
@@ -1501,6 +1505,141 @@ static void aac_remove_one(struct pci_dev *pdev)
 	}
 }
 
+static 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);
+
+			if (aac->handle_pci_error)
+				cmd->result = DID_NO_CONNECT << 16;
+			else
+				cmd->result = DID_RESET << 16;
+
+			cmd->scsi_done(cmd);
+		}
+	}
+}
+
+static 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;
+		aac->adapter_shutdown = 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->adapter_shutdown = 1;
+
+		aac_flush_ios(aac);
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static 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;
+}
+
+static 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;
+}
+
+
+static 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 +1650,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)
-- 
1.9.1

Re: [PATCH V3 3/9] aacraid: Added EEH support

[Johannes Thumshirn]

On Thu, Jan 14, 2016 at 11:16:16PM -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> Added support for PCI EEH(extended error handling).
> 
> Changes in V2:
> Made local functions static
> Removed call to  aac_fib_free_tag
> Set adapter_shutdown flag when PCI error detected
> 
> Changes in V3:
> None
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> Reviewed-by: Tomas Henzl <thenzl@redhat.com>
> ---
>  drivers/scsi/aacraid/aacraid.h |   1 +
>  drivers/scsi/aacraid/linit.c   | 140 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 141 insertions(+)
> 
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index fff1306..2916288 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -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;
> +	u32			handle_pci_error;
>  };
>  
>  #define aac_adapter_interrupt(dev) \
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 129a515..08c6835 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -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:
> @@ -1501,6 +1505,141 @@ static void aac_remove_one(struct pci_dev *pdev)
>  	}
>  }
>  
> +static 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);
> +
> +			if (aac->handle_pci_error)
> +				cmd->result = DID_NO_CONNECT << 16;
> +			else
> +				cmd->result = DID_RESET << 16;
> +
> +			cmd->scsi_done(cmd);
> +		}
> +	}
> +}
> +
> +static 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;
> +		aac->adapter_shutdown = 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->adapter_shutdown = 1;
> +
> +		aac_flush_ios(aac);
> +		return PCI_ERS_RESULT_DISCONNECT;
> +	}
> +
> +	return PCI_ERS_RESULT_NEED_RESET;
> +}
> +
> +static 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;
> +}
> +
> +static 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;
> +}
> +
> +
> +static 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 +1650,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)
> -- 
> 1.9.1
> 

Looks good,

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 3/9] aacraid: Added EEH support

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3801 bytes --]

Hi Raghava,

[auto build test ERROR on scsi/for-next]
[also build test ERROR on v4.4 next-20160122]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Raghava-Aditya-Renukunta/aacraid-SCSI-blk-tag-support/20160115-070931
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: x86_64-randconfig-s1-01240612 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/scsi/aacraid/linit.c: In function 'aac_pci_error_detected':
>> drivers/scsi/aacraid/linit.c:1546:3: error: implicit declaration of function 'aac_release_resources' [-Werror=implicit-function-declaration]
      aac_release_resources(aac);
      ^
   drivers/scsi/aacraid/linit.c: In function 'aac_pci_resume':
>> drivers/scsi/aacraid/linit.c:1617:2: error: implicit declaration of function 'aac_acquire_resources' [-Werror=implicit-function-declaration]
     aac_acquire_resources(aac);
     ^
   cc1: some warnings being treated as errors

vim +/aac_release_resources +1546 drivers/scsi/aacraid/linit.c

  1540	
  1541			aac->handle_pci_error = 1;
  1542			aac->adapter_shutdown = 1;
  1543	
  1544			scsi_block_requests(aac->scsi_host_ptr);
  1545			aac_flush_ios(aac);
> 1546			aac_release_resources(aac);
  1547	
  1548			pci_disable_pcie_error_reporting(pdev);
  1549			aac_adapter_ioremap(aac, 0);
  1550	
  1551			return PCI_ERS_RESULT_NEED_RESET;
  1552		case pci_channel_io_perm_failure:
  1553			aac->handle_pci_error = 1;
  1554			aac->adapter_shutdown = 1;
  1555	
  1556			aac_flush_ios(aac);
  1557			return PCI_ERS_RESULT_DISCONNECT;
  1558		}
  1559	
  1560		return PCI_ERS_RESULT_NEED_RESET;
  1561	}
  1562	
  1563	static pci_ers_result_t aac_pci_mmio_enabled(struct pci_dev *pdev)
  1564	{
  1565		dev_err(&pdev->dev, "aacraid: PCI error - mmio enabled\n");
  1566		return PCI_ERS_RESULT_NEED_RESET;
  1567	}
  1568	
  1569	static pci_ers_result_t aac_pci_slot_reset(struct pci_dev *pdev)
  1570	{
  1571		dev_err(&pdev->dev, "aacraid: PCI error - slot reset\n");
  1572		pci_restore_state(pdev);
  1573		if (pci_enable_device(pdev)) {
  1574			dev_warn(&pdev->dev,
  1575				"aacraid: failed to enable slave\n");
  1576			goto fail_device;
  1577		}
  1578	
  1579		pci_set_master(pdev);
  1580	
  1581		if (pci_enable_device_mem(pdev)) {
  1582			dev_err(&pdev->dev, "pci_enable_device_mem failed\n");
  1583			goto fail_device;
  1584		}
  1585	
  1586		return PCI_ERS_RESULT_RECOVERED;
  1587	
  1588	fail_device:
  1589		dev_err(&pdev->dev, "aacraid: PCI error - slot reset failed\n");
  1590		return PCI_ERS_RESULT_DISCONNECT;
  1591	}
  1592	
  1593	
  1594	static void aac_pci_resume(struct pci_dev *pdev)
  1595	{
  1596		struct Scsi_Host *shost = pci_get_drvdata(pdev);
  1597		struct scsi_device *sdev = NULL;
  1598		struct aac_dev *aac = (struct aac_dev *)shost_priv(shost);
  1599	
  1600		pci_cleanup_aer_uncorrect_error_status(pdev);
  1601	
  1602		if (aac_adapter_ioremap(aac, aac->base_size)) {
  1603	
  1604			dev_err(&pdev->dev, "aacraid: ioremap failed\n");
  1605			/* remap failed, go back ... */
  1606			aac->comm_interface = AAC_COMM_PRODUCER;
  1607			if (aac_adapter_ioremap(aac, AAC_MIN_FOOTPRINT_SIZE)) {
  1608				dev_warn(&pdev->dev,
  1609					"aacraid: unable to map adapter.\n");
  1610	
  1611				return;
  1612			}
  1613		}
  1614	
  1615		msleep(10000);
  1616	
> 1617		aac_acquire_resources(aac);
  1618	
  1619		/*
  1620		 * reset this flag to unblock ioctl() as it was set

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 26518 bytes --]

[PATCH V3 4/9] aacraid: Fix memory leak in aac_fib_map_free

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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.

Changes in V2:
None

Changes in V3:
None

Fixes: 9ad5204d6 - "[SCSI]aacraid: incorrect dma mapping mask
during blinked recover or user initiated reset"
Cc: stable@vger.kernel.org

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
Reviewed-by: Tomas Henzl <thenzl@redhat.com>
---
 drivers/scsi/aacraid/commsup.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index e9f6e70..55459b4 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -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;
 }
-- 
1.9.1

[PATCH V3 5/9] aacraid: Set correct msix count for EEH recovery

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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.

Changes in V2:
Replaced fib vector allocation code with aac_fib_vector_assign

Changes in V3:
None

Fixes: MSI-X vector calculation for suspend/resume
Cc: stable@vger.kernel.org

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
---
 drivers/scsi/aacraid/linit.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 08c6835..6d79181 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1410,8 +1410,18 @@ static int aac_acquire_resources(struct aac_dev *dev)
 
 	aac_adapter_enable_int(dev);
 
-	if (!dev->sync_mode)
+	/*max msix may change  after EEH
+	 * Re-assign vectors to fibs
+	 */
+	aac_fib_vector_assign(dev);
+
+	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:
-- 
1.9.1

Re: [PATCH V3 5/9] aacraid: Set correct msix count for EEH recovery

[Johannes Thumshirn]

On Thu, Jan 14, 2016 at 11:16:18PM -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> 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.
> 
> Changes in V2:
> Replaced fib vector allocation code with aac_fib_vector_assign
> 
> Changes in V3:
> None
> 
> Fixes: MSI-X vector calculation for suspend/resume
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
> ---
>  drivers/scsi/aacraid/linit.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 08c6835..6d79181 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -1410,8 +1410,18 @@ static int aac_acquire_resources(struct aac_dev *dev)
>  
>  	aac_adapter_enable_int(dev);
>  
> -	if (!dev->sync_mode)
> +	/*max msix may change  after EEH
> +	 * Re-assign vectors to fibs
> +	 */
> +	aac_fib_vector_assign(dev);
> +
> +	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:
> -- 
> 1.9.1
> 

Looks good,

Reviewed-by: Johannes Thumshirn <jthushirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 6/9] aacraid: Fundamental reset support for Series 7

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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

Enabled fundamental reset only for Series 7

Changes in V2:
None

Changes in V3:
None

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/aacraid/linit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 6d79181..6944560 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -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;
 
+	/*
+	 * 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;
-- 
1.9.1

Re: [PATCH V3 6/9] aacraid: Fundamental reset support for Series 7

[Tomas Henzl]

On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>
> Series 7 does not support PCI hot reset used by EEH.
>
> Enabled fundamental reset only for Series 7
>
> Changes in V2:
> None
>
> Changes in V3:
> None

Reviewed-by: Tomas Henzl <thenzl@redhat.com>

Tomas

>
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  drivers/scsi/aacraid/linit.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 6d79181..6944560 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -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;
>  
> +	/*
> +	 * 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;

[PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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.

Changes in V2:
Set adapter_shutdown flag before shutdown command is sent to \
controller

Changes in V3:
Call aac_send_shut_shutdown first thing in __aac_shutdown
Convert adapter_shutdown to atomic_t variable to prevent \
SMP coherency issues(race conditions)

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
---
 drivers/scsi/aacraid/aacraid.h  |  2 +-
 drivers/scsi/aacraid/comminit.c |  6 +++---
 drivers/scsi/aacraid/linit.c    | 15 ++++++++-------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 2916288..3473668 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1234,7 +1234,7 @@ struct aac_dev
 	int			msi_enabled;	/* MSI/MSI-X enabled */
 	struct msix_entry	msixentry[AAC_MAX_MSIX];
 	struct aac_msix_ctx	aac_msix[AAC_MAX_MSIX]; /* context */
-	u8			adapter_shutdown;
+	atomic_t		adapter_shutdown;
 	u32			handle_pci_error;
 };
 
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 0e954e3..d361673 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
 		return -ENOMEM;
 	aac_fib_init(fibctx);
 
-	cmd = (struct aac_close *) fib_data(fibctx);
+	atomic_set(&dev->adapter_shutdown, 1);
 
+	cmd = (struct aac_close *) fib_data(fibctx);
 	cmd->command = cpu_to_le32(VM_CloseAll);
 	cmd->cid = cpu_to_le32(0xfffffffe);
 
@@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
 	/* FIB should be freed only after getting the response from the F/W */
 	if (status != -ERESTARTSYS)
 		aac_fib_free(fibctx);
-	dev->adapter_shutdown = 1;
 	if ((dev->pdev->device == PMC_DEVICE_S7 ||
 	     dev->pdev->device == PMC_DEVICE_S8 ||
 	     dev->pdev->device == PMC_DEVICE_S9) &&
@@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
 	}
 	dev->max_msix = 0;
 	dev->msi_enabled = 0;
-	dev->adapter_shutdown = 0;
+	atomic_set(&dev->adapter_shutdown, 0);
 	if ((!aac_adapter_sync_cmd(dev, GET_COMM_PREFERRED_SETTINGS,
 	  0, 0, 0, 0, 0, 0,
 	  status+0, status+1, status+2, status+3, status+4))
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 6944560..27b3fcd 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
 	int ret;
 	struct aac_dev *aac;
 	aac = (struct aac_dev *)file->private_data;
-	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
+	if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac->adapter_shutdown))
 		return -EPERM;
 	mutex_lock(&aac_mutex);
 	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
@@ -1078,6 +1078,8 @@ static void __aac_shutdown(struct aac_dev * aac)
 	int i;
 	int cpu;
 
+	aac_send_shutdown(aac);
+
 	if (aac->aif_thread) {
 		int i;
 		/* Clear out events first */
@@ -1089,7 +1091,7 @@ static void __aac_shutdown(struct aac_dev * aac)
 		}
 		kthread_stop(aac->thread);
 	}
-	aac_send_shutdown(aac);
+
 	aac_adapter_disable_int(aac);
 	cpu = cpumask_first(cpu_online_mask);
 	if (aac->pdev->device == PMC_DEVICE_S6 ||
@@ -1474,7 +1476,7 @@ static int aac_resume(struct pci_dev *pdev)
 	* reset this flag to unblock ioctl() as it was set at
 	* aac_send_shutdown() to block ioctls from upperlayer
 	*/
-	aac->adapter_shutdown = 0;
+	atomic_set(&aac->adapter_shutdown, 0);
 	scsi_unblock_requests(shost);
 
 	return 0;
@@ -1553,9 +1555,8 @@ static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
 	case pci_channel_io_normal:
 		return PCI_ERS_RESULT_CAN_RECOVER;
 	case pci_channel_io_frozen:
-
 		aac->handle_pci_error = 1;
-		aac->adapter_shutdown = 1;
+		atomic_set(&aac->adapter_shutdown, 1);
 
 		scsi_block_requests(aac->scsi_host_ptr);
 		aac_flush_ios(aac);
@@ -1567,7 +1568,7 @@ static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
 		return PCI_ERS_RESULT_NEED_RESET;
 	case pci_channel_io_perm_failure:
 		aac->handle_pci_error = 1;
-		aac->adapter_shutdown = 1;
+		atomic_set(&aac->adapter_shutdown, 1);
 
 		aac_flush_ios(aac);
 		return PCI_ERS_RESULT_DISCONNECT;
@@ -1636,7 +1637,7 @@ static void aac_pci_resume(struct pci_dev *pdev)
 	 * reset this flag to unblock ioctl() as it was set
 	 * at aac_send_shutdown() to block ioctls from upperlayer
 	 */
-	aac->adapter_shutdown = 0;
+	atomic_set(&aac->adapter_shutdown, 0);
 	aac->handle_pci_error = 0;
 
 	shost_for_each_device(sdev, shost)
-- 
1.9.1

RE: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Seymour, Shane M]

Reviewed-by: Shane Seymour <shane.seymour@hpe.com>

Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Johannes Thumshirn]

On Thu, Jan 14, 2016 at 11:16:20PM -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> 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.
> 
> Changes in V2:
> Set adapter_shutdown flag before shutdown command is sent to \
> controller
> 
> Changes in V3:
> Call aac_send_shut_shutdown first thing in __aac_shutdown
> Convert adapter_shutdown to atomic_t variable to prevent \
> SMP coherency issues(race conditions)
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aacraid.h  |  2 +-
>  drivers/scsi/aacraid/comminit.c |  6 +++---
>  drivers/scsi/aacraid/linit.c    | 15 ++++++++-------
>  3 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 2916288..3473668 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -1234,7 +1234,7 @@ struct aac_dev
>  	int			msi_enabled;	/* MSI/MSI-X enabled */
>  	struct msix_entry	msixentry[AAC_MAX_MSIX];
>  	struct aac_msix_ctx	aac_msix[AAC_MAX_MSIX]; /* context */
> -	u8			adapter_shutdown;
> +	atomic_t		adapter_shutdown;
>  	u32			handle_pci_error;
>  };
>  
> diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
> index 0e954e3..d361673 100644
> --- a/drivers/scsi/aacraid/comminit.c
> +++ b/drivers/scsi/aacraid/comminit.c
> @@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
>  		return -ENOMEM;
>  	aac_fib_init(fibctx);
>  
> -	cmd = (struct aac_close *) fib_data(fibctx);
> +	atomic_set(&dev->adapter_shutdown, 1);
>  
> +	cmd = (struct aac_close *) fib_data(fibctx);
>  	cmd->command = cpu_to_le32(VM_CloseAll);
>  	cmd->cid = cpu_to_le32(0xfffffffe);
>  
> @@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
>  	/* FIB should be freed only after getting the response from the F/W */
>  	if (status != -ERESTARTSYS)
>  		aac_fib_free(fibctx);
> -	dev->adapter_shutdown = 1;
>  	if ((dev->pdev->device == PMC_DEVICE_S7 ||
>  	     dev->pdev->device == PMC_DEVICE_S8 ||
>  	     dev->pdev->device == PMC_DEVICE_S9) &&
> @@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
>  	}
>  	dev->max_msix = 0;
>  	dev->msi_enabled = 0;
> -	dev->adapter_shutdown = 0;
> +	atomic_set(&dev->adapter_shutdown, 0);
>  	if ((!aac_adapter_sync_cmd(dev, GET_COMM_PREFERRED_SETTINGS,
>  	  0, 0, 0, 0, 0, 0,
>  	  status+0, status+1, status+2, status+3, status+4))
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 6944560..27b3fcd 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
>  	int ret;
>  	struct aac_dev *aac;
>  	aac = (struct aac_dev *)file->private_data;
> -	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> +	if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac->adapter_shutdown))
>  		return -EPERM;
>  	mutex_lock(&aac_mutex);
>  	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> @@ -1078,6 +1078,8 @@ static void __aac_shutdown(struct aac_dev * aac)
>  	int i;
>  	int cpu;
>  
> +	aac_send_shutdown(aac);
> +
>  	if (aac->aif_thread) {
>  		int i;
>  		/* Clear out events first */
> @@ -1089,7 +1091,7 @@ static void __aac_shutdown(struct aac_dev * aac)
>  		}
>  		kthread_stop(aac->thread);
>  	}
> -	aac_send_shutdown(aac);
> +
>  	aac_adapter_disable_int(aac);
>  	cpu = cpumask_first(cpu_online_mask);
>  	if (aac->pdev->device == PMC_DEVICE_S6 ||
> @@ -1474,7 +1476,7 @@ static int aac_resume(struct pci_dev *pdev)
>  	* reset this flag to unblock ioctl() as it was set at
>  	* aac_send_shutdown() to block ioctls from upperlayer
>  	*/
> -	aac->adapter_shutdown = 0;
> +	atomic_set(&aac->adapter_shutdown, 0);
>  	scsi_unblock_requests(shost);
>  
>  	return 0;
> @@ -1553,9 +1555,8 @@ static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
>  	case pci_channel_io_normal:
>  		return PCI_ERS_RESULT_CAN_RECOVER;
>  	case pci_channel_io_frozen:
> -
>  		aac->handle_pci_error = 1;
> -		aac->adapter_shutdown = 1;
> +		atomic_set(&aac->adapter_shutdown, 1);
>  
>  		scsi_block_requests(aac->scsi_host_ptr);
>  		aac_flush_ios(aac);
> @@ -1567,7 +1568,7 @@ static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
>  		return PCI_ERS_RESULT_NEED_RESET;
>  	case pci_channel_io_perm_failure:
>  		aac->handle_pci_error = 1;
> -		aac->adapter_shutdown = 1;
> +		atomic_set(&aac->adapter_shutdown, 1);
>  
>  		aac_flush_ios(aac);
>  		return PCI_ERS_RESULT_DISCONNECT;
> @@ -1636,7 +1637,7 @@ static void aac_pci_resume(struct pci_dev *pdev)
>  	 * reset this flag to unblock ioctl() as it was set
>  	 * at aac_send_shutdown() to block ioctls from upperlayer
>  	 */
> -	aac->adapter_shutdown = 0;
> +	atomic_set(&aac->adapter_shutdown, 0);
>  	aac->handle_pci_error = 0;
>  
>  	shost_for_each_device(sdev, shost)
> -- 
> 1.9.1
> 

Looks good.

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Tomas Henzl]

On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>
> 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.
>
> Changes in V2:
> Set adapter_shutdown flag before shutdown command is sent to \
> controller
>
> Changes in V3:
> Call aac_send_shut_shutdown first thing in __aac_shutdown
> Convert adapter_shutdown to atomic_t variable to prevent \
> SMP coherency issues(race conditions)

Hi,
I don't think that an atomic variable can change it, imagine that
thread just passed your test in aac_cfg_ioctl and another thread
enter a bit later the adapter_shutdown so both - an I/O and your shutdown code
will run in parallel.
Also you need to fix your compat_ioctl path too.

--tm

>
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aacraid.h  |  2 +-
>  drivers/scsi/aacraid/comminit.c |  6 +++---
>  drivers/scsi/aacraid/linit.c    | 15 ++++++++-------
>  3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 2916288..3473668 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -1234,7 +1234,7 @@ struct aac_dev
>  	int			msi_enabled;	/* MSI/MSI-X enabled */
>  	struct msix_entry	msixentry[AAC_MAX_MSIX];
>  	struct aac_msix_ctx	aac_msix[AAC_MAX_MSIX]; /* context */
> -	u8			adapter_shutdown;
> +	atomic_t		adapter_shutdown;
>  	u32			handle_pci_error;
>  };
>  
> diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
> index 0e954e3..d361673 100644
> --- a/drivers/scsi/aacraid/comminit.c
> +++ b/drivers/scsi/aacraid/comminit.c
> @@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
>  		return -ENOMEM;
>  	aac_fib_init(fibctx);
>  
> -	cmd = (struct aac_close *) fib_data(fibctx);
> +	atomic_set(&dev->adapter_shutdown, 1);
>  
> +	cmd = (struct aac_close *) fib_data(fibctx);
>  	cmd->command = cpu_to_le32(VM_CloseAll);
>  	cmd->cid = cpu_to_le32(0xfffffffe);
>  
> @@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
>  	/* FIB should be freed only after getting the response from the F/W */
>  	if (status != -ERESTARTSYS)
>  		aac_fib_free(fibctx);
> -	dev->adapter_shutdown = 1;
>  	if ((dev->pdev->device == PMC_DEVICE_S7 ||
>  	     dev->pdev->device == PMC_DEVICE_S8 ||
>  	     dev->pdev->device == PMC_DEVICE_S9) &&
> @@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
>  	}
>  	dev->max_msix = 0;
>  	dev->msi_enabled = 0;
> -	dev->adapter_shutdown = 0;
> +	atomic_set(&dev->adapter_shutdown, 0);
>  	if ((!aac_adapter_sync_cmd(dev, GET_COMM_PREFERRED_SETTINGS,
>  	  0, 0, 0, 0, 0, 0,
>  	  status+0, status+1, status+2, status+3, status+4))
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 6944560..27b3fcd 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
>  	int ret;
>  	struct aac_dev *aac;
>  	aac = (struct aac_dev *)file->private_data;
> -	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> +	if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac->adapter_shutdown))
>  		return -EPERM;
>  	mutex_lock(&aac_mutex);
>  	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> @@ -1078,6 +1078,8 @@ static void __aac_shutdown(struct aac_dev * aac)
>  	int i;
>  	int cpu;
>  
> +	aac_send_shutdown(aac);
> +
>  	if (aac->aif_thread) {
>  		int i;
>  		/* Clear out events first */
> @@ -1089,7 +1091,7 @@ static void __aac_shutdown(struct aac_dev * aac)
>  		}
>  		kthread_stop(aac->thread);
>  	}
> -	aac_send_shutdown(aac);
> +
>  	aac_adapter_disable_int(aac);
>  	cpu = cpumask_first(cpu_online_mask);
>  	if (aac->pdev->device == PMC_DEVICE_S6 ||
> @@ -1474,7 +1476,7 @@ static int aac_resume(struct pci_dev *pdev)
>  	* reset this flag to unblock ioctl() as it was set at
>  	* aac_send_shutdown() to block ioctls from upperlayer
>  	*/
> -	aac->adapter_shutdown = 0;
> +	atomic_set(&aac->adapter_shutdown, 0);
>  	scsi_unblock_requests(shost);
>  
>  	return 0;
> @@ -1553,9 +1555,8 @@ static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
>  	case pci_channel_io_normal:
>  		return PCI_ERS_RESULT_CAN_RECOVER;
>  	case pci_channel_io_frozen:
> -
>  		aac->handle_pci_error = 1;
> -		aac->adapter_shutdown = 1;
> +		atomic_set(&aac->adapter_shutdown, 1);
>  
>  		scsi_block_requests(aac->scsi_host_ptr);
>  		aac_flush_ios(aac);
> @@ -1567,7 +1568,7 @@ static pci_ers_result_t aac_pci_error_detected(struct pci_dev *pdev,
>  		return PCI_ERS_RESULT_NEED_RESET;
>  	case pci_channel_io_perm_failure:
>  		aac->handle_pci_error = 1;
> -		aac->adapter_shutdown = 1;
> +		atomic_set(&aac->adapter_shutdown, 1);
>  
>  		aac_flush_ios(aac);
>  		return PCI_ERS_RESULT_DISCONNECT;
> @@ -1636,7 +1637,7 @@ static void aac_pci_resume(struct pci_dev *pdev)
>  	 * reset this flag to unblock ioctl() as it was set
>  	 * at aac_send_shutdown() to block ioctls from upperlayer
>  	 */
> -	aac->adapter_shutdown = 0;
> +	atomic_set(&aac->adapter_shutdown, 0);
>  	aac->handle_pci_error = 0;
>  
>  	shost_for_each_device(sdev, shost)

RE: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Raghava Aditya Renukunta]

> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Tuesday, January 19, 2016 8:14 AM
> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com;
> David Carroll
> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
> 
> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> >
> > 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.
> >
> > Changes in V2:
> > Set adapter_shutdown flag before shutdown command is sent to \
> > controller
> >
> > Changes in V3:
> > Call aac_send_shut_shutdown first thing in __aac_shutdown
> > Convert adapter_shutdown to atomic_t variable to prevent \
> > SMP coherency issues(race conditions)
> 
> Hi,
> I don't think that an atomic variable can change it, imagine that
> thread just passed your test in aac_cfg_ioctl and another thread
> enter a bit later the adapter_shutdown so both - an I/O and your shutdown
> code
> will run in parallel.
> Also you need to fix your compat_ioctl path too.
> 
> --tm

Hello Tomas,
Yes that would not solve this problem.
I can think of 2 solutions

1.Place a delay after setting adapter_shutdown and before sending the actual
shutdown command in aac_send_shutdown. This way any impending commands will be 
processed before the adapter  actually receives the shutdown command. Since management 
commands are sync only , shutdown command will be sent last. This solution uses an
estimation of the delay

2.Since  commands are sync , place a check in aac_fib_send to block 
 commands once adapter_shutdown is set(only shutdown command will be sent thru)

I am more inclined to go with option 2.

Regards,
Raghava Aditya

> >
> > Signed-off-by: Raghava Aditya Renukunta
> <raghavaaditya.renukunta@pmcs.com>
> > ---
> >  drivers/scsi/aacraid/aacraid.h  |  2 +-
> >  drivers/scsi/aacraid/comminit.c |  6 +++---
> >  drivers/scsi/aacraid/linit.c    | 15 ++++++++-------
> >  3 files changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 2916288..3473668 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -1234,7 +1234,7 @@ struct aac_dev
> >  	int			msi_enabled;	/* MSI/MSI-X enabled */
> >  	struct msix_entry	msixentry[AAC_MAX_MSIX];
> >  	struct aac_msix_ctx	aac_msix[AAC_MAX_MSIX]; /* context */
> > -	u8			adapter_shutdown;
> > +	atomic_t		adapter_shutdown;
> >  	u32			handle_pci_error;
> >  };
> >
> > diff --git a/drivers/scsi/aacraid/comminit.c
> b/drivers/scsi/aacraid/comminit.c
> > index 0e954e3..d361673 100644
> > --- a/drivers/scsi/aacraid/comminit.c
> > +++ b/drivers/scsi/aacraid/comminit.c
> > @@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
> >  		return -ENOMEM;
> >  	aac_fib_init(fibctx);
> >
> > -	cmd = (struct aac_close *) fib_data(fibctx);
> > +	atomic_set(&dev->adapter_shutdown, 1);
> >
> > +	cmd = (struct aac_close *) fib_data(fibctx);
> >  	cmd->command = cpu_to_le32(VM_CloseAll);
> >  	cmd->cid = cpu_to_le32(0xfffffffe);
> >
> > @@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
> >  	/* FIB should be freed only after getting the response from the F/W
> */
> >  	if (status != -ERESTARTSYS)
> >  		aac_fib_free(fibctx);
> > -	dev->adapter_shutdown = 1;
> >  	if ((dev->pdev->device == PMC_DEVICE_S7 ||
> >  	     dev->pdev->device == PMC_DEVICE_S8 ||
> >  	     dev->pdev->device == PMC_DEVICE_S9) &&
> > @@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev
> *dev)
> >  	}
> >  	dev->max_msix = 0;
> >  	dev->msi_enabled = 0;
> > -	dev->adapter_shutdown = 0;
> > +	atomic_set(&dev->adapter_shutdown, 0);
> >  	if ((!aac_adapter_sync_cmd(dev,
> GET_COMM_PREFERRED_SETTINGS,
> >  	  0, 0, 0, 0, 0, 0,
> >  	  status+0, status+1, status+2, status+3, status+4))
> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index 6944560..27b3fcd 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
> >  	int ret;
> >  	struct aac_dev *aac;
> >  	aac = (struct aac_dev *)file->private_data;
> > -	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> > +	if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac-
> >adapter_shutdown))
> >  		return -EPERM;
> >  	mutex_lock(&aac_mutex);
> >  	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> > @@ -1078,6 +1078,8 @@ static void __aac_shutdown(struct aac_dev * aac)
> >  	int i;
> >  	int cpu;
> >
> > +	aac_send_shutdown(aac);
> > +
> >  	if (aac->aif_thread) {
> >  		int i;
> >  		/* Clear out events first */
> > @@ -1089,7 +1091,7 @@ static void __aac_shutdown(struct aac_dev * aac)
> >  		}
> >  		kthread_stop(aac->thread);
> >  	}
> > -	aac_send_shutdown(aac);
> > +
> >  	aac_adapter_disable_int(aac);
> >  	cpu = cpumask_first(cpu_online_mask);
> >  	if (aac->pdev->device == PMC_DEVICE_S6 ||
> > @@ -1474,7 +1476,7 @@ static int aac_resume(struct pci_dev *pdev)
> >  	* reset this flag to unblock ioctl() as it was set at
> >  	* aac_send_shutdown() to block ioctls from upperlayer
> >  	*/
> > -	aac->adapter_shutdown = 0;
> > +	atomic_set(&aac->adapter_shutdown, 0);
> >  	scsi_unblock_requests(shost);
> >
> >  	return 0;
> > @@ -1553,9 +1555,8 @@ static pci_ers_result_t
> aac_pci_error_detected(struct pci_dev *pdev,
> >  	case pci_channel_io_normal:
> >  		return PCI_ERS_RESULT_CAN_RECOVER;
> >  	case pci_channel_io_frozen:
> > -
> >  		aac->handle_pci_error = 1;
> > -		aac->adapter_shutdown = 1;
> > +		atomic_set(&aac->adapter_shutdown, 1);
> >
> >  		scsi_block_requests(aac->scsi_host_ptr);
> >  		aac_flush_ios(aac);
> > @@ -1567,7 +1568,7 @@ static pci_ers_result_t
> aac_pci_error_detected(struct pci_dev *pdev,
> >  		return PCI_ERS_RESULT_NEED_RESET;
> >  	case pci_channel_io_perm_failure:
> >  		aac->handle_pci_error = 1;
> > -		aac->adapter_shutdown = 1;
> > +		atomic_set(&aac->adapter_shutdown, 1);
> >
> >  		aac_flush_ios(aac);
> >  		return PCI_ERS_RESULT_DISCONNECT;
> > @@ -1636,7 +1637,7 @@ static void aac_pci_resume(struct pci_dev *pdev)
> >  	 * reset this flag to unblock ioctl() as it was set
> >  	 * at aac_send_shutdown() to block ioctls from upperlayer
> >  	 */
> > -	aac->adapter_shutdown = 0;
> > +	atomic_set(&aac->adapter_shutdown, 0);
> >  	aac->handle_pci_error = 0;
> >
> >  	shost_for_each_device(sdev, shost)

Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Tomas Henzl]

On 20.1.2016 21:32, Raghava Aditya Renukunta wrote:
>
>> -----Original Message-----
>> From: Tomas Henzl [mailto:thenzl@redhat.com]
>> Sent: Tuesday, January 19, 2016 8:14 AM
>> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
>> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
>> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
>> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com;
>> David Carroll
>> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
>>
>> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
>>> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>>>
>>> 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.
>>>
>>> Changes in V2:
>>> Set adapter_shutdown flag before shutdown command is sent to \
>>> controller
>>>
>>> Changes in V3:
>>> Call aac_send_shut_shutdown first thing in __aac_shutdown
>>> Convert adapter_shutdown to atomic_t variable to prevent \
>>> SMP coherency issues(race conditions)
>> Hi,
>> I don't think that an atomic variable can change it, imagine that
>> thread just passed your test in aac_cfg_ioctl and another thread
>> enter a bit later the adapter_shutdown so both - an I/O and your shutdown
>> code
>> will run in parallel.
>> Also you need to fix your compat_ioctl path too.
>>
>> --tm
> Hello Tomas,
> Yes that would not solve this problem.
> I can think of 2 solutions
>
> 1.Place a delay after setting adapter_shutdown and before sending the actual
> shutdown command in aac_send_shutdown. This way any impending commands will be 
> processed before the adapter  actually receives the shutdown command. Since management 
> commands are sync only , shutdown command will be sent last. This solution uses an
> estimation of the delay
>
> 2.Since  commands are sync , place a check in aac_fib_send to block 
>  commands once adapter_shutdown is set(only shutdown command will be sent thru)

This option looks better but I guess you still can find a tiny race window.
What do you think about a mutual exclusive access using a mutex, do you think this could work?

diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 54195a117f..b9505c58db 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -858,6 +858,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void __user *arg)
 	/*
 	 *	HBA gets first crack
 	 */
+	if (dev->adapter_shutdown)
+		return -EACCES;
 
 	status = aac_dev_ioctl(dev, cmd, arg);
 	if (status != -ENOTTY)
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
index 0e954e37f0..02535c07a4 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -212,6 +212,10 @@ int aac_send_shutdown(struct aac_dev * dev)
 		return -ENOMEM;
 	aac_fib_init(fibctx);
 
+	mutex_lock(&aac_mutex);
+	dev->adapter_shutdown = 1;
+	mutex_unlock(&aac_mutex);
+
 	cmd = (struct aac_close *) fib_data(fibctx);
 
 	cmd->command = cpu_to_le32(VM_CloseAll);
@@ -229,7 +233,6 @@ int aac_send_shutdown(struct aac_dev * dev)
 	/* FIB should be freed only after getting the response from the F/W */
 	if (status != -ERESTARTSYS)
 		aac_fib_free(fibctx);
-	dev->adapter_shutdown = 1;
 	if ((dev->pdev->device == PMC_DEVICE_S7 ||
 	     dev->pdev->device == PMC_DEVICE_S8 ||
 	     dev->pdev->device == PMC_DEVICE_S9) &&
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 6944560e22..a87880ab32 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -706,8 +706,9 @@ static long aac_cfg_ioctl(struct file *file,
 	int ret;
 	struct aac_dev *aac;
 	aac = (struct aac_dev *)file->private_data;
-	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
+	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
+
 	mutex_lock(&aac_mutex);
 	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
 	mutex_unlock(&aac_mutex);

>
> I am more inclined to go with option 2.
>
> Regards,
> Raghava Aditya
>
>>> Signed-off-by: Raghava Aditya Renukunta
>> <raghavaaditya.renukunta@pmcs.com>
>>> ---
>>>  drivers/scsi/aacraid/aacraid.h  |  2 +-
>>>  drivers/scsi/aacraid/comminit.c |  6 +++---
>>>  drivers/scsi/aacraid/linit.c    | 15 ++++++++-------
>>>  3 files changed, 12 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
>>> index 2916288..3473668 100644
>>> --- a/drivers/scsi/aacraid/aacraid.h
>>> +++ b/drivers/scsi/aacraid/aacraid.h
>>> @@ -1234,7 +1234,7 @@ struct aac_dev
>>>  	int			msi_enabled;	/* MSI/MSI-X enabled */
>>>  	struct msix_entry	msixentry[AAC_MAX_MSIX];
>>>  	struct aac_msix_ctx	aac_msix[AAC_MAX_MSIX]; /* context */
>>> -	u8			adapter_shutdown;
>>> +	atomic_t		adapter_shutdown;
>>>  	u32			handle_pci_error;
>>>  };
>>>
>>> diff --git a/drivers/scsi/aacraid/comminit.c
>> b/drivers/scsi/aacraid/comminit.c
>>> index 0e954e3..d361673 100644
>>> --- a/drivers/scsi/aacraid/comminit.c
>>> +++ b/drivers/scsi/aacraid/comminit.c
>>> @@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
>>>  		return -ENOMEM;
>>>  	aac_fib_init(fibctx);
>>>
>>> -	cmd = (struct aac_close *) fib_data(fibctx);
>>> +	atomic_set(&dev->adapter_shutdown, 1);
>>>
>>> +	cmd = (struct aac_close *) fib_data(fibctx);
>>>  	cmd->command = cpu_to_le32(VM_CloseAll);
>>>  	cmd->cid = cpu_to_le32(0xfffffffe);
>>>
>>> @@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
>>>  	/* FIB should be freed only after getting the response from the F/W
>> */
>>>  	if (status != -ERESTARTSYS)
>>>  		aac_fib_free(fibctx);
>>> -	dev->adapter_shutdown = 1;
>>>  	if ((dev->pdev->device == PMC_DEVICE_S7 ||
>>>  	     dev->pdev->device == PMC_DEVICE_S8 ||
>>>  	     dev->pdev->device == PMC_DEVICE_S9) &&
>>> @@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev
>> *dev)
>>>  	}
>>>  	dev->max_msix = 0;
>>>  	dev->msi_enabled = 0;
>>> -	dev->adapter_shutdown = 0;
>>> +	atomic_set(&dev->adapter_shutdown, 0);
>>>  	if ((!aac_adapter_sync_cmd(dev,
>> GET_COMM_PREFERRED_SETTINGS,
>>>  	  0, 0, 0, 0, 0, 0,
>>>  	  status+0, status+1, status+2, status+3, status+4))
>>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
>>> index 6944560..27b3fcd 100644
>>> --- a/drivers/scsi/aacraid/linit.c
>>> +++ b/drivers/scsi/aacraid/linit.c
>>> @@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
>>>  	int ret;
>>>  	struct aac_dev *aac;
>>>  	aac = (struct aac_dev *)file->private_data;
>>> -	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
>>> +	if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac-
>>> adapter_shutdown))
>>>  		return -EPERM;
>>>  	mutex_lock(&aac_mutex);
>>>  	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
>>> @@ -1078,6 +1078,8 @@ static void __aac_shutdown(struct aac_dev * aac)
>>>  	int i;
>>>  	int cpu;
>>>
>>> +	aac_send_shutdown(aac);
>>> +
>>>  	if (aac->aif_thread) {
>>>  		int i;
>>>  		/* Clear out events first */
>>> @@ -1089,7 +1091,7 @@ static void __aac_shutdown(struct aac_dev * aac)
>>>  		}
>>>  		kthread_stop(aac->thread);
>>>  	}
>>> -	aac_send_shutdown(aac);
>>> +
>>>  	aac_adapter_disable_int(aac);
>>>  	cpu = cpumask_first(cpu_online_mask);
>>>  	if (aac->pdev->device == PMC_DEVICE_S6 ||
>>> @@ -1474,7 +1476,7 @@ static int aac_resume(struct pci_dev *pdev)
>>>  	* reset this flag to unblock ioctl() as it was set at
>>>  	* aac_send_shutdown() to block ioctls from upperlayer
>>>  	*/
>>> -	aac->adapter_shutdown = 0;
>>> +	atomic_set(&aac->adapter_shutdown, 0);
>>>  	scsi_unblock_requests(shost);
>>>
>>>  	return 0;
>>> @@ -1553,9 +1555,8 @@ static pci_ers_result_t
>> aac_pci_error_detected(struct pci_dev *pdev,
>>>  	case pci_channel_io_normal:
>>>  		return PCI_ERS_RESULT_CAN_RECOVER;
>>>  	case pci_channel_io_frozen:
>>> -
>>>  		aac->handle_pci_error = 1;
>>> -		aac->adapter_shutdown = 1;
>>> +		atomic_set(&aac->adapter_shutdown, 1);
>>>
>>>  		scsi_block_requests(aac->scsi_host_ptr);
>>>  		aac_flush_ios(aac);
>>> @@ -1567,7 +1568,7 @@ static pci_ers_result_t
>> aac_pci_error_detected(struct pci_dev *pdev,
>>>  		return PCI_ERS_RESULT_NEED_RESET;
>>>  	case pci_channel_io_perm_failure:
>>>  		aac->handle_pci_error = 1;
>>> -		aac->adapter_shutdown = 1;
>>> +		atomic_set(&aac->adapter_shutdown, 1);
>>>
>>>  		aac_flush_ios(aac);
>>>  		return PCI_ERS_RESULT_DISCONNECT;
>>> @@ -1636,7 +1637,7 @@ static void aac_pci_resume(struct pci_dev *pdev)
>>>  	 * reset this flag to unblock ioctl() as it was set
>>>  	 * at aac_send_shutdown() to block ioctls from upperlayer
>>>  	 */
>>> -	aac->adapter_shutdown = 0;
>>> +	atomic_set(&aac->adapter_shutdown, 0);
>>>  	aac->handle_pci_error = 0;
>>>
>>>  	shost_for_each_device(sdev, shost)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Raghava Aditya Renukunta]

Hello Tomas,

> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Friday, January 22, 2016 5:15 AM
> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com;
> zzzDavid Carroll
> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
> 
> On 20.1.2016 21:32, Raghava Aditya Renukunta wrote:
> >
> >> -----Original Message-----
> >> From: Tomas Henzl [mailto:thenzl@redhat.com]
> >> Sent: Tuesday, January 19, 2016 8:14 AM
> >> To: Raghava Aditya Renukunta;
> James.Bottomley@HansenPartnership.com;
> >> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> >> sierra.com; Scott Benesh; jthumshirn@suse.de;
> shane.seymour@hpe.com;
> >> David Carroll
> >> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
> >>
> >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> >>> From: Raghava Aditya Renukunta
> <raghavaaditya.renukunta@pmcs.com>
> >>>
> >>> 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.
> >>>
> >>> Changes in V2:
> >>> Set adapter_shutdown flag before shutdown command is sent to \
> >>> controller
> >>>
> >>> Changes in V3:
> >>> Call aac_send_shut_shutdown first thing in __aac_shutdown
> >>> Convert adapter_shutdown to atomic_t variable to prevent \
> >>> SMP coherency issues(race conditions)
> >> Hi,
> >> I don't think that an atomic variable can change it, imagine that
> >> thread just passed your test in aac_cfg_ioctl and another thread
> >> enter a bit later the adapter_shutdown so both - an I/O and your
> shutdown
> >> code
> >> will run in parallel.
> >> Also you need to fix your compat_ioctl path too.
> >>
> >> --tm
> > Hello Tomas,
> > Yes that would not solve this problem.
> > I can think of 2 solutions
> >
> > 1.Place a delay after setting adapter_shutdown and before sending the
> actual
> > shutdown command in aac_send_shutdown. This way any impending
> commands will be
> > processed before the adapter  actually receives the shutdown command.
> Since management
> > commands are sync only , shutdown command will be sent last. This
> solution uses an
> > estimation of the delay
> >
> > 2.Since  commands are sync , place a check in aac_fib_send to block
> >  commands once adapter_shutdown is set(only shutdown command will
> be sent thru)
> 
> This option looks better but I guess you still can find a tiny race window.
> What do you think about a mutual exclusive access using a mutex, do you
> think this could work?
> diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
> index 54195a117f..b9505c58db 100644
> --- a/drivers/scsi/aacraid/commctrl.c
> +++ b/drivers/scsi/aacraid/commctrl.c
> @@ -858,6 +858,8 @@ int aac_do_ioctl(struct aac_dev * dev, int cmd, void
> __user *arg)
>  	/*
>  	 *	HBA gets first crack
>  	 */
> +	if (dev->adapter_shutdown)
> +		return -EACCES;
> 
>  	status = aac_dev_ioctl(dev, cmd, arg);
>  	if (status != -ENOTTY)
> diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c
> index 0e954e37f0..02535c07a4 100644
> --- a/drivers/scsi/aacraid/comminit.c
> +++ b/drivers/scsi/aacraid/comminit.c
> @@ -212,6 +212,10 @@ int aac_send_shutdown(struct aac_dev * dev)
>  		return -ENOMEM;
>  	aac_fib_init(fibctx);
> 
> +	mutex_lock(&aac_mutex);
> +	dev->adapter_shutdown = 1;
> +	mutex_unlock(&aac_mutex);
> +
 
The  intention here is to have mutually exclusive access to adapter_shutdown, or
aac_do_Ioctl  which has a check for adapter_shutdown so  either one can be
 command is sent at one time if I am not mistaken?.
 Yes this solution will work. I will make the necessary changes. 
Thank you.

Regards,
Raghava Aditya

 
>  	cmd = (struct aac_close *) fib_data(fibctx);
> 
>  	cmd->command = cpu_to_le32(VM_CloseAll);
> @@ -229,7 +233,6 @@ int aac_send_shutdown(struct aac_dev * dev)
>  	/* FIB should be freed only after getting the response from the F/W
> */
>  	if (status != -ERESTARTSYS)
>  		aac_fib_free(fibctx);
> -	dev->adapter_shutdown = 1;
>  	if ((dev->pdev->device == PMC_DEVICE_S7 ||
>  	     dev->pdev->device == PMC_DEVICE_S8 ||
>  	     dev->pdev->device == PMC_DEVICE_S9) &&
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 6944560e22..a87880ab32 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -706,8 +706,9 @@ static long aac_cfg_ioctl(struct file *file,
>  	int ret;
>  	struct aac_dev *aac;
>  	aac = (struct aac_dev *)file->private_data;
> -	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> +	if (!capable(CAP_SYS_RAWIO))
>  		return -EPERM;
> +
>  	mutex_lock(&aac_mutex);
>  	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
>  	mutex_unlock(&aac_mutex);
> 
> >
> > I am more inclined to go with option 2.
> >
> > Regards,
> > Raghava Aditya
> >
> >>> Signed-off-by: Raghava Aditya Renukunta
> >> <raghavaaditya.renukunta@pmcs.com>
> >>> ---
> >>>  drivers/scsi/aacraid/aacraid.h  |  2 +-
> >>>  drivers/scsi/aacraid/comminit.c |  6 +++---
> >>>  drivers/scsi/aacraid/linit.c    | 15 ++++++++-------
> >>>  3 files changed, 12 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> >>> index 2916288..3473668 100644
> >>> --- a/drivers/scsi/aacraid/aacraid.h
> >>> +++ b/drivers/scsi/aacraid/aacraid.h
> >>> @@ -1234,7 +1234,7 @@ struct aac_dev
> >>>  	int			msi_enabled;	/* MSI/MSI-X enabled */
> >>>  	struct msix_entry	msixentry[AAC_MAX_MSIX];
> >>>  	struct aac_msix_ctx	aac_msix[AAC_MAX_MSIX]; /* context */
> >>> -	u8			adapter_shutdown;
> >>> +	atomic_t		adapter_shutdown;
> >>>  	u32			handle_pci_error;
> >>>  };
> >>>
> >>> diff --git a/drivers/scsi/aacraid/comminit.c
> >> b/drivers/scsi/aacraid/comminit.c
> >>> index 0e954e3..d361673 100644
> >>> --- a/drivers/scsi/aacraid/comminit.c
> >>> +++ b/drivers/scsi/aacraid/comminit.c
> >>> @@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
> >>>  		return -ENOMEM;
> >>>  	aac_fib_init(fibctx);
> >>>
> >>> -	cmd = (struct aac_close *) fib_data(fibctx);
> >>> +	atomic_set(&dev->adapter_shutdown, 1);
> >>>
> >>> +	cmd = (struct aac_close *) fib_data(fibctx);
> >>>  	cmd->command = cpu_to_le32(VM_CloseAll);
> >>>  	cmd->cid = cpu_to_le32(0xfffffffe);
> >>>
> >>> @@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
> >>>  	/* FIB should be freed only after getting the response from the F/W
> >> */
> >>>  	if (status != -ERESTARTSYS)
> >>>  		aac_fib_free(fibctx);
> >>> -	dev->adapter_shutdown = 1;
> >>>  	if ((dev->pdev->device == PMC_DEVICE_S7 ||
> >>>  	     dev->pdev->device == PMC_DEVICE_S8 ||
> >>>  	     dev->pdev->device == PMC_DEVICE_S9) &&
> >>> @@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev
> >> *dev)
> >>>  	}
> >>>  	dev->max_msix = 0;
> >>>  	dev->msi_enabled = 0;
> >>> -	dev->adapter_shutdown = 0;
> >>> +	atomic_set(&dev->adapter_shutdown, 0);
> >>>  	if ((!aac_adapter_sync_cmd(dev,
> >> GET_COMM_PREFERRED_SETTINGS,
> >>>  	  0, 0, 0, 0, 0, 0,
> >>>  	  status+0, status+1, status+2, status+3, status+4))
> >>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> >>> index 6944560..27b3fcd 100644
> >>> --- a/drivers/scsi/aacraid/linit.c
> >>> +++ b/drivers/scsi/aacraid/linit.c
> >>> @@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
> >>>  	int ret;
> >>>  	struct aac_dev *aac;
> >>>  	aac = (struct aac_dev *)file->private_data;
> >>> -	if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> >>> +	if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac-
> >>> adapter_shutdown))
> >>>  		return -EPERM;
> >>>  	mutex_lock(&aac_mutex);
> >>>  	ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> >>> @@ -1078,6 +1078,8 @@ static void __aac_shutdown(struct aac_dev *
> aac)
> >>>  	int i;
> >>>  	int cpu;
> >>>
> >>> +	aac_send_shutdown(aac);
> >>> +
> >>>  	if (aac->aif_thread) {
> >>>  		int i;
> >>>  		/* Clear out events first */
> >>> @@ -1089,7 +1091,7 @@ static void __aac_shutdown(struct aac_dev *
> aac)
> >>>  		}
> >>>  		kthread_stop(aac->thread);
> >>>  	}
> >>> -	aac_send_shutdown(aac);
> >>> +
> >>>  	aac_adapter_disable_int(aac);
> >>>  	cpu = cpumask_first(cpu_online_mask);
> >>>  	if (aac->pdev->device == PMC_DEVICE_S6 ||
> >>> @@ -1474,7 +1476,7 @@ static int aac_resume(struct pci_dev *pdev)
> >>>  	* reset this flag to unblock ioctl() as it was set at
> >>>  	* aac_send_shutdown() to block ioctls from upperlayer
> >>>  	*/
> >>> -	aac->adapter_shutdown = 0;
> >>> +	atomic_set(&aac->adapter_shutdown, 0);
> >>>  	scsi_unblock_requests(shost);
> >>>
> >>>  	return 0;
> >>> @@ -1553,9 +1555,8 @@ static pci_ers_result_t
> >> aac_pci_error_detected(struct pci_dev *pdev,
> >>>  	case pci_channel_io_normal:
> >>>  		return PCI_ERS_RESULT_CAN_RECOVER;
> >>>  	case pci_channel_io_frozen:
> >>> -
> >>>  		aac->handle_pci_error = 1;
> >>> -		aac->adapter_shutdown = 1;
> >>> +		atomic_set(&aac->adapter_shutdown, 1);
> >>>
> >>>  		scsi_block_requests(aac->scsi_host_ptr);
> >>>  		aac_flush_ios(aac);
> >>> @@ -1567,7 +1568,7 @@ static pci_ers_result_t
> >> aac_pci_error_detected(struct pci_dev *pdev,
> >>>  		return PCI_ERS_RESULT_NEED_RESET;
> >>>  	case pci_channel_io_perm_failure:
> >>>  		aac->handle_pci_error = 1;
> >>> -		aac->adapter_shutdown = 1;
> >>> +		atomic_set(&aac->adapter_shutdown, 1);
> >>>
> >>>  		aac_flush_ios(aac);
> >>>  		return PCI_ERS_RESULT_DISCONNECT;
> >>> @@ -1636,7 +1637,7 @@ static void aac_pci_resume(struct pci_dev
> *pdev)
> >>>  	 * reset this flag to unblock ioctl() as it was set
> >>>  	 * at aac_send_shutdown() to block ioctls from upperlayer
> >>>  	 */
> >>> -	aac->adapter_shutdown = 0;
> >>> +	atomic_set(&aac->adapter_shutdown, 0);
> >>>  	aac->handle_pci_error = 0;
> >>>
> >>>  	shost_for_each_device(sdev, shost)
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Martin K. Petersen]

>>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:

>> 2.Since commands are sync , place a check in aac_fib_send to block
>> commands once adapter_shutdown is set(only shutdown command will be
>> sent thru)

Tomas> This option looks better but I guess you still can find a tiny
Tomas> race window.  What do you think about a mutual exclusive access
Tomas> using a mutex, do you think this could work?

[...]

Raghava?

-- 
Martin K. Petersen	Oracle Linux Engineering

RE: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Raghava Aditya Renukunta]

Hello Martin,

> -----Original Message-----
> From: Martin K. Petersen [mailto:martin.petersen@oracle.com]
> Sent: Tuesday, January 26, 2016 6:31 PM
> To: Raghava Aditya Renukunta
> Cc: Tomas Henzl; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org; Mahesh
> Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-sierra.com; Scott
> Benesh; jthumshirn@suse.de; shane.seymour@hpe.com; zzzDavid Carroll
> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
> 
> >>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:
> 
> >> 2.Since commands are sync , place a check in aac_fib_send to block
> >> commands once adapter_shutdown is set(only shutdown command will
> be
> >> sent thru)
> 
> Tomas> This option looks better but I guess you still can find a tiny
> Tomas> race window.  What do you think about a mutual exclusive access
> Tomas> using a mutex, do you think this could work?
> 
> [...]
> 
> Raghava?

I have replied that I will create a new set patches with this change shortly. 
Please expect it by tomorrow at the latest.

Regards,
Raghava Aditya

> 
> --
> Martin K. Petersen	Oracle Linux Engineering

[PATCH V3 8/9] aacraid: Fix character device re-initialization

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

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.

Changes in V2:
Added macros to track character device state

Changes in V3:
None

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
---
 drivers/scsi/aacraid/aacraid.h |  7 +++++++
 drivers/scsi/aacraid/linit.c   | 21 ++++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 3473668..4b669ef 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -94,6 +94,13 @@ enum {
 #define aac_phys_to_logical(x)  ((x)+1)
 #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
 
+/*
+ * These macros are for keeping track of
+ * character device state.
+ */
+#define AAC_CHARDEV_UNREGISTERED	(-1)
+#define AAC_CHARDEV_NEEDS_REINIT	(-2)
+
 /* #define AAC_DETAILED_STATUS_INFO */
 
 struct diskparm
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 27b3fcd..057c07c 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
 
 static DEFINE_MUTEX(aac_mutex);
 static LIST_HEAD(aac_devices);
-static int aac_cfg_major = -1;
+static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
 char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
 
 /*
@@ -1125,6 +1125,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)
 {
@@ -1182,6 +1189,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 == AAC_CHARDEV_NEEDS_REINIT)
+		aac_init_char();
+
 	aac = (struct aac_dev *)shost->hostdata;
 	aac->base_start = pci_resource_start(pdev, 0);
 	aac->scsi_host_ptr = shost;
@@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev *pdev)
 	pci_disable_device(pdev);
 	if (list_empty(&aac_devices)) {
 		unregister_chrdev(aac_cfg_major, "aac");
-		aac_cfg_major = -1;
+		aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
 	}
 }
 
@@ -1681,11 +1691,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();
+
 
 	return 0;
 }
-- 
1.9.1

Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization

[Johannes Thumshirn]

On Thu, Jan 14, 2016 at 11:16:21PM -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> 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.
> 
> Changes in V2:
> Added macros to track character device state
> 
> Changes in V3:
> None
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
> ---
>  drivers/scsi/aacraid/aacraid.h |  7 +++++++
>  drivers/scsi/aacraid/linit.c   | 21 ++++++++++++++-------
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 3473668..4b669ef 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -94,6 +94,13 @@ enum {
>  #define aac_phys_to_logical(x)  ((x)+1)
>  #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
>  
> +/*
> + * These macros are for keeping track of
> + * character device state.
> + */
> +#define AAC_CHARDEV_UNREGISTERED	(-1)
> +#define AAC_CHARDEV_NEEDS_REINIT	(-2)
> +
>  /* #define AAC_DETAILED_STATUS_INFO */
>  
>  struct diskparm
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 27b3fcd..057c07c 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
>  
>  static DEFINE_MUTEX(aac_mutex);
>  static LIST_HEAD(aac_devices);
> -static int aac_cfg_major = -1;
> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
>  char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
>  
>  /*
> @@ -1125,6 +1125,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)
>  {
> @@ -1182,6 +1189,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 == AAC_CHARDEV_NEEDS_REINIT)
> +		aac_init_char();
> +
>  	aac = (struct aac_dev *)shost->hostdata;
>  	aac->base_start = pci_resource_start(pdev, 0);
>  	aac->scsi_host_ptr = shost;
> @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev *pdev)
>  	pci_disable_device(pdev);
>  	if (list_empty(&aac_devices)) {
>  		unregister_chrdev(aac_cfg_major, "aac");
> -		aac_cfg_major = -1;
> +		aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
>  	}
>  }
>  
> @@ -1681,11 +1691,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();
> +
>  
>  	return 0;
>  }
> -- 
> 1.9.1
> 

Looks good

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization

[Tomas Henzl]

On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>
> 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.
>
> Changes in V2:
> Added macros to track character device state
>
> Changes in V3:
> None
>
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> Reviewed-by: Shane Seymour <shane.seymour@hpe.com>

Hi Raghava,
when aacraid is loaded (modprobe) without an controller attached to the system
the driver loads and creates the character device. Later when you hotplug a
device and remove again we see the driver loaded but now without the
char device. I'd prefer consistency here - either create the char device
when the first controller is probed (preferred) or do not remove it
until the driver exits.
This is not a nack, just a wish that you changed it in next series.

--tm

> ---
>  drivers/scsi/aacraid/aacraid.h |  7 +++++++
>  drivers/scsi/aacraid/linit.c   | 21 ++++++++++++++-------
>  2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 3473668..4b669ef 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -94,6 +94,13 @@ enum {
>  #define aac_phys_to_logical(x)  ((x)+1)
>  #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
>  
> +/*
> + * These macros are for keeping track of
> + * character device state.
> + */
> +#define AAC_CHARDEV_UNREGISTERED	(-1)
> +#define AAC_CHARDEV_NEEDS_REINIT	(-2)
> +
>  /* #define AAC_DETAILED_STATUS_INFO */
>  
>  struct diskparm
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 27b3fcd..057c07c 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
>  
>  static DEFINE_MUTEX(aac_mutex);
>  static LIST_HEAD(aac_devices);
> -static int aac_cfg_major = -1;
> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
>  char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
>  
>  /*
> @@ -1125,6 +1125,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)
>  {
> @@ -1182,6 +1189,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 == AAC_CHARDEV_NEEDS_REINIT)
> +		aac_init_char();
> +
>  	aac = (struct aac_dev *)shost->hostdata;
>  	aac->base_start = pci_resource_start(pdev, 0);
>  	aac->scsi_host_ptr = shost;
> @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev *pdev)
>  	pci_disable_device(pdev);
>  	if (list_empty(&aac_devices)) {
>  		unregister_chrdev(aac_cfg_major, "aac");
> -		aac_cfg_major = -1;
> +		aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
>  	}
>  }
>  
> @@ -1681,11 +1691,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();
> +
>  
>  	return 0;
>  }

RE: [PATCH V3 8/9] aacraid: Fix character device re-initialization

[Raghava Aditya Renukunta]

Hello Tomas,

> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Wednesday, January 20, 2016 5:41 AM
> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com;
> zzzDavid Carroll
> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization
> 
> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> >
> > 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.
> >
> > Changes in V2:
> > Added macros to track character device state
> >
> > Changes in V3:
> > None
> >
> > Signed-off-by: Raghava Aditya Renukunta
> <raghavaaditya.renukunta@pmcs.com>
> > Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
> 
> Hi Raghava,
> when aacraid is loaded (modprobe) without an controller attached to the
> system
> the driver loads and creates the character device. Later when you hotplug a
> device and remove again we see the driver loaded but now without the
> char device. I'd prefer consistency here - either create the char device
> when the first controller is probed (preferred) or do not remove it
> until the driver exits.
> This is not a nack, just a wish that you changed it in next series.
> 
> --tm


Yes I will make the necessary changes  so that character device is created when
The controller is probed, and when the driver is removed (aac_remove_one),delete 
the character device. I will keep the character device during resume and suspend.

Do you want to do this in the next version of the patches or the next series of patches after this one is 
Accepted. ?
 
Regards,
Raghava Aditya


> > ---
> >  drivers/scsi/aacraid/aacraid.h |  7 +++++++
> >  drivers/scsi/aacraid/linit.c   | 21 ++++++++++++++-------
> >  2 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 3473668..4b669ef 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -94,6 +94,13 @@ enum {
> >  #define aac_phys_to_logical(x)  ((x)+1)
> >  #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
> >
> > +/*
> > + * These macros are for keeping track of
> > + * character device state.
> > + */
> > +#define AAC_CHARDEV_UNREGISTERED	(-1)
> > +#define AAC_CHARDEV_NEEDS_REINIT	(-2)
> > +
> >  /* #define AAC_DETAILED_STATUS_INFO */
> >
> >  struct diskparm
> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index 27b3fcd..057c07c 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
> >
> >  static DEFINE_MUTEX(aac_mutex);
> >  static LIST_HEAD(aac_devices);
> > -static int aac_cfg_major = -1;
> > +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
> >  char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
> >
> >  /*
> > @@ -1125,6 +1125,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)
> >  {
> > @@ -1182,6 +1189,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 == AAC_CHARDEV_NEEDS_REINIT)
> > +		aac_init_char();
> > +
> >  	aac = (struct aac_dev *)shost->hostdata;
> >  	aac->base_start = pci_resource_start(pdev, 0);
> >  	aac->scsi_host_ptr = shost;
> > @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev
> *pdev)
> >  	pci_disable_device(pdev);
> >  	if (list_empty(&aac_devices)) {
> >  		unregister_chrdev(aac_cfg_major, "aac");
> > -		aac_cfg_major = -1;
> > +		aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
> >  	}
> >  }
> >
> > @@ -1681,11 +1691,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();
> > +
> >
> >  	return 0;
> >  }

Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization

[Tomas Henzl]

On 20.1.2016 21:43, Raghava Aditya Renukunta wrote:
> Hello Tomas,
>
>> -----Original Message-----
>> From: Tomas Henzl [mailto:thenzl@redhat.com]
>> Sent: Wednesday, January 20, 2016 5:41 AM
>> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
>> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
>> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
>> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com;
>> zzzDavid Carroll
>> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization
>>
>> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
>>> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>>>
>>> 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.
>>>
>>> Changes in V2:
>>> Added macros to track character device state
>>>
>>> Changes in V3:
>>> None
>>>
>>> Signed-off-by: Raghava Aditya Renukunta
>> <raghavaaditya.renukunta@pmcs.com>
>>> Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
>> Hi Raghava,
>> when aacraid is loaded (modprobe) without an controller attached to the
>> system
>> the driver loads and creates the character device. Later when you hotplug a
>> device and remove again we see the driver loaded but now without the
>> char device. I'd prefer consistency here - either create the char device
>> when the first controller is probed (preferred) or do not remove it
>> until the driver exits.
>> This is not a nack, just a wish that you changed it in next series.
>>
>> --tm
>
> Yes I will make the necessary changes  so that character device is created when
> The controller is probed, and when the driver is removed (aac_remove_one),delete 
> the character device. I will keep the character device during resume and suspend.
>
> Do you want to do this in the next version of the patches or the next series of patches after this one is 
> Accepted. ?

sure, next series is fine, as I wrote already

>  
> Regards,
> Raghava Aditya
>
>
>>> ---
>>>  drivers/scsi/aacraid/aacraid.h |  7 +++++++
>>>  drivers/scsi/aacraid/linit.c   | 21 ++++++++++++++-------
>>>  2 files changed, 21 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
>>> index 3473668..4b669ef 100644
>>> --- a/drivers/scsi/aacraid/aacraid.h
>>> +++ b/drivers/scsi/aacraid/aacraid.h
>>> @@ -94,6 +94,13 @@ enum {
>>>  #define aac_phys_to_logical(x)  ((x)+1)
>>>  #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
>>>
>>> +/*
>>> + * These macros are for keeping track of
>>> + * character device state.
>>> + */
>>> +#define AAC_CHARDEV_UNREGISTERED	(-1)
>>> +#define AAC_CHARDEV_NEEDS_REINIT	(-2)
>>> +
>>>  /* #define AAC_DETAILED_STATUS_INFO */
>>>
>>>  struct diskparm
>>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
>>> index 27b3fcd..057c07c 100644
>>> --- a/drivers/scsi/aacraid/linit.c
>>> +++ b/drivers/scsi/aacraid/linit.c
>>> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
>>>
>>>  static DEFINE_MUTEX(aac_mutex);
>>>  static LIST_HEAD(aac_devices);
>>> -static int aac_cfg_major = -1;
>>> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
>>>  char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
>>>
>>>  /*
>>> @@ -1125,6 +1125,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)
>>>  {
>>> @@ -1182,6 +1189,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 == AAC_CHARDEV_NEEDS_REINIT)
>>> +		aac_init_char();
>>> +
>>>  	aac = (struct aac_dev *)shost->hostdata;
>>>  	aac->base_start = pci_resource_start(pdev, 0);
>>>  	aac->scsi_host_ptr = shost;
>>> @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev
>> *pdev)
>>>  	pci_disable_device(pdev);
>>>  	if (list_empty(&aac_devices)) {
>>>  		unregister_chrdev(aac_cfg_major, "aac");
>>> -		aac_cfg_major = -1;
>>> +		aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
>>>  	}
>>>  }
>>>
>>> @@ -1681,11 +1691,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();
>>> +
>>>
>>>  	return 0;
>>>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V3 8/9] aacraid: Fix character device re-initialization

[Raghava Aditya Renukunta]

> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Friday, January 22, 2016 5:08 AM
> To: Raghava Aditya Renukunta; James.Bottomley@HansenPartnership.com;
> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumshirn@suse.de; shane.seymour@hpe.com;
> zzzDavid Carroll
> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization
> 
> On 20.1.2016 21:43, Raghava Aditya Renukunta wrote:
> > Hello Tomas,
> >
> >> -----Original Message-----
> >> From: Tomas Henzl [mailto:thenzl@redhat.com]
> >> Sent: Wednesday, January 20, 2016 5:41 AM
> >> To: Raghava Aditya Renukunta;
> James.Bottomley@HansenPartnership.com;
> >> martin.petersen@oracle.com; linux-scsi@vger.kernel.org
> >> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> >> sierra.com; Scott Benesh; jthumshirn@suse.de;
> shane.seymour@hpe.com;
> >> zzzDavid Carroll
> >> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization
> >>
> >> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> >>> From: Raghava Aditya Renukunta
> <raghavaaditya.renukunta@pmcs.com>
> >>>
> >>> 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.
> >>>
> >>> Changes in V2:
> >>> Added macros to track character device state
> >>>
> >>> Changes in V3:
> >>> None
> >>>
> >>> Signed-off-by: Raghava Aditya Renukunta
> >> <raghavaaditya.renukunta@pmcs.com>
> >>> Reviewed-by: Shane Seymour <shane.seymour@hpe.com>
> >> Hi Raghava,
> >> when aacraid is loaded (modprobe) without an controller attached to the
> >> system
> >> the driver loads and creates the character device. Later when you hotplug
> a
> >> device and remove again we see the driver loaded but now without the
> >> char device. I'd prefer consistency here - either create the char device
> >> when the first controller is probed (preferred) or do not remove it
> >> until the driver exits.
> >> This is not a nack, just a wish that you changed it in next series.
> >>
> >> --tm
> >
> > Yes I will make the necessary changes  so that character device is created
> when
> > The controller is probed, and when the driver is removed
> (aac_remove_one),delete
> > the character device. I will keep the character device during resume and
> suspend.
> >
> > Do you want to do this in the next version of the patches or the next series
> of patches after this one is
> > Accepted. ?
> 
> sure, next series is fine, as I wrote already

Will do , Thank you Tomas.

> >
> > Regards,
> > Raghava Aditya
> >
> >
> >>> ---
> >>>  drivers/scsi/aacraid/aacraid.h |  7 +++++++
> >>>  drivers/scsi/aacraid/linit.c   | 21 ++++++++++++++-------
> >>>  2 files changed, 21 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> >>> index 3473668..4b669ef 100644
> >>> --- a/drivers/scsi/aacraid/aacraid.h
> >>> +++ b/drivers/scsi/aacraid/aacraid.h
> >>> @@ -94,6 +94,13 @@ enum {
> >>>  #define aac_phys_to_logical(x)  ((x)+1)
> >>>  #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
> >>>
> >>> +/*
> >>> + * These macros are for keeping track of
> >>> + * character device state.
> >>> + */
> >>> +#define AAC_CHARDEV_UNREGISTERED	(-1)
> >>> +#define AAC_CHARDEV_NEEDS_REINIT	(-2)
> >>> +
> >>>  /* #define AAC_DETAILED_STATUS_INFO */
> >>>
> >>>  struct diskparm
> >>> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> >>> index 27b3fcd..057c07c 100644
> >>> --- a/drivers/scsi/aacraid/linit.c
> >>> +++ b/drivers/scsi/aacraid/linit.c
> >>> @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
> >>>
> >>>  static DEFINE_MUTEX(aac_mutex);
> >>>  static LIST_HEAD(aac_devices);
> >>> -static int aac_cfg_major = -1;
> >>> +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
> >>>  char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
> >>>
> >>>  /*
> >>> @@ -1125,6 +1125,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)
> >>>  {
> >>> @@ -1182,6 +1189,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 == AAC_CHARDEV_NEEDS_REINIT)
> >>> +		aac_init_char();
> >>> +
> >>>  	aac = (struct aac_dev *)shost->hostdata;
> >>>  	aac->base_start = pci_resource_start(pdev, 0);
> >>>  	aac->scsi_host_ptr = shost;
> >>> @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev
> >> *pdev)
> >>>  	pci_disable_device(pdev);
> >>>  	if (list_empty(&aac_devices)) {
> >>>  		unregister_chrdev(aac_cfg_major, "aac");
> >>> -		aac_cfg_major = -1;
> >>> +		aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
> >>>  	}
> >>>  }
> >>>
> >>> @@ -1681,11 +1691,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();
> >>> +
> >>>
> >>>  	return 0;
> >>>  }
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V3 9/9] aacraid: Update driver version

[Raghava Aditya Renukunta]

From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

Updated diver version to 41052

Changes in V2:
None

Changes in V3:
None

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/aacraid/aacraid.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 4b669ef..0143a9d 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -62,7 +62,7 @@ enum {
 #define	PMC_GLOBAL_INT_BIT0		0x00000001
 
 #ifndef AAC_DRIVER_BUILD
-# define AAC_DRIVER_BUILD 41010
+# define AAC_DRIVER_BUILD 41052
 # define AAC_DRIVER_BRANCH "-ms"
 #endif
 #define MAXIMUM_NUM_CONTAINERS	32
-- 
1.9.1