linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: John Garry <john.garry@huawei.com>,
	JBottomley@odin.com, martin.petersen@oracle.com
Cc: linuxarm@huawei.com, zhangfei.gao@linaro.org,
	xuwei5@hisilicon.com, john.garry2@mail.dcu.ie,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure()
Date: Thu, 18 Feb 2016 08:40:26 +0100	[thread overview]
Message-ID: <56C5756A.9060700@suse.de> (raw)
In-Reply-To: <56C354A5.6080503@huawei.com>

On 02/16/2016 05:56 PM, John Garry wrote:
> On 16/02/2016 15:33, Hannes Reinecke wrote:
>> On 02/16/2016 01:22 PM, John Garry wrote:
>>> In high-datarate aging tests, it is found that
>>> the SCSI framework can periodically
>>> issue lu resets to the device. This is because scsi
>>> commands begin to timeout. It is found that TASK SET
>>> FULL may be returned many times for the same command,
>>> causing the timeouts.
>>> To overcome this, the queue depth for the device needs
>>> to be reduced to 64 (from 256, set in
>>> sas_slave_configure()).
>>>
>> Hmm. TASK SET FULL should cause the queue depth to be reduced
>> automatically, no?
>>
>> Cheers,
>>
>> Hannes
>>
> 
> I need to double-check if Task set full reduces the depth, I don't
> think it does.
> 
> Regardless I found we were getting a combination of commands being
> retried due to Task Set Full and also SAS_QUEUE_FULL errors. For
> sure the SAS_QUEUE_FULL task errors reduce the queue depth in
> scsi_track_queue_full(). However I found it to be very slow in
> tracking, and we were getting commands timing out before the queue
> depth fell enough.
> 
> It would be nice to change default queue depth in
> sas_slave_configure() to a lower value so we can avoid this patch,
> but I am not sure if other vendor's HBA performance would be
> affected. From looking at the history of sas_slave_configure(), it
> would seem the value of 256 was inherited from mpt2sas driver, so
> I'm not sure.
> 
Well, the classical thing would be to associate each request tag
with a SAS task; or, in your case, associate each slot index with a
request tag.
You probably would need to reserve some slots for TMFs, ie you'd
need to decrease the resulting ->can_queue variable by that.
But once you've done that you shouldn't hit any QUEUE_FULL issues,
as the block layer will ensure that no tags will be reused while the
command is in flight.
Plus this is something you really need to be doing if you ever
consider moving to scsi-mq ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

  reply	other threads:[~2016-02-18  7:40 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-16 12:22 [PATCH 0/6] hisi_sas: add abort and retry feature John Garry
2016-02-16 12:22 ` [PATCH 1/6] hisi_sas: add TMF_RESP_FUNC_SUCC check John Garry
2016-02-16 15:20   ` Hannes Reinecke
2016-02-16 12:22 ` [PATCH 2/6] hisi_sas: add hisi_sas_slot_abort() John Garry
2016-02-16 15:22   ` Hannes Reinecke
2016-02-16 15:41     ` John Garry
2016-02-18  9:30       ` John Garry
2016-02-16 12:22 ` [PATCH 3/6] hisi_sas: use slot abort in v1 hw John Garry
2016-02-16 15:31   ` Hannes Reinecke
2016-02-16 16:13     ` John Garry
2016-02-18  7:16       ` Hannes Reinecke
2016-02-18  9:52         ` John Garry
2016-02-16 12:22 ` [PATCH 4/6] hisi_sas: use slot abort in v2 hw John Garry
2016-02-16 15:32   ` Hannes Reinecke
2016-02-16 16:58     ` John Garry
2016-02-16 12:22 ` [PATCH 5/6] hisi_sas: add hisi_sas_slave_configure() John Garry
2016-02-16 15:33   ` Hannes Reinecke
2016-02-16 16:56     ` John Garry
2016-02-18  7:40       ` Hannes Reinecke [this message]
2016-02-18 10:12         ` John Garry
2016-02-18 10:30           ` Hannes Reinecke
2016-02-18 10:57             ` John Garry
2016-02-19 10:46               ` John Garry
2016-02-19 14:31                 ` Hannes Reinecke
2016-02-22 10:02                   ` John Garry
2016-02-16 12:22 ` [PATCH 6/6] hisi_sas: update driver version to 1.3 John Garry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56C5756A.9060700@suse.de \
    --to=hare@suse.de \
    --cc=JBottomley@odin.com \
    --cc=john.garry2@mail.dcu.ie \
    --cc=john.garry@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=martin.petersen@oracle.com \
    --cc=xuwei5@hisilicon.com \
    --cc=zhangfei.gao@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).