Re: [PATCH 01/17] smartpqi: change aio sg processing

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:11AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Take advantage of controller improvements.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---
>  drivers/scsi/smartpqi/smartpqi_init.c |   68 
> +++--
>  1 file changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index 906f1aa..418f636 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -4263,48 +4263,58 @@ static int pqi_build_aio_sg_list(struct pqi_ctrl_info 
> *ctrl_info,
>   int i;
>   u16 iu_length;
>   int sg_count;
> - unsigned int num_sg_in_iu = 0;
> + bool chained;
> + unsigned int num_sg_in_iu;
> + unsigned int max_sg_per_iu;
>   struct scatterlist *sg;
>   struct pqi_sg_descriptor *sg_descriptor;
>  
>   sg_count = scsi_dma_map(scmd);
>   if (sg_count < 0)
>   return sg_count;
> +
> + iu_length = offsetof(struct pqi_aio_path_request, sg_descriptors) -
> + PQI_REQUEST_HEADER_LENGTH;
> + num_sg_in_iu = 0;
> +
>   if (sg_count == 0)
>   goto out;
>  
> - if (sg_count <= ctrl_info->max_sg_per_iu) {
> - sg_descriptor = &request->sg_descriptors[0];
> - scsi_for_each_sg(scmd, sg, sg_count, i) {
> - pqi_set_sg_descriptor(sg_descriptor, sg);
> - sg_descriptor++;
> - }
> - put_unaligned_le32(CISS_SG_LAST,
> - &request->sg_descriptors[sg_count - 1].flags);
> - num_sg_in_iu = sg_count;
> - } else {
> - sg_descriptor = &request->sg_descriptors[0];
> - put_unaligned_le64((u64)io_request->sg_chain_buffer_dma_handle,
> - &sg_descriptor->address);
> - put_unaligned_le32(sg_count * sizeof(*sg_descriptor),
> - &sg_descriptor->length);
> - put_unaligned_le32(CISS_SG_CHAIN, &sg_descriptor->flags);
> -
> - sg_descriptor = io_request->sg_chain_buffer;
> - scsi_for_each_sg(scmd, sg, sg_count, i) {
> - pqi_set_sg_descriptor(sg_descriptor, sg);
> - sg_descriptor++;
> + sg = scsi_sglist(scmd);
> + sg_descriptor = request->sg_descriptors;
> + max_sg_per_iu = ctrl_info->max_sg_per_iu - 1;
> + chained = false;
> + i = 0;
> +
> + while (1) {

Is there any compelling reason why you didn't use scsi_for_each_sg()
here? I don't see a reason for the while (1) construct.


> + pqi_set_sg_descriptor(sg_descriptor, sg);
> + if (!chained)
> + num_sg_in_iu++;
> + i++;
> + if (i == sg_count)
> + break;
> + sg_descriptor++;
> + if (i == max_sg_per_iu) {
> + put_unaligned_le64(
> + (u64)io_request->sg_chain_buffer_dma_handle,
> + &sg_descriptor->address);
> + put_unaligned_le32((sg_count - num_sg_in_iu)
> + * sizeof(*sg_descriptor),
> + &sg_descriptor->length);
> + put_unaligned_le32(CISS_SG_CHAIN,
> + &sg_descriptor->flags);
> + chained = true;
> + num_sg_in_iu++;
> + sg_descriptor = io_request->sg_chain_buffer;
>   }
> - put_unaligned_le32(CISS_SG_LAST,
> - &io_request->sg_chain_buffer[sg_count - 1].flags);
> - num_sg_in_iu = 1;
> - request->partial = 1;
> + sg = sg_next(sg);
>   }
>  
> -out:
> - iu_length = offsetof(struct pqi_aio_path_request, sg_descriptors) -
> - PQI_REQUEST_HEADER_LENGTH;
> + put_unaligned_le32(CISS_SG_LAST, &sg_descriptor->flags);
> + request->partial = chained;
>   iu_length += num_sg_in_iu * sizeof(*sg_descriptor);
> +
> +out:
>   put_unaligned_le16(iu_length, &request->header.iu_length);
>   request->num_sg_descriptors = num_sg_in_iu;
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at 

Re: [PATCH 02/17] smartpqi: change tmf macro names

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:17AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> small change to make code look cleaner
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/17] smartpqi: simplify spanning

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:24AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Removed the workaround for the transition to spanning.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 04/17] smartpqi: enhance drive offline informational message

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:30AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Made a couple of error messages more verbose.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---
>  drivers/scsi/smartpqi/smartpqi_init.c |5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index 9922e31..198a7c2 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -2298,11 +2298,16 @@ static inline void pqi_aio_path_disabled(struct 
> pqi_io_request *io_request)
>  static inline void pqi_take_device_offline(struct scsi_device *sdev)
>  {
>   struct pqi_ctrl_info *ctrl_info;
> + struct pqi_scsi_dev *device;
>  
>   if (scsi_device_online(sdev)) {
>   scsi_device_set_state(sdev, SDEV_OFFLINE);
>   ctrl_info = shost_to_hba(sdev->host);
>   schedule_delayed_work(&ctrl_info->rescan_work, 0);
> + device = sdev->hostdata;
> + dev_err(&ctrl_info->pci_dev->dev, "offlined scsi %d:%d:%d:%d\n",
> + ctrl_info->scsi_host->host_no, device->bus,
> + device->target, device->lun);
>   }
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

You /could/ move the definition of device into the if() block to
shorten it's scope, but that's purely cosmetical.

Otherwise,
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/17] smartpqi: enhance reset logic

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:36AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Eliminated timeout from LUN reset logic.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/17] smartpqi: change commonly used function to inline

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:42AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> A tiny tweak to convert a small, commonly used function to inline.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---
>  drivers/scsi/smartpqi/smartpqi_init.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index dbc8b40..2656124 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -316,7 +316,7 @@ static struct pqi_io_request *pqi_alloc_io_request(
>   return io_request;
>  }
>  
> -static void pqi_free_io_request(struct pqi_io_request *io_request)
> +static inline void pqi_free_io_request(struct pqi_io_request *io_request)
>  {
>   atomic_dec(&io_request->refcount);
>  }
> 

Did you check whether the compiler does this for you already?

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 07/17] smartpqi: add kdump support

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:48AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/17] smartpqi: correct controller offline issue

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:44:54AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Fixed a bug where the driver would not free all of the
> controller resources if the controller ever went offline.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Fixes: 6c223761e 'smartpqi: initial commit of Microsemi smartpqi driver' ?

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 09/17] smartpqi: correct event acknowledgment timeout issue

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:00AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> the driver no longer waits for the firmware to consume
> the event ack IU.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 11/17] smartpqi: minor tweaks to update time support

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:12AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> minor tweaks to update time support
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---
>  drivers/scsi/smartpqi/smartpqi_init.c |8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index 2759c90..a8b8671 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -588,10 +588,6 @@ static void pqi_update_time_worker(struct work_struct 
> *work)
>   ctrl_info = container_of(to_delayed_work(work), struct pqi_ctrl_info,
>   update_time_work);
>  
> - if (!ctrl_info) {
> - printk("%s: NULL controller pointer.\n", __func__);
> - return;
> - }
>   rc = pqi_write_current_time_to_host_wellness(ctrl_info);
>   if (rc)
>   dev_warn(&ctrl_info->pci_dev->dev,
> @@ -602,9 +598,9 @@ static void pqi_update_time_worker(struct work_struct 
> *work)
>  }
>  
>  static inline void pqi_schedule_update_time_worker(
> - struct pqi_ctrl_info *ctrl_info)
> + struct pqi_ctrl_info *ctrl_info)
>  {
> - schedule_delayed_work(&ctrl_info->update_time_work, 120);
> + schedule_delayed_work(&ctrl_info->update_time_work, 0);
>  }

Why are you using schedule_delayed_work() if you have a timeout of 0?
>From __queue_delayed_work():
1513 /*
1514  * If @delay is 0, queue @dwork->work immediately.  This is for
1515  * both optimization and correctness.  The earliest @timer can
1516  * expire is on the closest next tick and delayed_work users depend
1517  * on that there's no such delay when @delay is 0.
1518  */
1519 if (!delay) {
1520 __queue_work(cpu, wq, &dwork->work);
1521 return;
1522 }


-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 12/17] smartpqi: scsi queuecommand cleanup

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:18AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> minor cleanup of scsi queue command function
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 13/17] smartpqi: remove timeout for cache flush operations

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:24AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Some cache flush operations can take longer than the
> timeout value. Best to not impose a time limit to
> handle all cases.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---
>  drivers/scsi/smartpqi/smartpqi_init.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
> b/drivers/scsi/smartpqi/smartpqi_init.c
> index 87eb603..e7d7e99 100644
> --- a/drivers/scsi/smartpqi/smartpqi_init.c
> +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> @@ -407,7 +407,6 @@ static int pqi_identify_physical_device(struct 
> pqi_ctrl_info *ctrl_info,
>  }
>  
>  #define SA_CACHE_FLUSH_BUFFER_LENGTH 4
> -#define PQI_FLUSH_CACHE_TIMEOUT  (30 * 1000)
>  
>  static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info)
>  {
> @@ -434,7 +433,7 @@ static int pqi_flush_cache(struct pqi_ctrl_info 
> *ctrl_info)
>   goto out;
>  
>   rc = pqi_submit_raid_request_synchronous(ctrl_info, &request.header,
> - 0, NULL, PQI_FLUSH_CACHE_TIMEOUT);
> + 0, NULL, NO_TIMEOUT);
>  
>   pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1,
>   pci_direction);

IIRC I asked you to introduce this timeout, as you're calling
pqi_flush_cache() from the driver's shutdown callback and I still
doubt users like their shutdown's to hang indefinitely because cache
flush isn't working. Just my $0.02.

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 14/17] smartpqi: update Kconfig

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:30AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> The aacraid driver will not managage Microsemi
> smartpqi controllers, but will still manage
> older aacraid devices.
> 
> Updated help section.
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Reviewed-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/17] smartpqi: update maintainers

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:36AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> added Documentation/scsi/smartpqi.txt
> 
> Reviewed-by: Kevin Barnett 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Don Brace 
> ---
>  MAINTAINERS |1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f50fe50..d980685 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5447,6 +5447,7 @@ F:  drivers/scsi/smartpqi/Kconfig
>  F:   drivers/scsi/smartpqi/Makefile
>  F:   include/linux/cciss*.h
>  F:   include/uapi/linux/cciss*.h
> +F:   Documentation/scsi/smartpqi.txt

This should be foldet into the next patch where you actually add the
documentation ;-).

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 16/17] smartpqi: add smartpqi.txt

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:43AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Reviewed-by: Kevin Barnett 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Don Brace 
> ---

With the MAINTAINERS update patch folded in
Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 17/17] smartqi: bump driver version

2016-08-29 Thread Johannes Thumshirn
On Fri, Aug 26, 2016 at 11:45:49AM -0500, Don Brace wrote:
> From: Kevin Barnett 
> 
> Reviewed-by: Scott Teel 
> Reviewed-by: Scott Benesh 
> Signed-off-by: Kevin Barnett 
> Signed-off-by: Don Brace 
> ---

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] scsi: ufs: add missing header dependencies for tc-dwc-g210

2016-08-29 Thread Baoyou Xie
We get 2 warnings when build kernel with W=1:
drivers/scsi/ufs/tc-dwc-g210.c:261:5: warning: no previous prototype for 
'tc_dwc_g210_config_40_bit' [-Wmissing-prototypes]
drivers/scsi/ufs/tc-dwc-g210.c:293:5: warning: no previous prototype for 
'tc_dwc_g210_config_20_bit' [-Wmissing-prototypes]

In fact, these functions are declared in ufs/tc-dwc-g210.h, so this patch
add missing header dependencies

Signed-off-by: Baoyou Xie 
---
 drivers/scsi/ufs/tc-dwc-g210.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ufs/tc-dwc-g210.c b/drivers/scsi/ufs/tc-dwc-g210.c
index 70db6d9..dc03e47 100644
--- a/drivers/scsi/ufs/tc-dwc-g210.c
+++ b/drivers/scsi/ufs/tc-dwc-g210.c
@@ -15,6 +15,7 @@
 
 #include "ufshcd-dwc.h"
 #include "ufshci-dwc.h"
+#include "tc-dwc-g210.h"
 
 /**
  * tc_dwc_g210_setup_40bit_rmmi()
-- 
2.7.4

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


[PATCH] [SCSI] qla4xxx: mark symbols static where possible

2016-08-29 Thread Baoyou Xie
We get 1 warning when build kernel with W=1:
drivers/scsi/qla4xxx/ql4_nx.c:1846:10: warning: no previous prototype for 
'ql4_84xx_ipmdio_rd_reg' [-Wmissing-prototypes]

In fact, this function is only used in the file in which it is
declared and don't need a declaration, but can be made static.
so this patch marks this function with 'static'.

Signed-off-by: Baoyou Xie 
---
 drivers/scsi/qla4xxx/ql4_nx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c
index ae87d6c..06ddd13 100644
--- a/drivers/scsi/qla4xxx/ql4_nx.c
+++ b/drivers/scsi/qla4xxx/ql4_nx.c
@@ -1843,7 +1843,7 @@ static uint32_t ql4_84xx_poll_wait_for_ready(struct 
scsi_qla_host *ha,
return rval;
 }
 
-uint32_t ql4_84xx_ipmdio_rd_reg(struct scsi_qla_host *ha, uint32_t addr1,
+static uint32_t ql4_84xx_ipmdio_rd_reg(struct scsi_qla_host *ha, uint32_t 
addr1,
uint32_t addr3, uint32_t mask, uint32_t addr,
uint32_t *data_ptr)
 {
-- 
2.7.4

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


Re: [PATCH] scsi: move function declarationes to scsi_priv.h

2016-08-29 Thread Bart Van Assche

On 08/27/2016 09:59 AM, Baoyou Xie wrote:

We get 2 warnings about global functions without a declaration
in the scsi driver when building with W=1:
drivers/scsi/scsi_lib.c:467:6: warning: no previous prototype for 
'scsi_requeue_run_queue' [-Wmissing-prototypes]
drivers/scsi/scsi_lib.c:2609:6: warning: no previous prototype for 
'scsi_evt_thread' [-Wmissing-prototypes]

in fact, both functiones are declared in drivers/scsi/scsi_scan.c
but need to move the declarationes into scsi_priv.h.


Please fix the spelling in the patch title and description (functiones 
-> functions and declarationes -> declarations). Anyway:


Reviewed-by: Bart Van Assche 

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


Re: Oops when completing request on the wrong queue

2016-08-29 Thread Gabriel Krisman Bertazi
Jens Axboe  writes:
>> Can you try this patch? It's not perfect, but I'll be interested if it
>> makes a difference for you.
>

Hi Jens,

Sorry for the delay.  I just got back to this and have been running your
patch on top of 4.8 without a crash for over 1 hour.  I wanna give it
more time to make sure it's running properly, though.

Let me get back to you after a few more rounds of test.

> This one should handle the WARN_ON() for running the hw queue on the
> wrong CPU as well.

On the workaround you added to prevent WARN_ON, we surely need to
prevent blk_mq_hctx_next_cpu from scheduling dead cpus in the first
place, right..  How do you feel about the following RFC?  I know it's
not a complete fix, but it feels like a good improvement to me.

http://www.spinics.net/lists/linux-scsi/msg98608.html

-- 
Gabriel Krisman Bertazi

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


Re: Time to make dynamically allocated devt the default for scsi disks?

2016-08-29 Thread Bart Van Assche
On 08/14/2016 10:21 AM, James Bottomley wrote:
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index d3e852a..222771d 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3000,7 +3000,13 @@ static void sd_probe_async(void *data, async_cookie_t 
> cookie)
>   }
>  
>   blk_pm_runtime_init(sdp->request_queue, dev);
> - device_add_disk(dev, gd);
> + /*
> +  * previously the parent of the gendisk was the scsi device.  It
> +  * was moved to fix lifetime rules, so now we install a symlink
> +  * to the new location of the block class directory
> +  */
> + device_add_disk(&sdkp->dev, gd);
> + WARN_ON(sysfs_add_link_to_group(&dev->kobj, "block", &sdkp->dev.kobj, 
> "block"));
>   if (sdkp->capacity)
>   sd_dif_config_host(sdkp);
>  
> @@ -3142,6 +3148,7 @@ static int sd_remove(struct device *dev)
>  
>   async_synchronize_full_domain(&scsi_sd_pm_domain);
>   async_synchronize_full_domain(&scsi_sd_probe_domain);
> + sysfs_remove_link(&dev->kobj, "block");
>   device_del(&sdkp->dev);
>   del_gendisk(sdkp->disk);
>   sd_shutdown(dev);

Hello James,

Sorry that it took so long before I could test this patch and
the previous patch that was posted in this e-mail thread. But I
did so earlier this morning. What I see is that the following
warning message is reported frequently:

WARNING: CPU: 1 PID: 136 at drivers/scsi/sd.c:3009 sd_probe_async+0x1ce/0x1e0
Modules linked in: ib_srp libcrc32c scsi_transport_srp target_core_pscsi 
target_core_file ib_srpt target_core_iblock target_core_mod brd dm_multipath 
scsi_dh_rdac scsi_dh_emc scsi_dh_alua netconsole xt_CHECKSUM iptable_mangle 
ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat 
nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT xt_tcpudp 
tun ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter ip_tables 
x_tables af_packet bridge stp llc iscsi_ibft iscsi_boot_sysfs ib_ipoib rdma_ucm 
ib_ucm ib_uverbs ib_umad msr rdma_cm configfs ib_cm iw_cm mlx4_ib ib_core 
sb_edac edac_core x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm 
dm_mod irqbypass ipmi_ssif crct10dif_pclmul ipmi_devintf crc32_pclmul 
ghash_clmulni_intel aesni_intel iTCO_wdt aes_x86_64 lrw iTCO_vendor_support 
dcdbas gf128mul tg3 mlx4_core glue_helper ablk_helper ptp cryptd fjes ipmi_si 
pcspkr pps_core libphy ipmi_msghandler mei_me tpm_tis tpm_tis_core mei tpm 
lpc_ich shpchp mfd_core wmi button hid_generic usbhid mgag200 i2c_algo_bit 
drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm sr_mod 
cdrom crc32c_intel ehci_pci ehci_hcd usbcore usb_common sg [last unloaded: 
scsi_transport_srp]
CPU: 1 PID: 136 Comm: kworker/u64:8 Tainted: GW   4.8.0-rc4-dbg+ #2
Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.0.2 11/17/2014
Workqueue: events_unbound async_run_entry_fn
Call Trace:
 [] dump_stack+0x68/0x93
 [] __warn+0xc6/0xe0
 [] warn_slowpath_null+0x18/0x20
 [] sd_probe_async+0x1ce/0x1e0
 [] async_run_entry_fn+0x34/0x140
 [] process_one_work+0x1f5/0x690
 [] worker_thread+0x49/0x490
 [] kthread+0xea/0x100
 [] ret_from_fork+0x1f/0x40

The test I ran is available at https://github.com/bvanassche/srp-test.

Bart.

N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: Oops when completing request on the wrong queue

2016-08-29 Thread Jens Axboe

On 08/29/2016 12:06 PM, Gabriel Krisman Bertazi wrote:

Jens Axboe  writes:

Can you try this patch? It's not perfect, but I'll be interested if it
makes a difference for you.




Hi Jens,

Sorry for the delay.  I just got back to this and have been running your
patch on top of 4.8 without a crash for over 1 hour.  I wanna give it
more time to make sure it's running properly, though.

Let me get back to you after a few more rounds of test.


Thanks, sounds good. The patches have landed in mainline too.


This one should handle the WARN_ON() for running the hw queue on the
wrong CPU as well.


On the workaround you added to prevent WARN_ON, we surely need to
prevent blk_mq_hctx_next_cpu from scheduling dead cpus in the first
place, right..  How do you feel about the following RFC?  I know it's
not a complete fix, but it feels like a good improvement to me.

http://www.spinics.net/lists/linux-scsi/msg98608.html


But we can't completely prevent it, and I don't think we have to. I just
don't want to trigger a warning for something that's a valid condition.
I want the warning to trigger if this happens without the CPU going
offline, since then it's indicative of a real bug in the mapping. Your
patch isn't going to prevent it either - it'll shrink the window, at the
expense of making blk_mq_hctx_next_cpu() more expensive. So I don't
think it's worthwhile.


--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH -next] ibmvscsis: Use list_move_tail instead of list_del/list_add_tail

2016-08-29 Thread Tyrel Datwyler
On 07/22/2016 07:03 AM, Wei Yongjun wrote:
> Using list_move_tail() instead of list_del() + list_add_tail().
> 
> Signed-off-by: Wei Yongjun 

Reviewed-by: Tyrel Datwyler 

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


RE: [PATCH 06/17] smartpqi: change commonly used function to inline

2016-08-29 Thread Don Brace
> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Monday, August 29, 2016 4:07 AM
> To: Don Brace
> Cc: j...@linux.vnet.ibm.com; Viswas G; Mahesh Rajashekhara;
> h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 06/17] smartpqi: change commonly used function to
> inline
> 
> EXTERNAL EMAIL
> 
> 
> On Fri, Aug 26, 2016 at 11:44:42AM -0500, Don Brace wrote:
> > From: Kevin Barnett 
> >
> > A tiny tweak to convert a small, commonly used function to inline.
> >
> > Reviewed-by: Scott Teel 
> > Reviewed-by: Scott Benesh 
> > Signed-off-by: Kevin Barnett 
> > Signed-off-by: Don Brace 
> > ---
> >  drivers/scsi/smartpqi/smartpqi_init.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
> b/drivers/scsi/smartpqi/smartpqi_init.c
> > index dbc8b40..2656124 100644
> > --- a/drivers/scsi/smartpqi/smartpqi_init.c
> > +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> > @@ -316,7 +316,7 @@ static struct pqi_io_request *pqi_alloc_io_request(
> >   return io_request;
> >  }
> >
> > -static void pqi_free_io_request(struct pqi_io_request *io_request)
> > +static inline void pqi_free_io_request(struct pqi_io_request *io_request)
> >  {
> >   atomic_dec(&io_request->refcount);
> >  }
> >
> 
> Did you check whether the compiler does this for you already?

You are correct, it does. I will redact this patch.

> 
> --
> Johannes Thumshirn  Storage
> jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 15/17] smartpqi: update maintainers

2016-08-29 Thread Don Brace
> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Monday, August 29, 2016 4:42 AM
> To: Don Brace
> Cc: j...@linux.vnet.ibm.com; Viswas G; Mahesh Rajashekhara;
> h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 15/17] smartpqi: update maintainers
> 
> EXTERNAL EMAIL
> 
> 
> On Fri, Aug 26, 2016 at 11:45:36AM -0500, Don Brace wrote:
> > From: Kevin Barnett 
> >
> > added Documentation/scsi/smartpqi.txt
> >
> > Reviewed-by: Kevin Barnett 
> > Reviewed-by: Scott Benesh 
> > Signed-off-by: Don Brace 
> > ---
> >  MAINTAINERS |1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f50fe50..d980685 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5447,6 +5447,7 @@ F:  drivers/scsi/smartpqi/Kconfig
> >  F:   drivers/scsi/smartpqi/Makefile
> >  F:   include/linux/cciss*.h
> >  F:   include/uapi/linux/cciss*.h
> > +F:   Documentation/scsi/smartpqi.txt
> 
> This should be foldet into the next patch where you actually add the
> documentation ;-).

I squash smartpqi-add-smartpqi.txt and smartpqi-update-maintainers

Thanks,
Don
> 
> --
> Johannes Thumshirn  Storage
> jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 11/17] smartpqi: minor tweaks to update time support

2016-08-29 Thread Don Brace
> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Monday, August 29, 2016 4:34 AM
> To: Don Brace
> Cc: j...@linux.vnet.ibm.com; Viswas G; Mahesh Rajashekhara;
> h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 11/17] smartpqi: minor tweaks to update time support
> 
> EXTERNAL EMAIL
> 
> 


> >
> >  static inline void pqi_schedule_update_time_worker(
> > - struct pqi_ctrl_info *ctrl_info)
> > + struct pqi_ctrl_info *ctrl_info)
> >  {
> > - schedule_delayed_work(&ctrl_info->update_time_work, 120);
> > + schedule_delayed_work(&ctrl_info->update_time_work, 0);
> >  }
> 
> Why are you using schedule_delayed_work() if you have a timeout of 0?
> From __queue_delayed_work():
> 1513 /*
> 1514  * If @delay is 0, queue @dwork->work immediately.  This is for
> 1515  * both optimization and correctness.  The earliest @timer can
> 1516  * expire is on the closest next tick and delayed_work users 
> depend
> 1517  * on that there's no such delay when @delay is 0.
> 1518  */
> 1519 if (!delay) {
> 1520 __queue_work(cpu, wq, &dwork->work);
> 1521 return;
> 1522 }
> 
> 
> --
> Johannes Thumshirn  Storage
> jthumsh...@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

The reason is that we want it run immediately (or close to immediately).

The code could be restructured to avoid calling this function with a 0, but it 
would result in more code and no benefit.

Kevin


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


RE: [PATCH 13/17] smartpqi: remove timeout for cache flush operations

2016-08-29 Thread Don Brace
> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Monday, August 29, 2016 4:40 AM
> To: Don Brace
> Cc: j...@linux.vnet.ibm.com; Viswas G; Mahesh Rajashekhara;
> h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 13/17] smartpqi: remove timeout for cache flush
> operations
> 
> EXTERNAL EMAIL
> 
> 
> On Fri, Aug 26, 2016 at 11:45:24AM -0500, Don Brace wrote:
> > From: Kevin Barnett 
> >
> > Some cache flush operations can take longer than the
> > timeout value. Best to not impose a time limit to
> > handle all cases.
> >
> > Reviewed-by: Scott Teel 
> > Reviewed-by: Scott Benesh 
> > Signed-off-by: Kevin Barnett 
> > Signed-off-by: Don Brace 
> > ---
> >  drivers/scsi/smartpqi/smartpqi_init.c |3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
> b/drivers/scsi/smartpqi/smartpqi_init.c
> > index 87eb603..e7d7e99 100644
> > --- a/drivers/scsi/smartpqi/smartpqi_init.c
> > +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> > @@ -407,7 +407,6 @@ static int pqi_identify_physical_device(struct
> pqi_ctrl_info *ctrl_info,
> >  }
> >
> >  #define SA_CACHE_FLUSH_BUFFER_LENGTH 4
> > -#define PQI_FLUSH_CACHE_TIMEOUT  (30 * 1000)
> >
> >  static int pqi_flush_cache(struct pqi_ctrl_info *ctrl_info)
> >  {
> > @@ -434,7 +433,7 @@ static int pqi_flush_cache(struct pqi_ctrl_info
> *ctrl_info)
> >   goto out;
> >
> >   rc = pqi_submit_raid_request_synchronous(ctrl_info, &request.header,
> > - 0, NULL, PQI_FLUSH_CACHE_TIMEOUT);
> > + 0, NULL, NO_TIMEOUT);
> >
> >   pqi_pci_unmap(ctrl_info->pci_dev, request.sg_descriptors, 1,
> >   pci_direction);
> 
> IIRC I asked you to introduce this timeout, as you're calling
> pqi_flush_cache() from the driver's shutdown callback and I still
> doubt users like their shutdown's to hang indefinitely because cache
> flush isn't working. Just my $0.02.
> 

The issue is that we do not know how long to wait. The cache module has gotten
quite large and some flush operations can take longer to complete than others.
That is why we took the timeout value out.

Thanks,
Don Brace
ESC - Smart Storage
Microsemi Corporation
> --
> Johannes Thumshirn  Storage
> jthumsh...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 01/17] smartpqi: change aio sg processing

2016-08-29 Thread Don Brace
> -Original Message-
> From: Johannes Thumshirn [mailto:jthumsh...@suse.de]
> Sent: Monday, August 29, 2016 3:54 AM
> To: Don Brace
> Cc: j...@linux.vnet.ibm.com; Viswas G; Mahesh Rajashekhara;
> h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; Scott Benesh;
> elli...@hpe.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 01/17] smartpqi: change aio sg processing
> 
> EXTERNAL EMAIL
> 
> 
> On Fri, Aug 26, 2016 at 11:44:11AM -0500, Don Brace wrote:
> > From: Kevin Barnett 
> >
> > Take advantage of controller improvements.
> >
> > Reviewed-by: Scott Teel 
> > Reviewed-by: Scott Benesh 
> > Signed-off-by: Kevin Barnett 
> > Signed-off-by: Don Brace 
> > ---
> >  drivers/scsi/smartpqi/smartpqi_init.c |   68 +++
> --
> >  1 file changed, 39 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
> b/drivers/scsi/smartpqi/smartpqi_init.c
> > index 906f1aa..418f636 100644
> > --- a/drivers/scsi/smartpqi/smartpqi_init.c
> > +++ b/drivers/scsi/smartpqi/smartpqi_init.c
> > @@ -4263,48 +4263,58 @@ static int pqi_build_aio_sg_list(struct
> pqi_ctrl_info *ctrl_info,
> >   int i;
> >   u16 iu_length;
> >   int sg_count;
> > - unsigned int num_sg_in_iu = 0;
> > + bool chained;
> > + unsigned int num_sg_in_iu;
> > + unsigned int max_sg_per_iu;
> >   struct scatterlist *sg;
> >   struct pqi_sg_descriptor *sg_descriptor;
> >
> >   sg_count = scsi_dma_map(scmd);
> >   if (sg_count < 0)
> >   return sg_count;
> > +
> > + iu_length = offsetof(struct pqi_aio_path_request, sg_descriptors) -
> > + PQI_REQUEST_HEADER_LENGTH;
> > + num_sg_in_iu = 0;
> > +
> >   if (sg_count == 0)
> >   goto out;
> >
> > - if (sg_count <= ctrl_info->max_sg_per_iu) {
> > - sg_descriptor = &request->sg_descriptors[0];
> > - scsi_for_each_sg(scmd, sg, sg_count, i) {
> > - pqi_set_sg_descriptor(sg_descriptor, sg);
> > - sg_descriptor++;
> > - }
> > - put_unaligned_le32(CISS_SG_LAST,
> > - &request->sg_descriptors[sg_count - 1].flags);
> > - num_sg_in_iu = sg_count;
> > - } else {
> > - sg_descriptor = &request->sg_descriptors[0];
> > - put_unaligned_le64((u64)io_request-
> >sg_chain_buffer_dma_handle,
> > - &sg_descriptor->address);
> > - put_unaligned_le32(sg_count * sizeof(*sg_descriptor),
> > - &sg_descriptor->length);
> > - put_unaligned_le32(CISS_SG_CHAIN, &sg_descriptor->flags);
> > -
> > - sg_descriptor = io_request->sg_chain_buffer;
> > - scsi_for_each_sg(scmd, sg, sg_count, i) {
> > - pqi_set_sg_descriptor(sg_descriptor, sg);
> > - sg_descriptor++;
> > + sg = scsi_sglist(scmd);
> > + sg_descriptor = request->sg_descriptors;
> > + max_sg_per_iu = ctrl_info->max_sg_per_iu - 1;
> > + chained = false;
> > + i = 0;
> > +
> > + while (1) {
> 
> Is there any compelling reason why you didn't use scsi_for_each_sg()
> here? I don't see a reason for the while (1) construct.
> 
The PQI chaining makes using scsi_for_each_sg a little more difficult to
maintain. We would prefer to leave the code as is.

> 
> > + pqi_set_sg_descriptor(sg_descriptor, sg);
> > + if (!chained)
> > + num_sg_in_iu++;
> > + i++;
> > + if (i == sg_count)
> > + break;
> > + sg_descriptor++;
> > + if (i == max_sg_per_iu) {
> > + put_unaligned_le64(
> > + (u64)io_request->sg_chain_buffer_dma_handle,
> > + &sg_descriptor->address);
> > + put_unaligned_le32((sg_count - num_sg_in_iu)
> > + * sizeof(*sg_descriptor),
> > + &sg_descriptor->length);
> > + put_unaligned_le32(CISS_SG_CHAIN,
> > + &sg_descriptor->flags);
> > + chained = true;
> > + num_sg_in_iu++;
> > + sg_descriptor = io_request->sg_chain_buffer;
> >   }
> > - put_unaligned_le32(CISS_SG_LAST,
> > - &io_request->sg_chain_buffer[sg_count - 1].flags);
> > - num_sg_in_iu = 1;
> > - request->partial = 1;
> > + sg = sg_next(sg);
> >   }
> >
> > -out:
> > - iu_length = offsetof(struct pqi_aio_path_request, sg_descriptors) -
> > - PQI_REQUEST_HEADER_LENGTH;
> > + put_unaligned_le32(CISS_SG_LAST, &sg_descriptor->flags);
> > + request->partial = chained;
> >   iu_length += num_sg_in_iu * sizeof(*sg_descriptor);
> > +
> > +out:
> >   put_unaligned_le16(iu_length, &request->header.iu_le