[PATCH v2 00/11] mptfusion/mpt2/mpt3 static checker fixups

[PATCH v2 00/11] mptfusion/mpt2/mpt3 static checker fixups

[Joe Lawrence]

v2:

Combined the mptfusion and mpt{2,3}sas static checker patches,
re-ordering them by driver.  Updated Reviewed-by and Acked-by tags as
well as Sreekanth's email address.  Based off v3.16-rc2, compile tested.

patches dropped:

* mpt3sas: fix possible memory leak in mpt3sas_send_trigger_data_event

  Christoph suggested combining this into a single allocation, so this
  patch was transformed into two new patches (removing the Reviewed-by
  and Acked-by tags):

  mpt2sas-combine-fw_event_work-and-its-event_data.patch
  mpt3sas-combine-fw_event_work-and-its-event_data.patch

* mptfusion: initChainBuffers should return errno

  Christoph noted that the whole callchain uses -1 instead of errno.
  Let it be.


patches modified:

* mptfusion: zero kernel-space source of copy_to_user

  A static checker false-positive brought me here, Christoph suggested
  using memdup_user.

* mptfusion: combine fw_event_work and its event_data

  Remove the unnecessary sz variables.

* mptfusion: tweak null pointer checks

  Moved commentry from myself (JL) and Christoph (HCH) into the commit
  message.


In the mpt{2,3}sas-combine-fw_event_work-and-its-event_data patches, I
was wondering if the alignment attribute should be:

  __attribute__ ((aligned (sizeof(unsigned long))))

instead of:

  char event_data[0] __aligned(4)

Regards,

Joe Lawrence (11):
  mpt2sas: correct scsi_{target,device} hostdata allocation
  mpt2sas: combine fw_event_work and its event_data
  mpt2sas: annotate ioc->reply_post_host_index as __iomem
  mpt3sas: correct scsi_{target,device} hostdata allocation
  mpt3sas: combine fw_event_work and its event_data
  mptfusion: mark file-private functions as static
  mptfusion: remove redundant kfree checks
  mptfusion: use memdup_user
  mptfusion: make adapter prod_name[] a pointer
  mptfusion: combine fw_event_work and its event_data
  mptfusion: tweak null pointer checks

 drivers/message/fusion/mptbase.c     |   23 +++++------
 drivers/message/fusion/mptbase.h     |    2 +-
 drivers/message/fusion/mptctl.c      |   18 +++------
 drivers/message/fusion/mptfc.c       |    3 +-
 drivers/message/fusion/mptsas.c      |   74 ++++++++++++++++------------------
 drivers/message/fusion/mptsas.h      |    2 +-
 drivers/message/fusion/mptscsih.c    |   19 ++++-----
 drivers/message/fusion/mptspi.c      |    5 +--
 drivers/scsi/mpt2sas/mpt2sas_base.c  |    9 +++--
 drivers/scsi/mpt2sas/mpt2sas_base.h  |    2 +-
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |   58 +++++++++++++++-----------
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   62 +++++++++++++++-------------
 12 files changed, 137 insertions(+), 140 deletions(-)

-- 
1.7.10.4

[PATCH v2 01/11] mpt2sas: correct scsi_{target,device} hostdata allocation

[Joe Lawrence]

In _scsih_{slave,target}_alloc, an incorrect structure type is passed
to sizeof() when allocating storage for hostdata.  Luckily larger
structure types were used, so at least the wrong sizes were safe:

  struct scsi_device (1784 bytes) > struct MPT2SAS_DEVICE (24 bytes)
  struct scsi_target (760 bytes)  > struct MPT2SAS_TARGET (40 bytes)

This fixes the following smatch warnings:

  drivers/scsi/mpt2sas/mpt2sas_scsih.c:1295 _scsih_target_alloc()
    warn: struct type mismatch 'MPT2SAS_TARGET vs scsi_target'

  drivers/scsi/mpt2sas/mpt2sas_scsih.c:1409 _scsih_slave_alloc()
    warn: struct type mismatch 'MPT2SAS_DEVICE vs scsi_device'

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index 5055f92..13e49c3 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -1292,7 +1292,8 @@ _scsih_target_alloc(struct scsi_target *starget)
 	unsigned long flags;
 	struct sas_rphy *rphy;
 
-	sas_target_priv_data = kzalloc(sizeof(struct scsi_target), GFP_KERNEL);
+	sas_target_priv_data = kzalloc(sizeof(*sas_target_priv_data),
+				       GFP_KERNEL);
 	if (!sas_target_priv_data)
 		return -ENOMEM;
 
@@ -1406,7 +1407,8 @@ _scsih_slave_alloc(struct scsi_device *sdev)
 	struct _sas_device *sas_device;
 	unsigned long flags;
 
-	sas_device_priv_data = kzalloc(sizeof(struct scsi_device), GFP_KERNEL);
+	sas_device_priv_data = kzalloc(sizeof(*sas_device_priv_data),
+				       GFP_KERNEL);
 	if (!sas_device_priv_data)
 		return -ENOMEM;
 
-- 
1.7.10.4

[PATCH v2 02/11] mpt2sas: combine fw_event_work and its event_data

[Joe Lawrence]

Tack the firmware reply event_data payload to the end of its
corresponding struct fw_event_work allocation.  This matches the
convention in the mptfusion driver and simplifies the code.

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |   52 ++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index 13e49c3..e7801ff 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -173,7 +173,7 @@ struct fw_event_work {
 	u8			VP_ID;
 	u8			ignore;
 	u16			event;
-	void			*event_data;
+	char			event_data[0] __aligned(4);
 };
 
 /* raid transport support */
@@ -2834,7 +2834,6 @@ _scsih_fw_event_free(struct MPT2SAS_ADAPTER *ioc, struct fw_event_work
 
 	spin_lock_irqsave(&ioc->fw_event_lock, flags);
 	list_del(&fw_event->list);
-	kfree(fw_event->event_data);
 	kfree(fw_event);
 	spin_unlock_irqrestore(&ioc->fw_event_lock, flags);
 }
@@ -3520,7 +3519,8 @@ _scsih_check_topo_delete_events(struct MPT2SAS_ADAPTER *ioc,
 		if (fw_event->event != MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST ||
 		    fw_event->ignore)
 			continue;
-		local_event_data = fw_event->event_data;
+		local_event_data = (Mpi2EventDataSasTopologyChangeList_t *)
+			fw_event->event_data;
 		if (local_event_data->ExpStatus ==
 		    MPI2_EVENT_SAS_TOPO_ES_ADDED ||
 		    local_event_data->ExpStatus ==
@@ -5504,7 +5504,9 @@ _scsih_sas_topology_change_event(struct MPT2SAS_ADAPTER *ioc,
 	u64 sas_address;
 	unsigned long flags;
 	u8 link_rate, prev_link_rate;
-	Mpi2EventDataSasTopologyChangeList_t *event_data = fw_event->event_data;
+	Mpi2EventDataSasTopologyChangeList_t *event_data =
+		(Mpi2EventDataSasTopologyChangeList_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
@@ -5699,7 +5701,8 @@ _scsih_sas_device_status_change_event(struct MPT2SAS_ADAPTER *ioc,
 	u64 sas_address;
 	unsigned long flags;
 	Mpi2EventDataSasDeviceStatusChange_t *event_data =
-	    fw_event->event_data;
+		(Mpi2EventDataSasDeviceStatusChange_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
@@ -5794,6 +5797,7 @@ _scsih_sas_enclosure_dev_status_change_event(struct MPT2SAS_ADAPTER *ioc,
 #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
 		_scsih_sas_enclosure_dev_status_change_event_debug(ioc,
+		     (Mpi2EventDataSasEnclDevStatusChange_t *)
 		     fw_event->event_data);
 #endif
 }
@@ -5818,7 +5822,9 @@ _scsih_sas_broadcast_primitive_event(struct MPT2SAS_ADAPTER *ioc,
 	u32 termination_count;
 	u32 query_count;
 	Mpi2SCSITaskManagementReply_t *mpi_reply;
-	Mpi2EventDataSasBroadcastPrimitive_t *event_data = fw_event->event_data;
+	Mpi2EventDataSasBroadcastPrimitive_t *event_data =
+		(Mpi2EventDataSasBroadcastPrimitive_t *)
+		fw_event->event_data;
 	u16 ioc_status;
 	unsigned long flags;
 	int r;
@@ -5969,7 +5975,9 @@ static void
 _scsih_sas_discovery_event(struct MPT2SAS_ADAPTER *ioc,
     struct fw_event_work *fw_event)
 {
-	Mpi2EventDataSasDiscovery_t *event_data = fw_event->event_data;
+	Mpi2EventDataSasDiscovery_t *event_data =
+		(Mpi2EventDataSasDiscovery_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK) {
@@ -6357,7 +6365,9 @@ _scsih_sas_ir_config_change_event(struct MPT2SAS_ADAPTER *ioc,
 	Mpi2EventIrConfigElement_t *element;
 	int i;
 	u8 foreign_config;
-	Mpi2EventDataIrConfigChangeList_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrConfigChangeList_t *event_data =
+		(Mpi2EventDataIrConfigChangeList_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
 	if ((ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
@@ -6425,7 +6435,9 @@ _scsih_sas_ir_volume_event(struct MPT2SAS_ADAPTER *ioc,
 	u16 handle;
 	u32 state;
 	int rc;
-	Mpi2EventDataIrVolume_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrVolume_t *event_data =
+		(Mpi2EventDataIrVolume_t *)
+		fw_event->event_data;
 
 	if (ioc->shost_recovery)
 		return;
@@ -6509,7 +6521,9 @@ _scsih_sas_ir_physical_disk_event(struct MPT2SAS_ADAPTER *ioc,
 	Mpi2ConfigReply_t mpi_reply;
 	Mpi2SasDevicePage0_t sas_device_pg0;
 	u32 ioc_status;
-	Mpi2EventDataIrPhysicalDisk_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrPhysicalDisk_t *event_data =
+		(Mpi2EventDataIrPhysicalDisk_t *)
+		fw_event->event_data;
 	u64 sas_address;
 
 	if (ioc->shost_recovery)
@@ -6632,7 +6646,9 @@ static void
 _scsih_sas_ir_operation_status_event(struct MPT2SAS_ADAPTER *ioc,
     struct fw_event_work *fw_event)
 {
-	Mpi2EventDataIrOperationStatus_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrOperationStatus_t *event_data =
+		(Mpi2EventDataIrOperationStatus_t *)
+		fw_event->event_data;
 	static struct _raid_device *raid_device;
 	unsigned long flags;
 	u16 handle;
@@ -7592,23 +7608,15 @@ mpt2sas_scsih_event_callback(struct MPT2SAS_ADAPTER *ioc, u8 msix_index,
 		return;
 	}
 
-	fw_event = kzalloc(sizeof(struct fw_event_work), GFP_ATOMIC);
-	if (!fw_event) {
-		printk(MPT2SAS_ERR_FMT "failure at %s:%d/%s()!\n",
-		    ioc->name, __FILE__, __LINE__, __func__);
-		return;
-	}
 	sz = le16_to_cpu(mpi_reply->EventDataLength) * 4;
-	fw_event->event_data = kzalloc(sz, GFP_ATOMIC);
-	if (!fw_event->event_data) {
+	fw_event = kzalloc(sizeof(*fw_event) + sz, GFP_ATOMIC);
+	if (!fw_event) {
 		printk(MPT2SAS_ERR_FMT "failure at %s:%d/%s()!\n",
 		    ioc->name, __FILE__, __LINE__, __func__);
-		kfree(fw_event);
 		return;
 	}
 
-	memcpy(fw_event->event_data, mpi_reply->EventData,
-	    sz);
+	memcpy(fw_event->event_data, mpi_reply->EventData, sz);
 	fw_event->ioc = ioc;
 	fw_event->VF_ID = mpi_reply->VF_ID;
 	fw_event->VP_ID = mpi_reply->VP_ID;
-- 
1.7.10.4

Re: [PATCH v2 02/11] mpt2sas: combine fw_event_work and its event_data

[Christoph Hellwig]

Sreekanth,

can you give me a review for this one?

On Wed, Jun 25, 2014 at 05:03:33PM -0400, Joe Lawrence wrote:
> Tack the firmware reply event_data payload to the end of its
> corresponding struct fw_event_work allocation.  This matches the
> convention in the mptfusion driver and simplifies the code.
> 
> Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
> ---
>  drivers/scsi/mpt2sas/mpt2sas_scsih.c |   52 ++++++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> index 13e49c3..e7801ff 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> @@ -173,7 +173,7 @@ struct fw_event_work {
>  	u8			VP_ID;
>  	u8			ignore;
>  	u16			event;
> -	void			*event_data;
> +	char			event_data[0] __aligned(4);
>  };
>  
>  /* raid transport support */
> @@ -2834,7 +2834,6 @@ _scsih_fw_event_free(struct MPT2SAS_ADAPTER *ioc, struct fw_event_work
>  
>  	spin_lock_irqsave(&ioc->fw_event_lock, flags);
>  	list_del(&fw_event->list);
> -	kfree(fw_event->event_data);
>  	kfree(fw_event);
>  	spin_unlock_irqrestore(&ioc->fw_event_lock, flags);
>  }
> @@ -3520,7 +3519,8 @@ _scsih_check_topo_delete_events(struct MPT2SAS_ADAPTER *ioc,
>  		if (fw_event->event != MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST ||
>  		    fw_event->ignore)
>  			continue;
> -		local_event_data = fw_event->event_data;
> +		local_event_data = (Mpi2EventDataSasTopologyChangeList_t *)
> +			fw_event->event_data;
>  		if (local_event_data->ExpStatus ==
>  		    MPI2_EVENT_SAS_TOPO_ES_ADDED ||
>  		    local_event_data->ExpStatus ==
> @@ -5504,7 +5504,9 @@ _scsih_sas_topology_change_event(struct MPT2SAS_ADAPTER *ioc,
>  	u64 sas_address;
>  	unsigned long flags;
>  	u8 link_rate, prev_link_rate;
> -	Mpi2EventDataSasTopologyChangeList_t *event_data = fw_event->event_data;
> +	Mpi2EventDataSasTopologyChangeList_t *event_data =
> +		(Mpi2EventDataSasTopologyChangeList_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
> @@ -5699,7 +5701,8 @@ _scsih_sas_device_status_change_event(struct MPT2SAS_ADAPTER *ioc,
>  	u64 sas_address;
>  	unsigned long flags;
>  	Mpi2EventDataSasDeviceStatusChange_t *event_data =
> -	    fw_event->event_data;
> +		(Mpi2EventDataSasDeviceStatusChange_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
> @@ -5794,6 +5797,7 @@ _scsih_sas_enclosure_dev_status_change_event(struct MPT2SAS_ADAPTER *ioc,
>  #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
>  		_scsih_sas_enclosure_dev_status_change_event_debug(ioc,
> +		     (Mpi2EventDataSasEnclDevStatusChange_t *)
>  		     fw_event->event_data);
>  #endif
>  }
> @@ -5818,7 +5822,9 @@ _scsih_sas_broadcast_primitive_event(struct MPT2SAS_ADAPTER *ioc,
>  	u32 termination_count;
>  	u32 query_count;
>  	Mpi2SCSITaskManagementReply_t *mpi_reply;
> -	Mpi2EventDataSasBroadcastPrimitive_t *event_data = fw_event->event_data;
> +	Mpi2EventDataSasBroadcastPrimitive_t *event_data =
> +		(Mpi2EventDataSasBroadcastPrimitive_t *)
> +		fw_event->event_data;
>  	u16 ioc_status;
>  	unsigned long flags;
>  	int r;
> @@ -5969,7 +5975,9 @@ static void
>  _scsih_sas_discovery_event(struct MPT2SAS_ADAPTER *ioc,
>      struct fw_event_work *fw_event)
>  {
> -	Mpi2EventDataSasDiscovery_t *event_data = fw_event->event_data;
> +	Mpi2EventDataSasDiscovery_t *event_data =
> +		(Mpi2EventDataSasDiscovery_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK) {
> @@ -6357,7 +6365,9 @@ _scsih_sas_ir_config_change_event(struct MPT2SAS_ADAPTER *ioc,
>  	Mpi2EventIrConfigElement_t *element;
>  	int i;
>  	u8 foreign_config;
> -	Mpi2EventDataIrConfigChangeList_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrConfigChangeList_t *event_data =
> +		(Mpi2EventDataIrConfigChangeList_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT2SAS_LOGGING
>  	if ((ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
> @@ -6425,7 +6435,9 @@ _scsih_sas_ir_volume_event(struct MPT2SAS_ADAPTER *ioc,
>  	u16 handle;
>  	u32 state;
>  	int rc;
> -	Mpi2EventDataIrVolume_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrVolume_t *event_data =
> +		(Mpi2EventDataIrVolume_t *)
> +		fw_event->event_data;
>  
>  	if (ioc->shost_recovery)
>  		return;
> @@ -6509,7 +6521,9 @@ _scsih_sas_ir_physical_disk_event(struct MPT2SAS_ADAPTER *ioc,
>  	Mpi2ConfigReply_t mpi_reply;
>  	Mpi2SasDevicePage0_t sas_device_pg0;
>  	u32 ioc_status;
> -	Mpi2EventDataIrPhysicalDisk_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrPhysicalDisk_t *event_data =
> +		(Mpi2EventDataIrPhysicalDisk_t *)
> +		fw_event->event_data;
>  	u64 sas_address;
>  
>  	if (ioc->shost_recovery)
> @@ -6632,7 +6646,9 @@ static void
>  _scsih_sas_ir_operation_status_event(struct MPT2SAS_ADAPTER *ioc,
>      struct fw_event_work *fw_event)
>  {
> -	Mpi2EventDataIrOperationStatus_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrOperationStatus_t *event_data =
> +		(Mpi2EventDataIrOperationStatus_t *)
> +		fw_event->event_data;
>  	static struct _raid_device *raid_device;
>  	unsigned long flags;
>  	u16 handle;
> @@ -7592,23 +7608,15 @@ mpt2sas_scsih_event_callback(struct MPT2SAS_ADAPTER *ioc, u8 msix_index,
>  		return;
>  	}
>  
> -	fw_event = kzalloc(sizeof(struct fw_event_work), GFP_ATOMIC);
> -	if (!fw_event) {
> -		printk(MPT2SAS_ERR_FMT "failure at %s:%d/%s()!\n",
> -		    ioc->name, __FILE__, __LINE__, __func__);
> -		return;
> -	}
>  	sz = le16_to_cpu(mpi_reply->EventDataLength) * 4;
> -	fw_event->event_data = kzalloc(sz, GFP_ATOMIC);
> -	if (!fw_event->event_data) {
> +	fw_event = kzalloc(sizeof(*fw_event) + sz, GFP_ATOMIC);
> +	if (!fw_event) {
>  		printk(MPT2SAS_ERR_FMT "failure at %s:%d/%s()!\n",
>  		    ioc->name, __FILE__, __LINE__, __func__);
> -		kfree(fw_event);
>  		return;
>  	}
>  
> -	memcpy(fw_event->event_data, mpi_reply->EventData,
> -	    sz);
> +	memcpy(fw_event->event_data, mpi_reply->EventData, sz);
>  	fw_event->ioc = ioc;
>  	fw_event->VF_ID = mpi_reply->VF_ID;
>  	fw_event->VP_ID = mpi_reply->VP_ID;
> -- 
> 1.7.10.4
> 
> --
> 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
---end quoted text---

[PATCH v2 03/11] mpt2sas: annotate ioc->reply_post_host_index as __iomem

[Joe Lawrence]

The MPT2SAS_ADAPTER reply_post_host_index[] holds calculated addresses
in memory mapped register space.  Add an "__iomem" annotation to silence
the following sparse warnings:

  drivers/scsi/mpt2sas/mpt2sas_base.c:1006:43:
    warning: incorrect type in argument 2 (different address spaces)
       expected void volatile [noderef] <asn:2>*addr
       got unsigned long long [usertype] *<noident>

  drivers/scsi/mpt2sas/mpt2sas_base.c:4299:22:
    warning: cast removes address space of expression
  drivers/scsi/mpt2sas/mpt2sas_base.c:4303:27:
    warning: cast removes address space of expression

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c |    9 +++++----
 drivers/scsi/mpt2sas/mpt2sas_base.h |    2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 8b88118..a31397c 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -4295,12 +4295,13 @@ mpt2sas_base_attach(struct MPT2SAS_ADAPTER *ioc)
 		goto out_free_resources;
 
 	if (ioc->is_warpdrive) {
-		ioc->reply_post_host_index[0] =
-		    (resource_size_t *)&ioc->chip->ReplyPostHostIndex;
+		ioc->reply_post_host_index[0] = (resource_size_t __iomem *)
+		    &ioc->chip->ReplyPostHostIndex;
 
 		for (i = 1; i < ioc->cpu_msix_table_sz; i++)
-			ioc->reply_post_host_index[i] = (resource_size_t *)
-			((u8 *)&ioc->chip->Doorbell + (0x4000 + ((i - 1)
+			ioc->reply_post_host_index[i] =
+			(resource_size_t __iomem *)
+			((u8 __iomem *)&ioc->chip->Doorbell + (0x4000 + ((i - 1)
 			* 4)));
 	}
 
diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h
index fd3b998..0ac5815 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.h
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.h
@@ -837,7 +837,7 @@ struct MPT2SAS_ADAPTER {
 	u8		msix_enable;
 	u16		msix_vector_count;
 	u8		*cpu_msix_table;
-	resource_size_t	**reply_post_host_index;
+	resource_size_t	__iomem **reply_post_host_index;
 	u16		cpu_msix_table_sz;
 	u32		ioc_reset_count;
 	MPT2SAS_FLUSH_RUNNING_CMDS schedule_dead_ioc_flush_running_cmds;
-- 
1.7.10.4

[PATCH v2 04/11] mpt3sas: correct scsi_{target,device} hostdata allocation

[Joe Lawrence]

In _scsih_{slave,target}_alloc, an incorrect structure type is passed
to sizeof() when allocating storage for hostdata.  Luckily larger
structure types were used, so at least the wrong sizes were safe:

  struct scsi_device (1784 bytes) > struct MPT3SAS_DEVICE (24 bytes)
  struct scsi_target (760 bytes)  > struct MPT3SAS_TARGET (32 bytes)

This fixes the following smatch warnings:

  drivers/scsi/mpt3sas/mpt3sas_scsih.c:1166 _scsih_target_alloc()
    warn: struct type mismatch 'MPT3SAS_TARGET vs scsi_target'

  drivers/scsi/mpt3sas/mpt3sas_scsih.c:1280 _scsih_slave_alloc()
    warn: struct type mismatch 'MPT3SAS_DEVICE vs scsi_device'

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Acked-by: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 18e713d..f3ee3b4 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -1163,7 +1163,8 @@ _scsih_target_alloc(struct scsi_target *starget)
 	unsigned long flags;
 	struct sas_rphy *rphy;
 
-	sas_target_priv_data = kzalloc(sizeof(struct scsi_target), GFP_KERNEL);
+	sas_target_priv_data = kzalloc(sizeof(*sas_target_priv_data),
+				       GFP_KERNEL);
 	if (!sas_target_priv_data)
 		return -ENOMEM;
 
@@ -1277,7 +1278,8 @@ _scsih_slave_alloc(struct scsi_device *sdev)
 	struct _sas_device *sas_device;
 	unsigned long flags;
 
-	sas_device_priv_data = kzalloc(sizeof(struct scsi_device), GFP_KERNEL);
+	sas_device_priv_data = kzalloc(sizeof(*sas_device_priv_data),
+				       GFP_KERNEL);
 	if (!sas_device_priv_data)
 		return -ENOMEM;
 
-- 
1.7.10.4

[PATCH v2 05/11] mpt3sas: combine fw_event_work and its event_data

[Joe Lawrence]

Tack the firmware reply event_data payload to the end of its
corresponding struct fw_event_work allocation.  This matches the
convention in the mptfusion driver and simplifies the code.

This avoids the following smatch warning:

  drivers/scsi/mpt3sas/mpt3sas_scsih.c:2519
    mpt3sas_send_trigger_data_event() warn: possible memory leak of
    'fw_event'

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   56 +++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index f3ee3b4..a14be8f 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -190,7 +190,7 @@ struct fw_event_work {
 	u8			VP_ID;
 	u8			ignore;
 	u16			event;
-	void			*event_data;
+	char			event_data[0] __aligned(4);
 };
 
 /* raid transport support */
@@ -2492,7 +2492,6 @@ _scsih_fw_event_free(struct MPT3SAS_ADAPTER *ioc, struct fw_event_work
 
 	spin_lock_irqsave(&ioc->fw_event_lock, flags);
 	list_del(&fw_event->list);
-	kfree(fw_event->event_data);
 	kfree(fw_event);
 	spin_unlock_irqrestore(&ioc->fw_event_lock, flags);
 }
@@ -2513,12 +2512,10 @@ mpt3sas_send_trigger_data_event(struct MPT3SAS_ADAPTER *ioc,
 
 	if (ioc->is_driver_loading)
 		return;
-	fw_event = kzalloc(sizeof(struct fw_event_work), GFP_ATOMIC);
+	fw_event = kzalloc(sizeof(*fw_event) + sizeof(*event_data),
+			   GFP_ATOMIC);
 	if (!fw_event)
 		return;
-	fw_event->event_data = kzalloc(sizeof(*event_data), GFP_ATOMIC);
-	if (!fw_event->event_data)
-		return;
 	fw_event->event = MPT3SAS_PROCESS_TRIGGER_DIAG;
 	fw_event->ioc = ioc;
 	memcpy(fw_event->event_data, event_data, sizeof(*event_data));
@@ -3213,7 +3210,8 @@ _scsih_check_topo_delete_events(struct MPT3SAS_ADAPTER *ioc,
 		if (fw_event->event != MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST ||
 		    fw_event->ignore)
 			continue;
-		local_event_data = fw_event->event_data;
+		local_event_data = (Mpi2EventDataSasTopologyChangeList_t *)
+				   fw_event->event_data;
 		if (local_event_data->ExpStatus ==
 		    MPI2_EVENT_SAS_TOPO_ES_ADDED ||
 		    local_event_data->ExpStatus ==
@@ -5045,7 +5043,9 @@ _scsih_sas_topology_change_event(struct MPT3SAS_ADAPTER *ioc,
 	u64 sas_address;
 	unsigned long flags;
 	u8 link_rate, prev_link_rate;
-	Mpi2EventDataSasTopologyChangeList_t *event_data = fw_event->event_data;
+	Mpi2EventDataSasTopologyChangeList_t *event_data =
+		(Mpi2EventDataSasTopologyChangeList_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
@@ -5243,7 +5243,8 @@ _scsih_sas_device_status_change_event(struct MPT3SAS_ADAPTER *ioc,
 	u64 sas_address;
 	unsigned long flags;
 	Mpi2EventDataSasDeviceStatusChange_t *event_data =
-	    fw_event->event_data;
+		(Mpi2EventDataSasDeviceStatusChange_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
@@ -5339,6 +5340,7 @@ _scsih_sas_enclosure_dev_status_change_event(struct MPT3SAS_ADAPTER *ioc,
 #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
 		_scsih_sas_enclosure_dev_status_change_event_debug(ioc,
+		     (Mpi2EventDataSasEnclDevStatusChange_t *)
 		     fw_event->event_data);
 #endif
 }
@@ -5363,7 +5365,9 @@ _scsih_sas_broadcast_primitive_event(struct MPT3SAS_ADAPTER *ioc,
 	u32 termination_count;
 	u32 query_count;
 	Mpi2SCSITaskManagementReply_t *mpi_reply;
-	Mpi2EventDataSasBroadcastPrimitive_t *event_data = fw_event->event_data;
+	Mpi2EventDataSasBroadcastPrimitive_t *event_data =
+		(Mpi2EventDataSasBroadcastPrimitive_t *)
+		fw_event->event_data;
 	u16 ioc_status;
 	unsigned long flags;
 	int r;
@@ -5515,7 +5519,8 @@ static void
 _scsih_sas_discovery_event(struct MPT3SAS_ADAPTER *ioc,
 	struct fw_event_work *fw_event)
 {
-	Mpi2EventDataSasDiscovery_t *event_data = fw_event->event_data;
+	Mpi2EventDataSasDiscovery_t *event_data =
+		(Mpi2EventDataSasDiscovery_t *) fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK) {
@@ -6001,7 +6006,9 @@ _scsih_sas_ir_config_change_event(struct MPT3SAS_ADAPTER *ioc,
 	Mpi2EventIrConfigElement_t *element;
 	int i;
 	u8 foreign_config;
-	Mpi2EventDataIrConfigChangeList_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrConfigChangeList_t *event_data =
+		(Mpi2EventDataIrConfigChangeList_t *)
+		fw_event->event_data;
 
 #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
 	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
@@ -6071,7 +6078,8 @@ _scsih_sas_ir_volume_event(struct MPT3SAS_ADAPTER *ioc,
 	u16 handle;
 	u32 state;
 	int rc;
-	Mpi2EventDataIrVolume_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrVolume_t *event_data =
+		(Mpi2EventDataIrVolume_t *) fw_event->event_data;
 
 	if (ioc->shost_recovery)
 		return;
@@ -6154,7 +6162,8 @@ _scsih_sas_ir_physical_disk_event(struct MPT3SAS_ADAPTER *ioc,
 	Mpi2ConfigReply_t mpi_reply;
 	Mpi2SasDevicePage0_t sas_device_pg0;
 	u32 ioc_status;
-	Mpi2EventDataIrPhysicalDisk_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrPhysicalDisk_t *event_data =
+		(Mpi2EventDataIrPhysicalDisk_t *) fw_event->event_data;
 	u64 sas_address;
 
 	if (ioc->shost_recovery)
@@ -6274,7 +6283,9 @@ static void
 _scsih_sas_ir_operation_status_event(struct MPT3SAS_ADAPTER *ioc,
 	struct fw_event_work *fw_event)
 {
-	Mpi2EventDataIrOperationStatus_t *event_data = fw_event->event_data;
+	Mpi2EventDataIrOperationStatus_t *event_data =
+		(Mpi2EventDataIrOperationStatus_t *)
+		fw_event->event_data;
 	static struct _raid_device *raid_device;
 	unsigned long flags;
 	u16 handle;
@@ -7036,7 +7047,9 @@ _mpt3sas_fw_work(struct MPT3SAS_ADAPTER *ioc, struct fw_event_work *fw_event)
 
 	switch (fw_event->event) {
 	case MPT3SAS_PROCESS_TRIGGER_DIAG:
-		mpt3sas_process_trigger_data(ioc, fw_event->event_data);
+		mpt3sas_process_trigger_data(ioc,
+			(struct SL_WH_TRIGGERS_EVENT_DATA_T *)
+			fw_event->event_data);
 		break;
 	case MPT3SAS_REMOVE_UNRESPONDING_DEVICES:
 		while (scsi_host_in_recovery(ioc->shost) || ioc->shost_recovery)
@@ -7194,18 +7207,11 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index,
 		return 1;
 	}
 
-	fw_event = kzalloc(sizeof(struct fw_event_work), GFP_ATOMIC);
-	if (!fw_event) {
-		pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
-		    ioc->name, __FILE__, __LINE__, __func__);
-		return 1;
-	}
 	sz = le16_to_cpu(mpi_reply->EventDataLength) * 4;
-	fw_event->event_data = kzalloc(sz, GFP_ATOMIC);
-	if (!fw_event->event_data) {
+	fw_event = kzalloc(sizeof(*fw_event) + sz, GFP_ATOMIC);
+	if (!fw_event) {
 		pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
 		    ioc->name, __FILE__, __LINE__, __func__);
-		kfree(fw_event);
 		return 1;
 	}
 
-- 
1.7.10.4

Re: [PATCH v2 05/11] mpt3sas: combine fw_event_work and its event_data

[Christoph Hellwig]

Sreekanth, can you give me a review for this patch?  Thanks!

On Wed, Jun 25, 2014 at 05:05:34PM -0400, Joe Lawrence wrote:
> Tack the firmware reply event_data payload to the end of its
> corresponding struct fw_event_work allocation.  This matches the
> convention in the mptfusion driver and simplifies the code.
> 
> This avoids the following smatch warning:
> 
>   drivers/scsi/mpt3sas/mpt3sas_scsih.c:2519
>     mpt3sas_send_trigger_data_event() warn: possible memory leak of
>     'fw_event'
> 
> Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c |   56 +++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index f3ee3b4..a14be8f 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -190,7 +190,7 @@ struct fw_event_work {
>  	u8			VP_ID;
>  	u8			ignore;
>  	u16			event;
> -	void			*event_data;
> +	char			event_data[0] __aligned(4);
>  };
>  
>  /* raid transport support */
> @@ -2492,7 +2492,6 @@ _scsih_fw_event_free(struct MPT3SAS_ADAPTER *ioc, struct fw_event_work
>  
>  	spin_lock_irqsave(&ioc->fw_event_lock, flags);
>  	list_del(&fw_event->list);
> -	kfree(fw_event->event_data);
>  	kfree(fw_event);
>  	spin_unlock_irqrestore(&ioc->fw_event_lock, flags);
>  }
> @@ -2513,12 +2512,10 @@ mpt3sas_send_trigger_data_event(struct MPT3SAS_ADAPTER *ioc,
>  
>  	if (ioc->is_driver_loading)
>  		return;
> -	fw_event = kzalloc(sizeof(struct fw_event_work), GFP_ATOMIC);
> +	fw_event = kzalloc(sizeof(*fw_event) + sizeof(*event_data),
> +			   GFP_ATOMIC);
>  	if (!fw_event)
>  		return;
> -	fw_event->event_data = kzalloc(sizeof(*event_data), GFP_ATOMIC);
> -	if (!fw_event->event_data)
> -		return;
>  	fw_event->event = MPT3SAS_PROCESS_TRIGGER_DIAG;
>  	fw_event->ioc = ioc;
>  	memcpy(fw_event->event_data, event_data, sizeof(*event_data));
> @@ -3213,7 +3210,8 @@ _scsih_check_topo_delete_events(struct MPT3SAS_ADAPTER *ioc,
>  		if (fw_event->event != MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST ||
>  		    fw_event->ignore)
>  			continue;
> -		local_event_data = fw_event->event_data;
> +		local_event_data = (Mpi2EventDataSasTopologyChangeList_t *)
> +				   fw_event->event_data;
>  		if (local_event_data->ExpStatus ==
>  		    MPI2_EVENT_SAS_TOPO_ES_ADDED ||
>  		    local_event_data->ExpStatus ==
> @@ -5045,7 +5043,9 @@ _scsih_sas_topology_change_event(struct MPT3SAS_ADAPTER *ioc,
>  	u64 sas_address;
>  	unsigned long flags;
>  	u8 link_rate, prev_link_rate;
> -	Mpi2EventDataSasTopologyChangeList_t *event_data = fw_event->event_data;
> +	Mpi2EventDataSasTopologyChangeList_t *event_data =
> +		(Mpi2EventDataSasTopologyChangeList_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
> @@ -5243,7 +5243,8 @@ _scsih_sas_device_status_change_event(struct MPT3SAS_ADAPTER *ioc,
>  	u64 sas_address;
>  	unsigned long flags;
>  	Mpi2EventDataSasDeviceStatusChange_t *event_data =
> -	    fw_event->event_data;
> +		(Mpi2EventDataSasDeviceStatusChange_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
> @@ -5339,6 +5340,7 @@ _scsih_sas_enclosure_dev_status_change_event(struct MPT3SAS_ADAPTER *ioc,
>  #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
>  		_scsih_sas_enclosure_dev_status_change_event_debug(ioc,
> +		     (Mpi2EventDataSasEnclDevStatusChange_t *)
>  		     fw_event->event_data);
>  #endif
>  }
> @@ -5363,7 +5365,9 @@ _scsih_sas_broadcast_primitive_event(struct MPT3SAS_ADAPTER *ioc,
>  	u32 termination_count;
>  	u32 query_count;
>  	Mpi2SCSITaskManagementReply_t *mpi_reply;
> -	Mpi2EventDataSasBroadcastPrimitive_t *event_data = fw_event->event_data;
> +	Mpi2EventDataSasBroadcastPrimitive_t *event_data =
> +		(Mpi2EventDataSasBroadcastPrimitive_t *)
> +		fw_event->event_data;
>  	u16 ioc_status;
>  	unsigned long flags;
>  	int r;
> @@ -5515,7 +5519,8 @@ static void
>  _scsih_sas_discovery_event(struct MPT3SAS_ADAPTER *ioc,
>  	struct fw_event_work *fw_event)
>  {
> -	Mpi2EventDataSasDiscovery_t *event_data = fw_event->event_data;
> +	Mpi2EventDataSasDiscovery_t *event_data =
> +		(Mpi2EventDataSasDiscovery_t *) fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK) {
> @@ -6001,7 +6006,9 @@ _scsih_sas_ir_config_change_event(struct MPT3SAS_ADAPTER *ioc,
>  	Mpi2EventIrConfigElement_t *element;
>  	int i;
>  	u8 foreign_config;
> -	Mpi2EventDataIrConfigChangeList_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrConfigChangeList_t *event_data =
> +		(Mpi2EventDataIrConfigChangeList_t *)
> +		fw_event->event_data;
>  
>  #ifdef CONFIG_SCSI_MPT3SAS_LOGGING
>  	if (ioc->logging_level & MPT_DEBUG_EVENT_WORK_TASK)
> @@ -6071,7 +6078,8 @@ _scsih_sas_ir_volume_event(struct MPT3SAS_ADAPTER *ioc,
>  	u16 handle;
>  	u32 state;
>  	int rc;
> -	Mpi2EventDataIrVolume_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrVolume_t *event_data =
> +		(Mpi2EventDataIrVolume_t *) fw_event->event_data;
>  
>  	if (ioc->shost_recovery)
>  		return;
> @@ -6154,7 +6162,8 @@ _scsih_sas_ir_physical_disk_event(struct MPT3SAS_ADAPTER *ioc,
>  	Mpi2ConfigReply_t mpi_reply;
>  	Mpi2SasDevicePage0_t sas_device_pg0;
>  	u32 ioc_status;
> -	Mpi2EventDataIrPhysicalDisk_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrPhysicalDisk_t *event_data =
> +		(Mpi2EventDataIrPhysicalDisk_t *) fw_event->event_data;
>  	u64 sas_address;
>  
>  	if (ioc->shost_recovery)
> @@ -6274,7 +6283,9 @@ static void
>  _scsih_sas_ir_operation_status_event(struct MPT3SAS_ADAPTER *ioc,
>  	struct fw_event_work *fw_event)
>  {
> -	Mpi2EventDataIrOperationStatus_t *event_data = fw_event->event_data;
> +	Mpi2EventDataIrOperationStatus_t *event_data =
> +		(Mpi2EventDataIrOperationStatus_t *)
> +		fw_event->event_data;
>  	static struct _raid_device *raid_device;
>  	unsigned long flags;
>  	u16 handle;
> @@ -7036,7 +7047,9 @@ _mpt3sas_fw_work(struct MPT3SAS_ADAPTER *ioc, struct fw_event_work *fw_event)
>  
>  	switch (fw_event->event) {
>  	case MPT3SAS_PROCESS_TRIGGER_DIAG:
> -		mpt3sas_process_trigger_data(ioc, fw_event->event_data);
> +		mpt3sas_process_trigger_data(ioc,
> +			(struct SL_WH_TRIGGERS_EVENT_DATA_T *)
> +			fw_event->event_data);
>  		break;
>  	case MPT3SAS_REMOVE_UNRESPONDING_DEVICES:
>  		while (scsi_host_in_recovery(ioc->shost) || ioc->shost_recovery)
> @@ -7194,18 +7207,11 @@ mpt3sas_scsih_event_callback(struct MPT3SAS_ADAPTER *ioc, u8 msix_index,
>  		return 1;
>  	}
>  
> -	fw_event = kzalloc(sizeof(struct fw_event_work), GFP_ATOMIC);
> -	if (!fw_event) {
> -		pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
> -		    ioc->name, __FILE__, __LINE__, __func__);
> -		return 1;
> -	}
>  	sz = le16_to_cpu(mpi_reply->EventDataLength) * 4;
> -	fw_event->event_data = kzalloc(sz, GFP_ATOMIC);
> -	if (!fw_event->event_data) {
> +	fw_event = kzalloc(sizeof(*fw_event) + sz, GFP_ATOMIC);
> +	if (!fw_event) {
>  		pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
>  		    ioc->name, __FILE__, __LINE__, __func__);
> -		kfree(fw_event);
>  		return 1;
>  	}
>  
> -- 
> 1.7.10.4
> 
> --
> 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
---end quoted text---

[PATCH v2 06/11] mptfusion: mark file-private functions as static

[Joe Lawrence]

Fixes the following sparse warnings:

  drivers/message/fusion/mptbase.c:7011:1: warning: symbol
    'mpt_SoftResetHandler' was not declared. Should it be static?

  drivers/message/fusion/mptsas.c:1578:23: warning: symbol
    'mptsas_refreshing_device_handles' was not declared. Should it be
    static?

  drivers/message/fusion/mptsas.c:3653:24: warning: symbol
    'mptsas_expander_add' was not declared. Should it be static?

  drivers/message/fusion/mptsas.c:5327:1: warning: symbol
    'mptsas_shutdown' was not declared. Should it be static?

  drivers/message/fusion/mptspi.c:624:1: warning: symbol
    'mptscsih_quiesce_raid' was not declared. Should it be static?

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/message/fusion/mptbase.c |    2 +-
 drivers/message/fusion/mptsas.c  |    6 +++---
 drivers/message/fusion/mptspi.c  |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index ebc0af7..ea30033 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -7007,7 +7007,7 @@ EXPORT_SYMBOL(mpt_halt_firmware);
  *	IOC doesn't reply to any outstanding request. This will transfer IOC
  *	to READY state.
  **/
-int
+static int
 mpt_SoftResetHandler(MPT_ADAPTER *ioc, int sleepFlag)
 {
 	int		 rc;
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 711fcb5..54b51a9 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1575,7 +1575,7 @@ mptsas_del_end_device(MPT_ADAPTER *ioc, struct mptsas_phyinfo *phy_info)
 	mptsas_port_delete(ioc, phy_info->port_details);
 }
 
-struct mptsas_phyinfo *
+static struct mptsas_phyinfo *
 mptsas_refreshing_device_handles(MPT_ADAPTER *ioc,
 	struct mptsas_devinfo *sas_device)
 {
@@ -3648,7 +3648,7 @@ mptsas_send_expander_event(struct fw_event_work *fw_event)
  * @handle:
  *
  */
-struct mptsas_portinfo *
+static struct mptsas_portinfo *
 mptsas_expander_add(MPT_ADAPTER *ioc, u16 handle)
 {
 	struct mptsas_portinfo buffer, *port_info;
@@ -5321,7 +5321,7 @@ mptsas_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return error;
 }
 
-void
+static void
 mptsas_shutdown(struct pci_dev *pdev)
 {
 	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
diff --git a/drivers/message/fusion/mptspi.c b/drivers/message/fusion/mptspi.c
index 49d1133..7b4db9a 100644
--- a/drivers/message/fusion/mptspi.c
+++ b/drivers/message/fusion/mptspi.c
@@ -620,7 +620,7 @@ static void mptspi_read_parameters(struct scsi_target *starget)
 	spi_width(starget) = (nego & MPI_SCSIDEVPAGE0_NP_WIDE) ? 1 : 0;
 }
 
-int
+static int
 mptscsih_quiesce_raid(MPT_SCSI_HOST *hd, int quiesce, u8 channel, u8 id)
 {
 	MPT_ADAPTER	*ioc = hd->ioc;
-- 
1.7.10.4

Re: [PATCH v2 06/11] mptfusion: mark file-private functions as static

[Christoph Hellwig]

Sreekanth, can you give me a review for this (and anything else in the series
to avoid repeating myself too often)

Re: [PATCH v2 06/11] mptfusion: mark file-private functions as static

[Christoph Hellwig]

Hi Sreekanth,

should I treat your replies as an Reviewd-by: or Acked-by: tag?

[PATCH v2 07/11] mptfusion: remove redundant kfree checks

[Joe Lawrence]

Fixes the following smatch warnings:

  drivers/message/fusion/mptfc.c:529 mptfc_target_destroy() info:
    redundant null check on starget->hostdata calling kfree()

  drivers/message/fusion/mptspi.c:465 mptspi_target_destroy() info:
    redundant null check on starget->hostdata calling kfree()

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/message/fusion/mptfc.c  |    3 +--
 drivers/message/fusion/mptspi.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
index 02a3eef..21ac845 100644
--- a/drivers/message/fusion/mptfc.c
+++ b/drivers/message/fusion/mptfc.c
@@ -525,8 +525,7 @@ mptfc_target_destroy(struct scsi_target *starget)
 		if (ri)	/* better be! */
 			ri->starget = NULL;
 	}
-	if (starget->hostdata)
-		kfree(starget->hostdata);
+	kfree(starget->hostdata);
 	starget->hostdata = NULL;
 }
 
diff --git a/drivers/message/fusion/mptspi.c b/drivers/message/fusion/mptspi.c
index 7b4db9a..787933d 100644
--- a/drivers/message/fusion/mptspi.c
+++ b/drivers/message/fusion/mptspi.c
@@ -461,8 +461,7 @@ static int mptspi_target_alloc(struct scsi_target *starget)
 static void
 mptspi_target_destroy(struct scsi_target *starget)
 {
-	if (starget->hostdata)
-		kfree(starget->hostdata);
+	kfree(starget->hostdata);
 	starget->hostdata = NULL;
 }
 
-- 
1.7.10.4

[PATCH v2 08/11] mptfusion: use memdup_user

[Joe Lawrence]

Let memdup_user handle the kmalloc, copy_from_user and error checking
kfree code.

Spotted by the following smatch (false positive) warning:

  drivers/message/fusion/mptctl.c:1369 mptctl_getiocinfo() warn:
    possible info leak 'karg'

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/message/fusion/mptctl.c |   18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
index 8a050e8..b0a892a 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -1261,19 +1261,11 @@ mptctl_getiocinfo (unsigned long arg, unsigned int data_size)
 	else
 		return -EFAULT;
 
-	karg = kmalloc(data_size, GFP_KERNEL);
-	if (karg == NULL) {
-		printk(KERN_ERR MYNAM "%s::mpt_ioctl_iocinfo() @%d - no memory available!\n",
-				__FILE__, __LINE__);
-		return -ENOMEM;
-	}
-
-	if (copy_from_user(karg, uarg, data_size)) {
-		printk(KERN_ERR MYNAM "%s@%d::mptctl_getiocinfo - "
-			"Unable to read in mpt_ioctl_iocinfo struct @ %p\n",
-				__FILE__, __LINE__, uarg);
-		kfree(karg);
-		return -EFAULT;
+	karg = memdup_user(uarg, data_size);
+	if (IS_ERR(karg)) {
+		printk(KERN_ERR MYNAM "%s@%d::mpt_ioctl_iocinfo() - memdup_user returned error [%ld]\n",
+				__FILE__, __LINE__, PTR_ERR(karg));
+		return PTR_ERR(karg);
 	}
 
 	if (((iocnum = mpt_verify_adapter(karg->hdr.iocnum, &ioc)) < 0) ||
-- 
1.7.10.4

[PATCH v2 09/11] mptfusion: make adapter prod_name[] a pointer

[Joe Lawrence]

The struct _MPT_ADAPTER doesn't need a full copy of the product string,
so prod_name can point to the string literal storage that the driver
already provides.

Avoids the following smatch warning:

  drivers/message/fusion/mptbase.c:2858 MptDisplayIocCapabilities()
    warn: this array is probably non-NULL. 'ioc->prod_name'

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/message/fusion/mptbase.c |   11 +++++------
 drivers/message/fusion/mptbase.h |    2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index ea30033..9d4c782 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1408,8 +1408,8 @@ mpt_verify_adapter(int iocid, MPT_ADAPTER **iocpp)
  *	in /proc/mpt/summary and /sysfs/class/scsi_host/host<X>/version_product
  *
  **/
-static void
-mpt_get_product_name(u16 vendor, u16 device, u8 revision, char *prod_name)
+static const char*
+mpt_get_product_name(u16 vendor, u16 device, u8 revision)
 {
 	char *product_str = NULL;
 
@@ -1635,8 +1635,7 @@ mpt_get_product_name(u16 vendor, u16 device, u8 revision, char *prod_name)
 	}
 
  out:
-	if (product_str)
-		sprintf(prod_name, "%s", product_str);
+	return product_str;
 }
 
 /**
@@ -1887,8 +1886,8 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "facts @ %p, pfacts[0] @ %p\n",
 	    ioc->name, &ioc->facts, &ioc->pfacts[0]));
 
-	mpt_get_product_name(pdev->vendor, pdev->device, pdev->revision,
-			     ioc->prod_name);
+	ioc->prod_name = mpt_get_product_name(pdev->vendor, pdev->device,
+					      pdev->revision);
 
 	switch (pdev->device)
 	{
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index 76c05bc..78c8104 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -605,7 +605,7 @@ typedef struct _MPT_ADAPTER
 	int			 id;		/* Unique adapter id N {0,1,2,...} */
 	int			 pci_irq;	/* This irq           */
 	char			 name[MPT_NAME_LENGTH];	/* "iocN"             */
-	char			 prod_name[MPT_NAME_LENGTH];	/* "LSIFC9x9"         */
+	const char		 *prod_name;	/* "LSIFC9x9"         */
 #ifdef CONFIG_FUSION_LOGGING
 	/* used in mpt_display_event_info */
 	char			 evStr[EVENT_DESCR_STR_SZ];
-- 
1.7.10.4

[PATCH v2 10/11] mptfusion: combine fw_event_work and its event_data

[Joe Lawrence]

Tack the firmware reply event_data payload to the end of its
corresponding struct fw_event_work allocation.  Rework fw_event_work
allocation calculations to include the event_data size where
appropriate.

This clarifies the code a bit and avoids the following smatch warnings:

  drivers/message/fusion/mptsas.c:1003 mptsas_queue_device_delete()
    error: memcpy() 'fw_event->event_data' too small (29 vs 36)

  drivers/message/fusion/mptsas.c:1017 mptsas_queue_rescan() error: not
    allocating enough data 168 vs 160

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/message/fusion/mptsas.c |   16 ++++++----------
 drivers/message/fusion/mptsas.h |    2 +-
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 54b51a9..5454d838 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -990,11 +990,10 @@ mptsas_queue_device_delete(MPT_ADAPTER *ioc,
 	MpiEventDataSasDeviceStatusChange_t *sas_event_data)
 {
 	struct fw_event_work *fw_event;
-	int sz;
 
-	sz = offsetof(struct fw_event_work, event_data) +
-	    sizeof(MpiEventDataSasDeviceStatusChange_t);
-	fw_event = kzalloc(sz, GFP_ATOMIC);
+	fw_event = kzalloc(sizeof(*fw_event) +
+			   sizeof(MpiEventDataSasDeviceStatusChange_t),
+			   GFP_ATOMIC);
 	if (!fw_event) {
 		printk(MYIOC_s_WARN_FMT "%s: failed at (line=%d)\n",
 		    ioc->name, __func__, __LINE__);
@@ -1011,10 +1010,8 @@ static void
 mptsas_queue_rescan(MPT_ADAPTER *ioc)
 {
 	struct fw_event_work *fw_event;
-	int sz;
 
-	sz = offsetof(struct fw_event_work, event_data);
-	fw_event = kzalloc(sz, GFP_ATOMIC);
+	fw_event = kzalloc(sizeof(*fw_event), GFP_ATOMIC);
 	if (!fw_event) {
 		printk(MYIOC_s_WARN_FMT "%s: failed at (line=%d)\n",
 		    ioc->name, __func__, __LINE__);
@@ -4983,7 +4980,7 @@ static int
 mptsas_event_process(MPT_ADAPTER *ioc, EventNotificationReply_t *reply)
 {
 	u32 event = le32_to_cpu(reply->Event);
-	int sz, event_data_sz;
+	int event_data_sz;
 	struct fw_event_work *fw_event;
 	unsigned long delay;
 
@@ -5093,8 +5090,7 @@ mptsas_event_process(MPT_ADAPTER *ioc, EventNotificationReply_t *reply)
 
 	event_data_sz = ((reply->MsgLength * 4) -
 	    offsetof(EventNotificationReply_t, Data));
-	sz = offsetof(struct fw_event_work, event_data) + event_data_sz;
-	fw_event = kzalloc(sz, GFP_ATOMIC);
+	fw_event = kzalloc(sizeof(*fw_event) + event_data_sz, GFP_ATOMIC);
 	if (!fw_event) {
 		printk(MYIOC_s_WARN_FMT "%s: failed at (line=%d)\n", ioc->name,
 		 __func__, __LINE__);
diff --git a/drivers/message/fusion/mptsas.h b/drivers/message/fusion/mptsas.h
index 57e86ab..c396483 100644
--- a/drivers/message/fusion/mptsas.h
+++ b/drivers/message/fusion/mptsas.h
@@ -110,7 +110,7 @@ struct fw_event_work {
 	MPT_ADAPTER	*ioc;
 	u32			event;
 	u8			retries;
-	u8			__attribute__((aligned(4))) event_data[1];
+	char			event_data[0] __aligned(4);
 };
 
 struct mptsas_discovery_event {
-- 
1.7.10.4

[PATCH v2 11/11] mptfusion: tweak null pointer checks

[Joe Lawrence]

Fixes the following smatch warnings:

  drivers/message/fusion/mptbase.c:652 mptbase_reply() warn: variable
    dereferenced before check 'reply' (see line 639)

      [JL: No-brainer, the enclosing switch statement dereferences
       reply, so we can't get here unless reply is valid.]

  drivers/message/fusion/mptsas.c:1255 mptsas_taskmgmt_complete() error:
    we previously assumed 'pScsiTmReply' could be null (see line 1227)

      [HCH: Reading the code in mptsas_taskmgmt_complete it's pretty
       obvious that it can't do anything useful if mr/pScsiTmReply are
       NULL, so I suspect it would be best to just return at the
       beginning of the function.

       I'd love to understand if it actually could ever be zero, which I
       doubt.  Maybe the LSI people can shed some light on that?]

  drivers/message/fusion/mptsas.c:3888 mptsas_not_responding_devices()
    error: we previously assumed 'port_info->phy_info' could be null
    (see line 3875)

      [HCH: It's pretty obvious from reading mptsas_sas_io_unit_pg0 that
       we never register a port_info with a NULL phy_info in the lists,
       so all NULL checks on it could be deleted.]

  drivers/message/fusion/mptscsih.c:1284 mptscsih_info() error:
    we previously assumed 'h' could be null (see line 1274)

      [HCH: shost_priv can't return NULL, so the if (h) should be
       removed.]

  drivers/message/fusion/mptscsih.c:1388 mptscsih_qcmd() error: we
    previously assumed 'vdevice' could be null (see line 1373)

      [HCH: vdevice can't ever be NULL here, it's allocated in
       ->slave_alloc and thus guaranteed to be around when
       ->queuecommand is called.]

Signed-off-by: Joe Lawrence <joe.lawrence@stratus.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Sreekanth Reddy <Sreekanth.Reddy@avagotech.com>
---
 drivers/message/fusion/mptbase.c  |   10 +++----
 drivers/message/fusion/mptsas.c   |   52 ++++++++++++++++++-------------------
 drivers/message/fusion/mptscsih.c |   19 ++++++--------
 3 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 9d4c782..a896d94 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -649,12 +649,10 @@ mptbase_reply(MPT_ADAPTER *ioc, MPT_FRAME_HDR *req, MPT_FRAME_HDR *reply)
 	case MPI_FUNCTION_CONFIG:
 	case MPI_FUNCTION_SAS_IO_UNIT_CONTROL:
 		ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD;
-		if (reply) {
-			ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
-			memcpy(ioc->mptbase_cmds.reply, reply,
-			    min(MPT_DEFAULT_FRAME_SIZE,
-				4 * reply->u.reply.MsgLength));
-		}
+		ioc->mptbase_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
+		memcpy(ioc->mptbase_cmds.reply, reply,
+		    min(MPT_DEFAULT_FRAME_SIZE,
+			4 * reply->u.reply.MsgLength));
 		if (ioc->mptbase_cmds.status & MPT_MGMT_STATUS_PENDING) {
 			ioc->mptbase_cmds.status &= ~MPT_MGMT_STATUS_PENDING;
 			complete(&ioc->mptbase_cmds.done);
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 5454d838..e3494a6 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -1203,27 +1203,28 @@ mptsas_taskmgmt_complete(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf, MPT_FRAME_HDR *mr)
 	    "(mf = %p, mr = %p)\n", ioc->name, mf, mr));
 
 	pScsiTmReply = (SCSITaskMgmtReply_t *)mr;
-	if (pScsiTmReply) {
-		dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT
-		    "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n"
-		    "\ttask_type = 0x%02X, iocstatus = 0x%04X "
-		    "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, "
-		    "term_cmnds = %d\n", ioc->name,
-		    pScsiTmReply->Bus, pScsiTmReply->TargetID,
-		    pScsiTmReply->TaskType,
-		    le16_to_cpu(pScsiTmReply->IOCStatus),
-		    le32_to_cpu(pScsiTmReply->IOCLogInfo),
-		    pScsiTmReply->ResponseCode,
-		    le32_to_cpu(pScsiTmReply->TerminationCount)));
-
-		if (pScsiTmReply->ResponseCode)
-			mptscsih_taskmgmt_response_code(ioc,
-			pScsiTmReply->ResponseCode);
-	}
-
-	if (pScsiTmReply && (pScsiTmReply->TaskType ==
+	if (!pScsiTmReply)
+		return 0;
+
+	dtmprintk(ioc, printk(MYIOC_s_DEBUG_FMT
+	    "\tTaskMgmt completed: fw_channel = %d, fw_id = %d,\n"
+	    "\ttask_type = 0x%02X, iocstatus = 0x%04X "
+	    "loginfo = 0x%08X,\n\tresponse_code = 0x%02X, "
+	    "term_cmnds = %d\n", ioc->name,
+	    pScsiTmReply->Bus, pScsiTmReply->TargetID,
+	    pScsiTmReply->TaskType,
+	    le16_to_cpu(pScsiTmReply->IOCStatus),
+	    le32_to_cpu(pScsiTmReply->IOCLogInfo),
+	    pScsiTmReply->ResponseCode,
+	    le32_to_cpu(pScsiTmReply->TerminationCount)));
+
+	if (pScsiTmReply->ResponseCode)
+		mptscsih_taskmgmt_response_code(ioc,
+		pScsiTmReply->ResponseCode);
+
+	if (pScsiTmReply->TaskType ==
 	    MPI_SCSITASKMGMT_TASKTYPE_QUERY_TASK || pScsiTmReply->TaskType ==
-	     MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET)) {
+	     MPI_SCSITASKMGMT_TASKTYPE_ABRT_TASK_SET) {
 		ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_COMMAND_GOOD;
 		ioc->taskmgmt_cmds.status |= MPT_MGMT_STATUS_RF_VALID;
 		memcpy(ioc->taskmgmt_cmds.reply, mr,
@@ -3853,10 +3854,8 @@ retry_page:
 			phy_info = mptsas_find_phyinfo_by_sas_address(ioc,
 					sas_info->sas_address);
 
-			if (phy_info) {
-				mptsas_del_end_device(ioc, phy_info);
-				goto redo_device_scan;
-			}
+			mptsas_del_end_device(ioc, phy_info);
+			goto redo_device_scan;
 		} else
 			mptsas_volume_delete(ioc, sas_info->fw.id);
 	}
@@ -3867,9 +3866,8 @@ retry_page:
  redo_expander_scan:
 	list_for_each_entry(port_info, &ioc->sas_topology, list) {
 
-		if (port_info->phy_info &&
-		    (!(port_info->phy_info[0].identify.device_info &
-		    MPI_SAS_DEVICE_INFO_SMP_TARGET)))
+		if (!(port_info->phy_info[0].identify.device_info &
+		    MPI_SAS_DEVICE_INFO_SMP_TARGET))
 			continue;
 		found_expander = 0;
 		handle = 0xFFFF;
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index 2a1c6f2..6b36c7e 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1271,15 +1271,13 @@ mptscsih_info(struct Scsi_Host *SChost)
 
 	h = shost_priv(SChost);
 
-	if (h) {
-		if (h->info_kbuf == NULL)
-			if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL)
-				return h->info_kbuf;
-		h->info_kbuf[0] = '\0';
-
-		mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0);
-		h->info_kbuf[size-1] = '\0';
-	}
+	if (h->info_kbuf == NULL)
+		if ((h->info_kbuf = kmalloc(0x1000 /* 4Kb */, GFP_KERNEL)) == NULL)
+			return h->info_kbuf;
+	h->info_kbuf[0] = '\0';
+
+	mpt_print_ioc_summary(h->ioc, h->info_kbuf, &size, 0, 0);
+	h->info_kbuf[size-1] = '\0';
 
 	return h->info_kbuf;
 }
@@ -1368,8 +1366,7 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt)
 	/* Default to untagged. Once a target structure has been allocated,
 	 * use the Inquiry data to determine if device supports tagged.
 	 */
-	if (vdevice
-	    && (vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
+	if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
 	    && (SCpnt->device->tagged_supported)) {
 		scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ;
 		if (SCpnt->request && SCpnt->request->ioprio) {
-- 
1.7.10.4