| Message ID | 20180227100750.32299-8-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
> + /* 256 tags should be high enough to saturate device */ > + int max_queues = DIV_ROUND_UP(h->scsi_host->can_queue, 256); > + > + /* per NUMA node hw queue */ > + h->scsi_host->nr_hw_queues = min_t(int, nr_node_ids, max_queues); I don't think this magic should be in a driver. The per-node hw_queue selection seems like something we'd better do in the core code. Also the whole idea to use nr_hw_queues for just partitioning tag space on hardware that doesn't really support multiple hardware queues seems more than odd.
On Thu, Mar 08, 2018 at 08:54:43AM +0100, Christoph Hellwig wrote: > > + /* 256 tags should be high enough to saturate device */ > > + int max_queues = DIV_ROUND_UP(h->scsi_host->can_queue, 256); > > + > > + /* per NUMA node hw queue */ > > + h->scsi_host->nr_hw_queues = min_t(int, nr_node_ids, max_queues); > > I don't think this magic should be in a driver. The per-node hw_queue > selection seems like something we'd better do in the core code. The thing is that driver code may need to know if multiple queues are used, then driver may partition its own resource into multi hw queues, and improve its .queuecommand and .complete_command. That seems what megaraid_sas should do in next time. > > Also the whole idea to use nr_hw_queues for just partitioning tag > space on hardware that doesn't really support multiple hardware queues > seems more than odd. The per-node hw queue is used together with BLK_MQ_F_HOST_TAGS, which is really for improving the single queue case(single tagset). If driver/device supports real multiple hw queues, they don't need this approach. Thanks, Ming
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 3a9eca163db8..0747751b7e1c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -978,6 +978,7 @@ static struct scsi_host_template hpsa_driver_template = { .shost_attrs = hpsa_shost_attrs, .max_sectors = 1024, .no_write_same = 1, + .host_tagset = 1, }; static inline u32 next_command(struct ctlr_info *h, u8 q) @@ -5761,6 +5762,11 @@ static int hpsa_scsi_host_alloc(struct ctlr_info *h) static int hpsa_scsi_add_host(struct ctlr_info *h) { int rv; + /* 256 tags should be high enough to saturate device */ + int max_queues = DIV_ROUND_UP(h->scsi_host->can_queue, 256); + + /* per NUMA node hw queue */ + h->scsi_host->nr_hw_queues = min_t(int, nr_node_ids, max_queues); rv = scsi_add_host(h->scsi_host, &h->pdev->dev); if (rv) {
It is observed that IOPS can be improved much by simply making hw queue per NUMA node on null_blk, so this patch applies the introduced .host_tagset for improving performance. In reality, .can_queue is quite big, and NUMA node number is often small, so each hw queue's depth should be high enough to saturate device. Cc: Arun Easi <arun.easi@cavium.com> 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: Peter Rivera <peter.rivera@broadcom.com> Cc: Laurence Oberman <loberman@redhat.com> Cc: Hannes Reinecke <hare@suse.de> Cc: Mike Snitzer <snitzer@redhat.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/scsi/hpsa.c | 6 ++++++ 1 file changed, 6 insertions(+)