* [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path
@ 2018-04-20 6:57 Ming Lei
2018-04-20 6:57 ` [PATCH 1/3] scsi: introduce scsi_host_busy() Ming Lei
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Ming Lei @ 2018-04-20 6:57 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-scsi, linux-block, Ming Lei, Omar Sandoval,
Martin K. Petersen, James Bottomley, Christoph Hellwig,
Don Brace, Kashyap Desai, Mike Snitzer, Hannes Reinecke,
Laurence Oberman
Hi,
This patches removes the expensive atomic opeation on host-wide counter
of .host_busy for scsi-mq, and it is observed that IOPS can be increased by
15% with this change in IO test over scsi_debug.
Ming Lei (3):
scsi: introduce scsi_host_busy()
scsi: read host_busy via scsi_host_busy()
scsi: avoid to hold host-wide counter of host_busy for scsi_mq
drivers/scsi/advansys.c | 8 ++++----
drivers/scsi/hosts.c | 32 +++++++++++++++++++++++++++++++
drivers/scsi/libsas/sas_scsi_host.c | 4 ++--
drivers/scsi/megaraid/megaraid_sas_base.c | 2 +-
drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++--
drivers/scsi/qlogicpti.c | 2 +-
drivers/scsi/scsi.c | 2 +-
drivers/scsi/scsi_error.c | 6 +++---
drivers/scsi/scsi_lib.c | 23 ++++++++++++++++------
drivers/scsi/scsi_sysfs.c | 2 +-
include/scsi/scsi_host.h | 1 +
11 files changed, 65 insertions(+), 21 deletions(-)
Cc: Omar Sandoval <osandov@fb.com>,
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
Cc: James Bottomley <james.bottomley@hansenpartnership.com>,
Cc: Christoph Hellwig <hch@lst.de>,
Cc: Don Brace <don.brace@microsemi.com>
Cc: Kashyap Desai <kashyap.desai@broadcom.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Laurence Oberman <loberman@redhat.com>
--
2.9.5
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/3] scsi: introduce scsi_host_busy() 2018-04-20 6:57 [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Ming Lei @ 2018-04-20 6:57 ` Ming Lei 2018-04-27 15:47 ` Bart Van Assche 2018-04-20 6:57 ` [PATCH 2/3] scsi: read host_busy via scsi_host_busy() Ming Lei ` (3 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Ming Lei @ 2018-04-20 6:57 UTC (permalink / raw) To: Jens Axboe Cc: linux-scsi, linux-block, Ming Lei, Omar Sandoval, Martin K. Petersen, James Bottomley, Christoph Hellwig, Don Brace, Kashyap Desai, Mike Snitzer, Hannes Reinecke, Laurence Oberman This patch introduces SCSI middle layer API of scsi_host_busy() for drivers to read the host-wide counter of scsi_host->host_busy. Cc: Omar Sandoval <osandov@fb.com>, Cc: "Martin K. Petersen" <martin.petersen@oracle.com>, Cc: James Bottomley <james.bottomley@hansenpartnership.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: Don Brace <don.brace@microsemi.com> Cc: Kashyap Desai <kashyap.desai@broadcom.com> Cc: Mike Snitzer <snitzer@redhat.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Laurence Oberman <loberman@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/hosts.c | 10 ++++++++++ include/scsi/scsi_host.h | 1 + 2 files changed, 11 insertions(+) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 7649d63a1b8d..69beb30205f1 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -565,6 +565,16 @@ struct Scsi_Host *scsi_host_get(struct Scsi_Host *shost) EXPORT_SYMBOL(scsi_host_get); /** + * scsi_host_busy - Return the host busy counter + * @shost: Pointer to Scsi_Host to inc. + **/ +int scsi_host_busy(struct Scsi_Host *shost) +{ + return atomic_read(&shost->host_busy); +} +EXPORT_SYMBOL(scsi_host_busy); + +/** * scsi_host_put - dec a Scsi_Host ref count * @shost: Pointer to Scsi_Host to dec. **/ diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 12f454cb6f61..44ab89268f30 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -758,6 +758,7 @@ extern void scsi_scan_host(struct Scsi_Host *); extern void scsi_rescan_device(struct device *); extern void scsi_remove_host(struct Scsi_Host *); extern struct Scsi_Host *scsi_host_get(struct Scsi_Host *); +extern int scsi_host_busy(struct Scsi_Host *shost); extern void scsi_host_put(struct Scsi_Host *t); extern struct Scsi_Host *scsi_host_lookup(unsigned short); extern const char *scsi_host_state_name(enum scsi_host_state); -- 2.9.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] scsi: introduce scsi_host_busy() 2018-04-20 6:57 ` [PATCH 1/3] scsi: introduce scsi_host_busy() Ming Lei @ 2018-04-27 15:47 ` Bart Van Assche 0 siblings, 0 replies; 17+ messages in thread From: Bart Van Assche @ 2018-04-27 15:47 UTC (permalink / raw) To: axboe, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai T24gRnJpLCAyMDE4LTA0LTIwIGF0IDE0OjU3ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gVGhp cyBwYXRjaCBpbnRyb2R1Y2VzIFNDU0kgbWlkZGxlIGxheWVyIEFQSSBvZiBzY3NpX2hvc3RfYnVz eSgpIGZvcg0KPiBkcml2ZXJzIHRvIHJlYWQgdGhlIGhvc3Qtd2lkZSBjb3VudGVyIG9mIHNjc2lf aG9zdC0+aG9zdF9idXN5Lg0KDQpUaGlzIHBhdGNoIGludHJvZHVjZXMgYSBmdW5jdGlvbiB0aGF0 IGhhcyBubyBjYWxsZXJzIHNvIEkgdGhpbmsgdGhpcyBwYXRjaA0Kc2hvdWxkIGJlIGNvbWJpbmVk IHdpdGggcGF0Y2ggMi8zIGZyb20gdGhpcyBzZXJpZXMgaW50byBhIHNpbmdsZSBwYXRjaC4NCg0K QmFydC4NCg0KDQo= ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] scsi: read host_busy via scsi_host_busy() 2018-04-20 6:57 [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Ming Lei 2018-04-20 6:57 ` [PATCH 1/3] scsi: introduce scsi_host_busy() Ming Lei @ 2018-04-20 6:57 ` Ming Lei 2018-04-27 15:51 ` Bart Van Assche 2018-04-20 6:57 ` [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq Ming Lei ` (2 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Ming Lei @ 2018-04-20 6:57 UTC (permalink / raw) To: Jens Axboe Cc: linux-scsi, linux-block, Ming Lei, Omar Sandoval, Martin K. Petersen, James Bottomley, Christoph Hellwig, Don Brace, Kashyap Desai, Mike Snitzer, Hannes Reinecke, Laurence Oberman No functional change, just replace the direct read of scsi_host->host_busy with scsi_host_busy(). Cc: Omar Sandoval <osandov@fb.com>, Cc: "Martin K. Petersen" <martin.petersen@oracle.com>, Cc: James Bottomley <james.bottomley@hansenpartnership.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: Don Brace <don.brace@microsemi.com> Cc: Kashyap Desai <kashyap.desai@broadcom.com> Cc: Mike Snitzer <snitzer@redhat.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Laurence Oberman <loberman@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/advansys.c | 8 ++++---- drivers/scsi/libsas/sas_scsi_host.c | 4 ++-- drivers/scsi/megaraid/megaraid_sas_base.c | 2 +- drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++-- drivers/scsi/qlogicpti.c | 2 +- drivers/scsi/scsi.c | 2 +- drivers/scsi/scsi_error.c | 6 +++--- drivers/scsi/scsi_sysfs.c | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c index 24e57e770432..38ee024cbfc7 100644 --- a/drivers/scsi/advansys.c +++ b/drivers/scsi/advansys.c @@ -2416,8 +2416,8 @@ static void asc_prt_scsi_host(struct Scsi_Host *s) struct asc_board *boardp = shost_priv(s); printk("Scsi_Host at addr 0x%p, device %s\n", s, dev_name(boardp->dev)); - printk(" host_busy %u, host_no %d,\n", - atomic_read(&s->host_busy), s->host_no); + printk(" host_busy %d, host_no %d,\n", + scsi_host_busy(s), s->host_no); printk(" base 0x%lx, io_port 0x%lx, irq %d,\n", (ulong)s->base, (ulong)s->io_port, boardp->irq); @@ -3182,8 +3182,8 @@ static void asc_prt_driver_conf(struct seq_file *m, struct Scsi_Host *shost) shost->host_no); seq_printf(m, - " host_busy %u, max_id %u, max_lun %llu, max_channel %u\n", - atomic_read(&shost->host_busy), shost->max_id, + " host_busy %d, max_id %u, max_lun %llu, max_channel %u\n", + scsi_host_busy(shost), shost->max_id, shost->max_lun, shost->max_channel); seq_printf(m, diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c index ceab5e5c41c2..33229348dcb6 100644 --- a/drivers/scsi/libsas/sas_scsi_host.c +++ b/drivers/scsi/libsas/sas_scsi_host.c @@ -759,7 +759,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost) spin_unlock_irq(shost->host_lock); SAS_DPRINTK("Enter %s busy: %d failed: %d\n", - __func__, atomic_read(&shost->host_busy), shost->host_failed); + __func__, scsi_host_busy(shost), shost->host_failed); /* * Deal with commands that still have SAS tasks (i.e. they didn't * complete via the normal sas_task completion mechanism), @@ -801,7 +801,7 @@ void sas_scsi_recover_host(struct Scsi_Host *shost) goto retry; SAS_DPRINTK("--- Exit %s: busy: %d failed: %d tries: %d\n", - __func__, atomic_read(&shost->host_busy), + __func__, scsi_host_busy(shost), shost->host_failed, tries); } diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index b89c6e6c0589..008a3bdfa948 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -2822,7 +2822,7 @@ static int megasas_reset_bus_host(struct scsi_cmnd *scmd) "SCSI command pointer: (%p)\t SCSI host state: %d\t" " SCSI host busy: %d\t FW outstanding: %d\n", scmd, scmd->device->host->shost_state, - atomic_read((atomic_t *)&scmd->device->host->host_busy), + scsi_host_busy(scmd->device->host), atomic_read(&instance->fw_outstanding)); /* diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 61f93a134956..b7325355df8e 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -3260,7 +3260,7 @@ _base_recovery_check(struct MPT3SAS_ADAPTER *ioc) * See _wait_for_commands_to_complete() call with regards to this code. */ if (ioc->shost_recovery && ioc->pending_io_count) { - ioc->pending_io_count = atomic_read(&ioc->shost->host_busy); + ioc->pending_io_count = scsi_host_busy(ioc->shost); if (ioc->pending_io_count == 0) wake_up(&ioc->reset_wq); } @@ -6658,7 +6658,7 @@ mpt3sas_wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc) return; /* pending command count */ - ioc->pending_io_count = atomic_read(&ioc->shost->host_busy); + ioc->pending_io_count = scsi_host_busy(ioc->shost); if (!ioc->pending_io_count) return; diff --git a/drivers/scsi/qlogicpti.c b/drivers/scsi/qlogicpti.c index cec9a14982e6..aaaaca685d65 100644 --- a/drivers/scsi/qlogicpti.c +++ b/drivers/scsi/qlogicpti.c @@ -959,7 +959,7 @@ static inline void update_can_queue(struct Scsi_Host *host, u_int in_ptr, u_int /* Temporary workaround until bug is found and fixed (one bug has been found already, but fixing it makes things even worse) -jj */ int num_free = QLOGICPTI_REQ_QUEUE_LEN - REQ_QUEUE_DEPTH(in_ptr, out_ptr) - 64; - host->can_queue = atomic_read(&host->host_busy) + num_free; + host->can_queue = scsi_host_busy(host) + num_free; host->sg_tablesize = QLOGICPTI_MAX_SG(num_free); } diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 4c60c260c5da..a15d078321a2 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -167,7 +167,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) if (level > 3) scmd_printk(KERN_INFO, cmd, "scsi host busy %d failed %d\n", - atomic_read(&cmd->device->host->host_busy), + scsi_host_busy(cmd->device->host), cmd->device->host->host_failed); } } diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 946039117bf4..bc06e465dc56 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -65,7 +65,7 @@ void scsi_eh_wakeup(struct Scsi_Host *shost) { lockdep_assert_held(shost->host_lock); - if (atomic_read(&shost->host_busy) == shost->host_failed) { + if (scsi_host_busy(shost) == shost->host_failed) { trace_scsi_eh_wakeup(shost); wake_up_process(shost->ehandler); SCSI_LOG_ERROR_RECOVERY(5, shost_printk(KERN_INFO, shost, @@ -2152,7 +2152,7 @@ int scsi_error_handler(void *data) break; if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) || - shost->host_failed != atomic_read(&shost->host_busy)) { + shost->host_failed != scsi_host_busy(shost)) { SCSI_LOG_ERROR_RECOVERY(1, shost_printk(KERN_INFO, shost, "scsi_eh_%d: sleeping\n", @@ -2167,7 +2167,7 @@ int scsi_error_handler(void *data) "scsi_eh_%d: waking up %d/%d/%d\n", shost->host_no, shost->host_eh_scheduled, shost->host_failed, - atomic_read(&shost->host_busy))); + scsi_host_busy(shost))); /* * We have a host that is failing for some reason. Figure out diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 1e36c9a9ad17..016a1c7930a8 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -382,7 +382,7 @@ static ssize_t show_host_busy(struct device *dev, struct device_attribute *attr, char *buf) { struct Scsi_Host *shost = class_to_shost(dev); - return snprintf(buf, 20, "%d\n", atomic_read(&shost->host_busy)); + return snprintf(buf, 20, "%d\n", scsi_host_busy(shost)); } static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL); -- 2.9.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] scsi: read host_busy via scsi_host_busy() 2018-04-20 6:57 ` [PATCH 2/3] scsi: read host_busy via scsi_host_busy() Ming Lei @ 2018-04-27 15:51 ` Bart Van Assche 2018-04-28 8:17 ` Ming Lei 0 siblings, 1 reply; 17+ messages in thread From: Bart Van Assche @ 2018-04-27 15:51 UTC (permalink / raw) To: axboe, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai T24gRnJpLCAyMDE4LTA0LTIwIGF0IDE0OjU3ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gIHNo b3dfaG9zdF9idXN5KHN0cnVjdCBkZXZpY2UgKmRldiwgc3RydWN0IGRldmljZV9hdHRyaWJ1dGUg KmF0dHIsIGNoYXIgKmJ1ZikNCj4gIHsNCj4gIAlzdHJ1Y3QgU2NzaV9Ib3N0ICpzaG9zdCA9IGNs YXNzX3RvX3Nob3N0KGRldik7DQo+IC0JcmV0dXJuIHNucHJpbnRmKGJ1ZiwgMjAsICIlZFxuIiwg YXRvbWljX3JlYWQoJnNob3N0LT5ob3N0X2J1c3kpKTsNCj4gKwlyZXR1cm4gc25wcmludGYoYnVm LCAyMCwgIiVkXG4iLCBzY3NpX2hvc3RfYnVzeShzaG9zdCkpOw0KPiAgfQ0KPiAgc3RhdGljIERF VklDRV9BVFRSKGhvc3RfYnVzeSwgU19JUlVHTywgc2hvd19ob3N0X2J1c3ksIE5VTEwpOw0KDQpU aGUgIiwgMjAiIHBhcnQgaXMgY2FyZ28tY3VsdCBwcm9ncmFtbWluZy4gU2luY2UgeW91IGhhdmUg dG8gdG91Y2ggdGhpcyBjb2RlLA0KcGxlYXNlIGVpdGhlciB1c2UgInNwcmludGYoYnVmLCAuLi4p IiBvciB1c2UgInNjbnByaW50ZihidWYsIFBBR0VfU0laRSwgLi4uKSIuDQoNClRoYW5rcywNCg0K QmFydC4NCg0KDQoNCg== ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] scsi: read host_busy via scsi_host_busy() 2018-04-27 15:51 ` Bart Van Assche @ 2018-04-28 8:17 ` Ming Lei 0 siblings, 0 replies; 17+ messages in thread From: Ming Lei @ 2018-04-28 8:17 UTC (permalink / raw) To: Bart Van Assche Cc: axboe, linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai On Fri, Apr 27, 2018 at 03:51:46PM +0000, Bart Van Assche wrote: > On Fri, 2018-04-20 at 14:57 +0800, Ming Lei wrote: > > show_host_busy(struct device *dev, struct device_attribute *attr, char *buf) > > { > > struct Scsi_Host *shost = class_to_shost(dev); > > - return snprintf(buf, 20, "%d\n", atomic_read(&shost->host_busy)); > > + return snprintf(buf, 20, "%d\n", scsi_host_busy(shost)); > > } > > static DEVICE_ATTR(host_busy, S_IRUGO, show_host_busy, NULL); > > The ", 20" part is cargo-cult programming. Since you have to touch this code, > please either use "sprintf(buf, ...)" or use "scnprintf(buf, PAGE_SIZE, ...)". This patch is only to replace atomic_read(&shost->host_busy) with scsi_host_busy(shost) which returns 'int' too, so nothing related with snprintf(buf, 20,..). No mention the string with 20 length is enough to hold integer, so it isn't needed too. So I don't see any reason to do that in this patch. Thanks, Ming ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq 2018-04-20 6:57 [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Ming Lei 2018-04-20 6:57 ` [PATCH 1/3] scsi: introduce scsi_host_busy() Ming Lei 2018-04-20 6:57 ` [PATCH 2/3] scsi: read host_busy via scsi_host_busy() Ming Lei @ 2018-04-20 6:57 ` Ming Lei 2018-04-27 16:16 ` Bart Van Assche 2018-04-27 15:31 ` [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Bart Van Assche 2018-06-22 15:29 ` Bart Van Assche 4 siblings, 1 reply; 17+ messages in thread From: Ming Lei @ 2018-04-20 6:57 UTC (permalink / raw) To: Jens Axboe Cc: linux-scsi, linux-block, Ming Lei, Omar Sandoval, Martin K. Petersen, James Bottomley, Christoph Hellwig, Don Brace, Kashyap Desai, Mike Snitzer, Hannes Reinecke, Laurence Oberman It isn't necessary to check the host depth in scsi_queue_rq() any more since it has been respected by blk-mq before calling scsi_queue_rq() via getting driver tag. Lots of LUNs may attach to same host, and per-host IOPS may reach millions level, so we should avoid to this expensive atomic operations on the hostwide counter in IO path. This patch implemens scsi_host_busy() via blk_mq_tagset_busy_iter() for reading the count of busy IOs for scsi_mq. It is observed that IOPS is increased by 15% in IO test on scsi_debug (32 LUNs, 32 submit queues, 1024 can_queue, libaio/dio) in one dual-socket system. Cc: Omar Sandoval <osandov@fb.com>, Cc: "Martin K. Petersen" <martin.petersen@oracle.com>, Cc: James Bottomley <james.bottomley@hansenpartnership.com>, Cc: Christoph Hellwig <hch@lst.de>, Cc: Don Brace <don.brace@microsemi.com> Cc: Kashyap Desai <kashyap.desai@broadcom.com> Cc: Mike Snitzer <snitzer@redhat.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Laurence Oberman <loberman@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/hosts.c | 24 +++++++++++++++++++++++- drivers/scsi/scsi_lib.c | 23 +++++++++++++++++------ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 69beb30205f1..ad56e2b10ac8 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -564,13 +564,35 @@ struct Scsi_Host *scsi_host_get(struct Scsi_Host *shost) } EXPORT_SYMBOL(scsi_host_get); +struct scsi_host_mq_in_flight { + int cnt; +}; + +static void scsi_host_check_in_flight(struct request *rq, void *data, + bool reserved) +{ + struct scsi_host_mq_in_flight *in_flight = data; + + if (blk_mq_request_started(rq)) + in_flight->cnt++; +} + /** * scsi_host_busy - Return the host busy counter * @shost: Pointer to Scsi_Host to inc. **/ int scsi_host_busy(struct Scsi_Host *shost) { - return atomic_read(&shost->host_busy); + struct scsi_host_mq_in_flight in_flight = { + .cnt = 0, + }; + + if (!shost->use_blk_mq) + return atomic_read(&shost->host_busy); + + blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight, + &in_flight); + return in_flight.cnt; } EXPORT_SYMBOL(scsi_host_busy); diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 0dfec0dedd5e..dc437c642934 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -345,7 +345,8 @@ static void scsi_dec_host_busy(struct Scsi_Host *shost) unsigned long flags; rcu_read_lock(); - atomic_dec(&shost->host_busy); + if (!shost->use_blk_mq) + atomic_dec(&shost->host_busy); if (unlikely(scsi_host_in_recovery(shost))) { spin_lock_irqsave(shost->host_lock, flags); if (shost->host_failed || shost->host_eh_scheduled) @@ -444,7 +445,12 @@ static inline bool scsi_target_is_busy(struct scsi_target *starget) static inline bool scsi_host_is_busy(struct Scsi_Host *shost) { - if (shost->can_queue > 0 && + /* + * blk-mq can handle host queue busy efficiently via host-wide driver + * tag allocation + */ + + if (!shost->use_blk_mq && shost->can_queue > 0 && atomic_read(&shost->host_busy) >= shost->can_queue) return true; if (atomic_read(&shost->host_blocked) > 0) @@ -1539,9 +1545,12 @@ static inline int scsi_host_queue_ready(struct request_queue *q, if (scsi_host_in_recovery(shost)) return 0; - busy = atomic_inc_return(&shost->host_busy) - 1; + if (!shost->use_blk_mq) + busy = atomic_inc_return(&shost->host_busy) - 1; + else + busy = 0; if (atomic_read(&shost->host_blocked) > 0) { - if (busy) + if (busy || scsi_host_busy(shost)) goto starved; /* @@ -1555,7 +1564,7 @@ static inline int scsi_host_queue_ready(struct request_queue *q, "unblocking host at zero depth\n")); } - if (shost->can_queue > 0 && busy >= shost->can_queue) + if (!shost->use_blk_mq && shost->can_queue > 0 && busy >= shost->can_queue) goto starved; if (shost->host_self_blocked) goto starved; @@ -1641,7 +1650,9 @@ static void scsi_kill_request(struct request *req, struct request_queue *q) * with the locks as normal issue path does. */ atomic_inc(&sdev->device_busy); - atomic_inc(&shost->host_busy); + + if (!shost->use_blk_mq) + atomic_inc(&shost->host_busy); if (starget->can_queue > 0) atomic_inc(&starget->target_busy); -- 2.9.5 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq 2018-04-20 6:57 ` [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq Ming Lei @ 2018-04-27 16:16 ` Bart Van Assche 2018-04-28 8:26 ` Ming Lei 0 siblings, 1 reply; 17+ messages in thread From: Bart Van Assche @ 2018-04-27 16:16 UTC (permalink / raw) To: axboe, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai T24gRnJpLCAyMDE4LTA0LTIwIGF0IDE0OjU3ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gK3N0 cnVjdCBzY3NpX2hvc3RfbXFfaW5fZmxpZ2h0IHsNCj4gKwlpbnQgY250Ow0KPiArfTsNCj4gKw0K PiArc3RhdGljIHZvaWQgc2NzaV9ob3N0X2NoZWNrX2luX2ZsaWdodChzdHJ1Y3QgcmVxdWVzdCAq cnEsIHZvaWQgKmRhdGEsDQo+ICsJCWJvb2wgcmVzZXJ2ZWQpDQo+ICt7DQo+ICsJc3RydWN0IHNj c2lfaG9zdF9tcV9pbl9mbGlnaHQgKmluX2ZsaWdodCA9IGRhdGE7DQo+ICsNCj4gKwlpZiAoYmxr X21xX3JlcXVlc3Rfc3RhcnRlZChycSkpDQo+ICsJCWluX2ZsaWdodC0+Y250Kys7DQo+ICt9DQo+ ICsNCj4gIC8qKg0KPiAgICogc2NzaV9ob3N0X2J1c3kgLSBSZXR1cm4gdGhlIGhvc3QgYnVzeSBj b3VudGVyDQo+ICAgKiBAc2hvc3Q6CVBvaW50ZXIgdG8gU2NzaV9Ib3N0IHRvIGluYy4NCj4gICAq Ki8NCj4gIGludCBzY3NpX2hvc3RfYnVzeShzdHJ1Y3QgU2NzaV9Ib3N0ICpzaG9zdCkNCj4gIHsN Cj4gLQlyZXR1cm4gYXRvbWljX3JlYWQoJnNob3N0LT5ob3N0X2J1c3kpOw0KPiArCXN0cnVjdCBz Y3NpX2hvc3RfbXFfaW5fZmxpZ2h0IGluX2ZsaWdodCA9IHsNCj4gKwkJLmNudCA9IDAsDQo+ICsJ fTsNCj4gKw0KPiArCWlmICghc2hvc3QtPnVzZV9ibGtfbXEpDQo+ICsJCXJldHVybiBhdG9taWNf cmVhZCgmc2hvc3QtPmhvc3RfYnVzeSk7DQo+ICsNCj4gKwlibGtfbXFfdGFnc2V0X2J1c3lfaXRl cigmc2hvc3QtPnRhZ19zZXQsIHNjc2lfaG9zdF9jaGVja19pbl9mbGlnaHQsDQo+ICsJCQkmaW5f ZmxpZ2h0KTsNCj4gKwlyZXR1cm4gaW5fZmxpZ2h0LmNudDsNCj4gIH0NCj4gIEVYUE9SVF9TWU1C T0woc2NzaV9ob3N0X2J1c3kpOw0KDQpUaGlzIHBhdGNoIGludHJvZHVjZXMgYSBzdWJ0bGUgYmVo YXZpb3IgY2hhbmdlIHRoYXQgaGFzIG5vdCBiZWVuIGV4cGxhaW5lZA0KaW4gdGhlIGNvbW1pdCBt ZXNzYWdlLiBJZiBhIFNDU0kgcmVxdWVzdCBnZXRzIHJlcXVldWVkIHRoYXQgcmVzdWx0cyBpbiBh DQpkZWNyZWFzZSBvZiB0aGUgLmhvc3RfYnVzeSBjb3VudGVyIGJ5IHNjc2lfZGV2aWNlX3VuYnVz eSgpIGJlZm9yZSB0aGUgcmVxdWVzdA0KaXMgcmVxdWV1ZWQgYW5kIGFuIGluY3JlYXNlIG9mIHRo ZSBob3N0X2J1c3kgY291bnRlciB3aGVuIHNjc2lfcXVldWVfcnEoKSBpcw0KY2FsbGVkIGFnYWlu LiBEdXJpbmcgdGhhdCB0aW1lIHN1Y2ggcmVxdWVzdHMgaGF2ZSB0aGUgc3RhdGUgTVFfUlFfQ09N UExFVEUgYW5kDQpoZW5jZSBibGtfbXFfcmVxdWVzdF9zdGFydGVkKCkgd2lsbCByZXR1cm4gdHJ1 ZSBhbmQgc2NzaV9ob3N0X2NoZWNrX2luX2ZsaWdodCgpDQp3aWxsIGluY2x1ZGUgdGhlc2UgcmVx dWVzdHMuIEluIG90aGVyIHdvcmRzLCB0aGlzIHBhdGNoIGludHJvZHVjZXMgYSBzdWJ0bGUNCmJl aGF2aW9yIGNoYW5nZSB0aGF0IGhhcyBub3QgYmVlbiBleHBsYWluZWQgaW4gdGhlIGNvbW1pdCBt ZXNzYWdlLiBIZW5jZSBJJ20NCmRvdWJ0IHRoYXQgdGhpcyBjaGFuZ2UgaXMgY29ycmVjdC4NCg0K QmFydC4NCg0KDQoNCg== ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq 2018-04-27 16:16 ` Bart Van Assche @ 2018-04-28 8:26 ` Ming Lei 0 siblings, 0 replies; 17+ messages in thread From: Ming Lei @ 2018-04-28 8:26 UTC (permalink / raw) To: Bart Van Assche Cc: axboe, linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai On Fri, Apr 27, 2018 at 04:16:48PM +0000, Bart Van Assche wrote: > On Fri, 2018-04-20 at 14:57 +0800, Ming Lei wrote: > > +struct scsi_host_mq_in_flight { > > + int cnt; > > +}; > > + > > +static void scsi_host_check_in_flight(struct request *rq, void *data, > > + bool reserved) > > +{ > > + struct scsi_host_mq_in_flight *in_flight = data; > > + > > + if (blk_mq_request_started(rq)) > > + in_flight->cnt++; > > +} > > + > > /** > > * scsi_host_busy - Return the host busy counter > > * @shost: Pointer to Scsi_Host to inc. > > **/ > > int scsi_host_busy(struct Scsi_Host *shost) > > { > > - return atomic_read(&shost->host_busy); > > + struct scsi_host_mq_in_flight in_flight = { > > + .cnt = 0, > > + }; > > + > > + if (!shost->use_blk_mq) > > + return atomic_read(&shost->host_busy); > > + > > + blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight, > > + &in_flight); > > + return in_flight.cnt; > > } > > EXPORT_SYMBOL(scsi_host_busy); > > This patch introduces a subtle behavior change that has not been explained > in the commit message. If a SCSI request gets requeued that results in a > decrease of the .host_busy counter by scsi_device_unbusy() before the request > is requeued and an increase of the host_busy counter when scsi_queue_rq() is > called again. During that time such requests have the state MQ_RQ_COMPLETE and > hence blk_mq_request_started() will return true and scsi_host_check_in_flight() No, __blk_mq_requeue_request() will change the rq state into MQ_RQ_IDLE, so such issue you worried about, please look at scsi_mq_requeue_cmd(), which calls blk_mq_requeue_request(), which puts driver tag and updates rq's state to IDLE. > will include these requests. In other words, this patch introduces a subtle > behavior change that has not been explained in the commit message. Hence I'm > doubt that this change is correct. As I explained above, no such issue. Thanks, Ming ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-20 6:57 [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Ming Lei ` (2 preceding siblings ...) 2018-04-20 6:57 ` [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq Ming Lei @ 2018-04-27 15:31 ` Bart Van Assche 2018-04-27 15:39 ` Jens Axboe 2018-06-22 15:29 ` Bart Van Assche 4 siblings, 1 reply; 17+ messages in thread From: Bart Van Assche @ 2018-04-27 15:31 UTC (permalink / raw) To: axboe, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai T24gRnJpLCAyMDE4LTA0LTIwIGF0IDE0OjU3ICswODAwLCBNaW5nIExlaSB3cm90ZToNCj4gVGhp cyBwYXRjaGVzIHJlbW92ZXMgdGhlIGV4cGVuc2l2ZSBhdG9taWMgb3BlYXRpb24gb24gaG9zdC13 aWRlIGNvdW50ZXINCj4gb2YgLmhvc3RfYnVzeSBmb3Igc2NzaS1tcSwgYW5kIGl0IGlzIG9ic2Vy dmVkIHRoYXQgSU9QUyBjYW4gYmUgaW5jcmVhc2VkIGJ5DQo+IDE1JSB3aXRoIHRoaXMgY2hhbmdl IGluIElPIHRlc3Qgb3ZlciBzY3NpX2RlYnVnLg0KPiANCj4gDQo+IE1pbmcgTGVpICgzKToNCj4g ICBzY3NpOiBpbnRyb2R1Y2Ugc2NzaV9ob3N0X2J1c3koKQ0KPiAgIHNjc2k6IHJlYWQgaG9zdF9i dXN5IHZpYSBzY3NpX2hvc3RfYnVzeSgpDQo+ICAgc2NzaTogYXZvaWQgdG8gaG9sZCBob3N0LXdp ZGUgY291bnRlciBvZiBob3N0X2J1c3kgZm9yIHNjc2lfbXENCj4gDQo+ICBkcml2ZXJzL3Njc2kv YWR2YW5zeXMuYyAgICAgICAgICAgICAgICAgICB8ICA4ICsrKystLS0tDQo+ICBkcml2ZXJzL3Nj c2kvaG9zdHMuYyAgICAgICAgICAgICAgICAgICAgICB8IDMyICsrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysNCj4gIGRyaXZlcnMvc2NzaS9saWJzYXMvc2FzX3Njc2lfaG9zdC5jICAgICAg IHwgIDQgKystLQ0KPiAgZHJpdmVycy9zY3NpL21lZ2FyYWlkL21lZ2FyYWlkX3Nhc19iYXNlLmMg fCAgMiArLQ0KPiAgZHJpdmVycy9zY3NpL21wdDNzYXMvbXB0M3Nhc19iYXNlLmMgICAgICAgfCAg NCArKy0tDQo+ICBkcml2ZXJzL3Njc2kvcWxvZ2ljcHRpLmMgICAgICAgICAgICAgICAgICB8ICAy ICstDQo+ICBkcml2ZXJzL3Njc2kvc2NzaS5jICAgICAgICAgICAgICAgICAgICAgICB8ICAyICst DQo+ICBkcml2ZXJzL3Njc2kvc2NzaV9lcnJvci5jICAgICAgICAgICAgICAgICB8ICA2ICsrKy0t LQ0KPiAgZHJpdmVycy9zY3NpL3Njc2lfbGliLmMgICAgICAgICAgICAgICAgICAgfCAyMyArKysr KysrKysrKysrKysrLS0tLS0tDQo+ICBkcml2ZXJzL3Njc2kvc2NzaV9zeXNmcy5jICAgICAgICAg ICAgICAgICB8ICAyICstDQo+ICBpbmNsdWRlL3Njc2kvc2NzaV9ob3N0LmggICAgICAgICAgICAg ICAgICB8ICAxICsNCj4gIDExIGZpbGVzIGNoYW5nZWQsIDY1IGluc2VydGlvbnMoKyksIDIxIGRl bGV0aW9ucygtKVwNCg0KSGVsbG8gTWluZywNCg0KRnJvbSB0aGUgTUFJTlRBSU5FUlMgZmlsZToN Cg0KU0NTSSBTVUJTWVNURU0NCk06ICAgICAgIkphbWVzIEUuSi4gQm90dG9tbGV5IiA8amVqYkBs aW51eC52bmV0LmlibS5jb20+DQpUOiAgICAgIGdpdCBnaXQ6Ly9naXQua2VybmVsLm9yZy9wdWIv c2NtL2xpbnV4L2tlcm5lbC9naXQvamVqYi9zY3NpLmdpdA0KTTogICAgICAiTWFydGluIEsuIFBl dGVyc2VuIiA8bWFydGluLnBldGVyc2VuQG9yYWNsZS5jb20+DQpUOiAgICAgIGdpdCBnaXQ6Ly9n aXQua2VybmVsLm9yZy9wdWIvc2NtL2xpbnV4L2tlcm5lbC9naXQvbWtwL3Njc2kuZ2l0DQpMOiAg ICAgIGxpbnV4LXNjc2lAdmdlci5rZXJuZWwub3JnDQpTOiAgICAgIE1haW50YWluZWQNCkY6ICAg ICAgRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3Njc2kvDQpGOiAgICAgIGRyaXZl cnMvc2NzaS8NCkY6ICAgICAgaW5jbHVkZS9zY3NpLw0KDQpIZW5jZSBteSBzdXJwcmlzZSB3aGVu IEkgc2F3IHRoYXQgeW91IHNlbnQgdGhpcyBwYXRjaCBzZXJpZXMgdG8gSmVucyBpbnN0ZWFkDQpv ZiBKYW1lcyBhbmQgTWFydGluPw0KDQpCYXJ0Lg== ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-27 15:31 ` [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Bart Van Assche @ 2018-04-27 15:39 ` Jens Axboe 2018-04-27 15:48 ` Bart Van Assche 2018-04-28 8:47 ` Ming Lei 0 siblings, 2 replies; 17+ messages in thread From: Jens Axboe @ 2018-04-27 15:39 UTC (permalink / raw) To: Bart Van Assche, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai On 4/27/18 9:31 AM, Bart Van Assche wrote: > On Fri, 2018-04-20 at 14:57 +0800, Ming Lei wrote: >> This patches removes the expensive atomic opeation on host-wide counter >> of .host_busy for scsi-mq, and it is observed that IOPS can be increased by >> 15% with this change in IO test over scsi_debug. >> >> >> Ming Lei (3): >> scsi: introduce scsi_host_busy() >> scsi: read host_busy via scsi_host_busy() >> scsi: avoid to hold host-wide counter of host_busy for scsi_mq >> >> drivers/scsi/advansys.c | 8 ++++---- >> drivers/scsi/hosts.c | 32 +++++++++++++++++++++++++++++++ >> drivers/scsi/libsas/sas_scsi_host.c | 4 ++-- >> drivers/scsi/megaraid/megaraid_sas_base.c | 2 +- >> drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++-- >> drivers/scsi/qlogicpti.c | 2 +- >> drivers/scsi/scsi.c | 2 +- >> drivers/scsi/scsi_error.c | 6 +++--- >> drivers/scsi/scsi_lib.c | 23 ++++++++++++++++------ >> drivers/scsi/scsi_sysfs.c | 2 +- >> include/scsi/scsi_host.h | 1 + >> 11 files changed, 65 insertions(+), 21 deletions(-)\ > > Hello Ming, > > From the MAINTAINERS file: > > SCSI SUBSYSTEM > M: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com> > T: git git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git > M: "Martin K. Petersen" <martin.petersen@oracle.com> > T: git git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git > L: linux-scsi@vger.kernel.org > S: Maintained > F: Documentation/devicetree/bindings/scsi/ > F: drivers/scsi/ > F: include/scsi/ > > Hence my surprise when I saw that you sent this patch series to Jens instead > of James and Martin? Martin and James are both on the CC as well. For what it's worth, the patch seems like a good approach to me. To handle the case that Hannes was concerned about (older drivers doing internal command issue), I would suggest that those drivers get instrumented to include a inc/dec of the host busy count for internal commands that bypass the normal tagging. That means the mq case needs to be blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight, &in_flight); return in_flight.cnt + atomic_read(&shost->host_busy); The atomic read is basically free, once we get rid of the dirty of that variable on each IO. -- Jens Axboe ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-27 15:39 ` Jens Axboe @ 2018-04-27 15:48 ` Bart Van Assche 2018-04-27 15:55 ` Laurence Oberman 2018-04-27 16:19 ` Jens Axboe 2018-04-28 8:47 ` Ming Lei 1 sibling, 2 replies; 17+ messages in thread From: Bart Van Assche @ 2018-04-27 15:48 UTC (permalink / raw) To: axboe, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai T24gRnJpLCAyMDE4LTA0LTI3IGF0IDA5OjM5IC0wNjAwLCBKZW5zIEF4Ym9lIHdyb3RlOg0KPiBi bGtfbXFfdGFnc2V0X2J1c3lfaXRlcigmc2hvc3QtPnRhZ19zZXQsIHNjc2lfaG9zdF9jaGVja19p bl9mbGlnaHQsDQo+IAkJCSZpbl9mbGlnaHQpOw0KPiByZXR1cm4gaW5fZmxpZ2h0LmNudCArIGF0 b21pY19yZWFkKCZzaG9zdC0+aG9zdF9idXN5KTsNCj4gDQo+IFRoZSBhdG9taWMgcmVhZCBpcyBi YXNpY2FsbHkgZnJlZSwgb25jZSB3ZSBnZXQgcmlkIG9mIHRoZSBkaXJ0eSBvZiB0aGF0DQo+IHZh cmlhYmxlIG9uIGVhY2ggSU8uDQoNCkhlbGxvIEplbnMsDQoNCldoYXQgbWFrZXMgeW91IHRoaW5r IHRoYXQgIiArIGF0b21pY19yZWFkKCZzaG9zdC0+aG9zdF9idXN5KSIgaXMgbmVjZXNzYXJ5Pw0K SSBhbSBub3QgYXdhcmUgb2YgYW55IGNvZGUgb3V0c2lkZSB0aGUgU0NTSSBjb3JlIHRoYXQgbW9k aWZpZXMgdGhlIGhvc3RfYnVzeQ0KbWVtYmVyLg0KDQpUaGFua3MsDQoNCkJhcnQuDQoNCg0KDQo= ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-27 15:48 ` Bart Van Assche @ 2018-04-27 15:55 ` Laurence Oberman 2018-04-27 16:19 ` Jens Axboe 1 sibling, 0 replies; 17+ messages in thread From: Laurence Oberman @ 2018-04-27 15:55 UTC (permalink / raw) To: Bart Van Assche, axboe, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, kashyap.desai On Fri, 2018-04-27 at 15:48 +0000, Bart Van Assche wrote: > On Fri, 2018-04-27 at 09:39 -0600, Jens Axboe wrote: > > blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight, > > &in_flight); > > return in_flight.cnt + atomic_read(&shost->host_busy); > > > > The atomic read is basically free, once we get rid of the dirty of > > that > > variable on each IO. > > Hello Jens, > > What makes you think that " + atomic_read(&shost->host_busy)" is > necessary? > I am not aware of any code outside the SCSI core that modifies the > host_busy > member. > > Thanks, > > Bart. > > > As part of testing latest upstream in MQ and non-MQ I intend to test this patch series fully on actual hardware F/C 8G to memory backed array LUNS and of course SRP/RDMA I have started working on this and will report back. Thanks Laurence ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-27 15:48 ` Bart Van Assche 2018-04-27 15:55 ` Laurence Oberman @ 2018-04-27 16:19 ` Jens Axboe 1 sibling, 0 replies; 17+ messages in thread From: Jens Axboe @ 2018-04-27 16:19 UTC (permalink / raw) To: Bart Van Assche, ming.lei Cc: linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai On 4/27/18 9:48 AM, Bart Van Assche wrote: > On Fri, 2018-04-27 at 09:39 -0600, Jens Axboe wrote: >> blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight, >> &in_flight); >> return in_flight.cnt + atomic_read(&shost->host_busy); >> >> The atomic read is basically free, once we get rid of the dirty of that >> variable on each IO. > > Hello Jens, > > What makes you think that " + atomic_read(&shost->host_busy)" is necessary? > I am not aware of any code outside the SCSI core that modifies the host_busy > member. It's a (scalable) hack to count those as well. Going forward they should be converted to just using reserved tags, of course. -- Jens Axboe ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-27 15:39 ` Jens Axboe 2018-04-27 15:48 ` Bart Van Assche @ 2018-04-28 8:47 ` Ming Lei 1 sibling, 0 replies; 17+ messages in thread From: Ming Lei @ 2018-04-28 8:47 UTC (permalink / raw) To: Jens Axboe Cc: Bart Van Assche, linux-block, snitzer, hch, martin.petersen, hare, linux-scsi, don.brace, james.bottomley, osandov, loberman, kashyap.desai On Fri, Apr 27, 2018 at 09:39:47AM -0600, Jens Axboe wrote: > On 4/27/18 9:31 AM, Bart Van Assche wrote: > > On Fri, 2018-04-20 at 14:57 +0800, Ming Lei wrote: > >> This patches removes the expensive atomic opeation on host-wide counter > >> of .host_busy for scsi-mq, and it is observed that IOPS can be increased by > >> 15% with this change in IO test over scsi_debug. > >> > >> > >> Ming Lei (3): > >> scsi: introduce scsi_host_busy() > >> scsi: read host_busy via scsi_host_busy() > >> scsi: avoid to hold host-wide counter of host_busy for scsi_mq > >> > >> drivers/scsi/advansys.c | 8 ++++---- > >> drivers/scsi/hosts.c | 32 +++++++++++++++++++++++++++++++ > >> drivers/scsi/libsas/sas_scsi_host.c | 4 ++-- > >> drivers/scsi/megaraid/megaraid_sas_base.c | 2 +- > >> drivers/scsi/mpt3sas/mpt3sas_base.c | 4 ++-- > >> drivers/scsi/qlogicpti.c | 2 +- > >> drivers/scsi/scsi.c | 2 +- > >> drivers/scsi/scsi_error.c | 6 +++--- > >> drivers/scsi/scsi_lib.c | 23 ++++++++++++++++------ > >> drivers/scsi/scsi_sysfs.c | 2 +- > >> include/scsi/scsi_host.h | 1 + > >> 11 files changed, 65 insertions(+), 21 deletions(-)\ > > > > Hello Ming, > > > > From the MAINTAINERS file: > > > > SCSI SUBSYSTEM > > M: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com> > > T: git git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git > > M: "Martin K. Petersen" <martin.petersen@oracle.com> > > T: git git://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git > > L: linux-scsi@vger.kernel.org > > S: Maintained > > F: Documentation/devicetree/bindings/scsi/ > > F: drivers/scsi/ > > F: include/scsi/ > > > > Hence my surprise when I saw that you sent this patch series to Jens instead > > of James and Martin? > > Martin and James are both on the CC as well. For what it's worth, the patch > seems like a good approach to me. To handle the case that Hannes was concerned > about (older drivers doing internal command issue), I would suggest that those > drivers get instrumented to include a inc/dec of the host busy count for > internal commands that bypass the normal tagging. That means the mq case needs > to be > > blk_mq_tagset_busy_iter(&shost->tag_set, scsi_host_check_in_flight, > &in_flight); > return in_flight.cnt + atomic_read(&shost->host_busy); > > The atomic read is basically free, once we get rid of the dirty of that > variable on each IO. Actually the internal command isn't submitted via normal IO path, then it won't be completed via the normal completion path(soft_irq_done, or .timeout), so handling internal command doesn't touch the scsi generic counter of .host_busy. I have talked with Hannes a bit about this at LSFMM, looks he agreed too. Thanks, Ming ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-04-20 6:57 [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Ming Lei ` (3 preceding siblings ...) 2018-04-27 15:31 ` [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Bart Van Assche @ 2018-06-22 15:29 ` Bart Van Assche 2018-06-22 21:43 ` Ming Lei 4 siblings, 1 reply; 17+ messages in thread From: Bart Van Assche @ 2018-06-22 15:29 UTC (permalink / raw) To: Ming Lei Cc: Jens Axboe, linux-scsi, linux-block, Omar Sandoval, Martin K. Petersen, James Bottomley, Christoph Hellwig, Don Brace, Kashyap Desai, Mike Snitzer, Hannes Reinecke, Laurence Oberman On 04/20/18 00:00, Ming Lei wrote: > This patches removes the expensive atomic opeation on host-wide counter > of .host_busy for scsi-mq, and it is observed that IOPS can be increased by > 15% with this change in IO test over scsi_debug. Hello Ming, Do you want to resend these patches to Martin or do you perhaps that I do that myself? Thanks, Bart. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path 2018-06-22 15:29 ` Bart Van Assche @ 2018-06-22 21:43 ` Ming Lei 0 siblings, 0 replies; 17+ messages in thread From: Ming Lei @ 2018-06-22 21:43 UTC (permalink / raw) To: Bart Van Assche Cc: Jens Axboe, linux-scsi, linux-block, Omar Sandoval, Martin K. Petersen, James Bottomley, Christoph Hellwig, Don Brace, Kashyap Desai, Mike Snitzer, Hannes Reinecke, Laurence Oberman On Fri, Jun 22, 2018 at 08:29:20AM -0700, Bart Van Assche wrote: > On 04/20/18 00:00, Ming Lei wrote: > > This patches removes the expensive atomic opeation on host-wide counter > > of .host_busy for scsi-mq, and it is observed that IOPS can be increased by > > 15% with this change in IO test over scsi_debug. > > Hello Ming, > > Do you want to resend these patches to Martin or do you perhaps that I do > that myself? Hi Bart, I will resend it later. thanks, Ming ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-06-22 21:43 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-20 6:57 [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Ming Lei 2018-04-20 6:57 ` [PATCH 1/3] scsi: introduce scsi_host_busy() Ming Lei 2018-04-27 15:47 ` Bart Van Assche 2018-04-20 6:57 ` [PATCH 2/3] scsi: read host_busy via scsi_host_busy() Ming Lei 2018-04-27 15:51 ` Bart Van Assche 2018-04-28 8:17 ` Ming Lei 2018-04-20 6:57 ` [PATCH 3/3] scsi: avoid to hold host-wide counter of host_busy for scsi_mq Ming Lei 2018-04-27 16:16 ` Bart Van Assche 2018-04-28 8:26 ` Ming Lei 2018-04-27 15:31 ` [PATCH 0/3] scsi: scsi-mq: don't hold host_busy in IO path Bart Van Assche 2018-04-27 15:39 ` Jens Axboe 2018-04-27 15:48 ` Bart Van Assche 2018-04-27 15:55 ` Laurence Oberman 2018-04-27 16:19 ` Jens Axboe 2018-04-28 8:47 ` Ming Lei 2018-06-22 15:29 ` Bart Van Assche 2018-06-22 21:43 ` Ming Lei
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.