linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-scsi@vger.kernel.org,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
	Sathya Prakash <sathya.prakash@broadcom.com>,
	Chaitra P B <chaitra.basappa@broadcom.com>,
	Suganath Prabu Subramani  <suganath-prabu.subramani@broadcom.com>,
	Kashyap Desai <kashyap.desai@broadcom.com>,
	Sumit Saxena <sumit.saxena@broadcom.com>,
	Shivasharan S <shivasharan.srikanteshwara@broadcom.com>,
	"Ewan D . Milne" <emilne@redhat.com>,
	Christoph Hellwig <hch@lst.de>, Hannes Reinecke <hare@suse.de>,
	Bart Van Assche <bart.vanassche@wdc.com>
Subject: Re: [PATCH 5/6] scsi: core: don't limit per-LUN queue depth for SSD when HBA needs
Date: Mon, 20 Jan 2020 23:52:06 -0500	[thread overview]
Message-ID: <yq1y2u1if7t.fsf@oracle.com> (raw)
In-Reply-To: <20200119071432.18558-6-ming.lei@redhat.com> (Ming Lei's message of "Sun, 19 Jan 2020 15:14:31 +0800")


Ming,

> NVMe doesn't have such per-request-queue(namespace) queue depth, so it
> is reasonable to ignore the limit for SCSI SSD too.

It is really not. A free host tag does not guarantee that the target
device can queue the command.

The assumption that SSDs are somehow special because they are "fast" is
not valid. Given the common hardware queue depth for a SAS device of
~128 it is often trivial to drive a device into a congestion
scenario. We see it all the time for non-rotational devices, SSDs and
arrays alike. The SSD heuristic is simply not going to fly.

Don't get me wrong, I am very sympathetic to obliterating device_busy in
the hot path. I just don't think it is as easy as just ignoring the
counter and hope for the best. Dynamic queue depth management is an
integral part of the SCSI protocol, not something we can just decide to
bypass because a device claims to be of a certain media type or speed.

I would prefer not to touch drivers that rely on cmd_per_lun / untagged
operation and focus exclusively on the ones that use .track_queue_depth.
For those we could consider an adaptive queue depth management scheme.
Something like not maintaining device_busy until we actually get a
QUEUE_FULL condition. And then rely on the existing queue depth ramp up
heuristics to determine when to disable the busy counter again. Maybe
with an additional watermark or time limit to avoid flip-flopping.

If that approach turns out to work, we should convert all remaining
non-legacy drivers to .track_queue_depth so we only have two driver
queuing flavors to worry about.

-- 
Martin K. Petersen	Oracle Linux Engineering

  parent reply	other threads:[~2020-01-21  4:52 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-19  7:14 [PATCH 0/6] scsi: support bypass device busy check for some high end HBA with SSD Ming Lei
2020-01-19  7:14 ` [PATCH 1/6] scsi: mpt3sas: don't use .device_busy in device reset routine Ming Lei
2020-01-19 20:28   ` Bart Van Assche
2020-01-19  7:14 ` [PATCH 2/6] scsi: remove .for_blk_mq Ming Lei
2020-01-19 20:29   ` Bart Van Assche
2020-01-20 10:17   ` John Garry
2020-01-20 22:12   ` Elliott, Robert (Servers)
2020-01-31  6:30   ` Christoph Hellwig
2020-02-05  2:13     ` Martin K. Petersen
2020-01-19  7:14 ` [PATCH 3/6] scsi: sd: register request queue after sd_revalidate_disk is done Ming Lei
2020-01-19 20:36   ` Bart Van Assche
2020-01-19  7:14 ` [PATCH 4/6] block: freeze queue for updating QUEUE_FLAG_NONROT Ming Lei
2020-01-19 20:40   ` Bart Van Assche
2020-01-19  7:14 ` [PATCH 5/6] scsi: core: don't limit per-LUN queue depth for SSD when HBA needs Ming Lei
2020-01-19 20:58   ` Bart Van Assche
2020-01-21  4:52   ` Martin K. Petersen [this message]
2020-01-23  2:54     ` Ming Lei
2020-01-24  1:21       ` Martin K. Petersen
2020-01-24  1:59         ` Ming Lei
2020-01-24 12:43           ` Sumit Saxena
2020-01-28  4:04             ` Martin K. Petersen
     [not found]     ` <ab676c4c-03fb-7eb9-6212-129eb83d0ee8@broadcom.com>
     [not found]       ` <yq1iml1ehtl.fsf@oracle.com>
2020-01-24 19:41         ` Sumanesh Samanta
2020-01-28  4:22           ` Martin K. Petersen
2020-01-31 11:39             ` Ming Lei
2020-01-19  7:14 ` [PATCH 6/6] scsi: megaraid: set flag of no_device_queue_for_ssd Ming Lei

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=yq1y2u1if7t.fsf@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@kernel.dk \
    --cc=bart.vanassche@wdc.com \
    --cc=chaitra.basappa@broadcom.com \
    --cc=emilne@redhat.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=kashyap.desai@broadcom.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=sathya.prakash@broadcom.com \
    --cc=shivasharan.srikanteshwara@broadcom.com \
    --cc=suganath-prabu.subramani@broadcom.com \
    --cc=sumit.saxena@broadcom.com \
    /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).