[patch,v3,repost 00/10] make I/O path allocations more numa-friendly

[patch,v3,repost 00/10] make I/O path allocations more numa-friendly

[Jeff Moyer]

Hi,

This patch set makes memory allocations for data structures used in
the I/O path more numa friendly by allocating them from the same numa
node as the storage device.  I've only converted a handful of drivers
at this point.  My testing is limited by the hardware I have on hand.
Using these patches, I was able to max out the bandwidth of the storage
controller when issuing I/O from any node on my 4 node system.  Without
the patch, I/O from nodes remote to the storage device would suffer a
penalty ranging from 6-12%.  Given my relatively low-end setup[1], I
wouldn't be surprised if others could show a more significant performance
advantage.

This is a repost of the last posting.  The only changes are additional
reviewed-by/acked-by tags.  I think this version is ready for inclusion.
James, would you mind taking a look?

Cheers,
Jeff

[1] LSI Megaraid SAS controller with 1GB battery-backed cache,
fronting a RAID 6 10+2.  The workload I used was tuned to not
have to hit disk.  Fio file attached.

--
changes from v2->v3:
- Made the numa_node Scsi_Host structure member dependent on CONFIG_NUMA
- Got rid of a GFP_ZERO I added accidentally
changes from v1->v2:
- got rid of the vfs patch, as Al pointed out some fundamental
  problems with it
- credited Bart van Assche properly


Jeff Moyer (10):
  scsi: add scsi_host_alloc_node
  scsi: make __scsi_alloc_queue numa-aware
  scsi: make scsi_alloc_sdev numa-aware
  scsi: allocate scsi_cmnd-s from the device's local numa node
  sd: use alloc_disk_node
  ata: use scsi_host_alloc_node
  megaraid_sas: use scsi_host_alloc_node
  mpt2sas: use scsi_host_alloc_node
  lpfc: use scsi_host_alloc_node
  cciss: use blk_init_queue_node

 drivers/ata/libata-scsi.c                 |    3 ++-
 drivers/block/cciss.c                     |    3 ++-
 drivers/scsi/hosts.c                      |   13 +++++++++++--
 drivers/scsi/lpfc/lpfc_init.c             |   10 ++++++----
 drivers/scsi/megaraid/megaraid_sas_base.c |    5 +++--
 drivers/scsi/mpt2sas/mpt2sas_scsih.c      |    4 ++--
 drivers/scsi/scsi.c                       |   16 ++++++++++------
 drivers/scsi/scsi_lib.c                   |    3 ++-
 drivers/scsi/scsi_scan.c                  |    4 ++--
 drivers/scsi/sd.c                         |    2 +-
 include/scsi/scsi_host.h                  |   28 ++++++++++++++++++++++++++++
 11 files changed, 69 insertions(+), 22 deletions(-)

[patch,v3,repost 01/10] scsi: add scsi_host_alloc_node

[Jeff Moyer]

Allow an LLD to specify on which numa node to allocate scsi data
structures.  Thanks to Bart Van Assche for the suggestion.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/hosts.c     |   13 +++++++++++--
 include/scsi/scsi_host.h |   28 ++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 593085a..06ce602 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -336,16 +336,25 @@ static struct device_type scsi_host_type = {
  **/
 struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 {
+	return scsi_host_alloc_node(sht, privsize, NUMA_NO_NODE);
+}
+EXPORT_SYMBOL(scsi_host_alloc);
+
+struct Scsi_Host *scsi_host_alloc_node(struct scsi_host_template *sht,
+				       int privsize, int node)
+{
 	struct Scsi_Host *shost;
 	gfp_t gfp_mask = GFP_KERNEL;
 
 	if (sht->unchecked_isa_dma && privsize)
 		gfp_mask |= __GFP_DMA;
 
-	shost = kzalloc(sizeof(struct Scsi_Host) + privsize, gfp_mask);
+	shost = kzalloc_node(sizeof(struct Scsi_Host) + privsize,
+			     gfp_mask, node);
 	if (!shost)
 		return NULL;
 
+	scsi_host_set_numa_node(shost, node);
 	shost->host_lock = &shost->default_lock;
 	spin_lock_init(shost->host_lock);
 	shost->shost_state = SHOST_CREATED;
@@ -443,7 +452,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	kfree(shost);
 	return NULL;
 }
-EXPORT_SYMBOL(scsi_host_alloc);
+EXPORT_SYMBOL(scsi_host_alloc_node);
 
 struct Scsi_Host *scsi_register(struct scsi_host_template *sht, int privsize)
 {
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 4908480..438856d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -732,6 +732,14 @@ struct Scsi_Host {
 	 */
 	struct device *dma_dev;
 
+#ifdef CONFIG_NUMA
+	/*
+	 * Numa node this device is closest to, used for allocating
+	 * data structures locally.
+	 */
+	int numa_node;
+#endif
+
 	/*
 	 * We should ensure that this is aligned, both for better performance
 	 * and also because some compilers (m68k) don't automatically force
@@ -776,6 +784,8 @@ extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
 extern void scsi_flush_work(struct Scsi_Host *);
 
 extern struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *, int);
+extern struct Scsi_Host *scsi_host_alloc_node(struct scsi_host_template *,
+					      int, int);
 extern int __must_check scsi_add_host_with_dma(struct Scsi_Host *,
 					       struct device *,
 					       struct device *);
@@ -919,6 +929,24 @@ static inline unsigned char scsi_host_get_guard(struct Scsi_Host *shost)
 	return shost->prot_guard_type;
 }
 
+#ifdef CONFIG_NUMA
+static inline int scsi_host_get_numa_node(struct Scsi_Host *shost)
+{
+	return shost->numa_node;
+}
+
+static inline void scsi_host_set_numa_node(struct Scsi_Host *shost, int node)
+{
+	shost->numa_node = node;
+}
+#else /* CONFIG_NUMA */
+static inline int scsi_host_get_numa_node(struct Scsi_Host *shost)
+{
+	return NUMA_NO_NODE;
+}
+static inline void scsi_host_set_numa_node(struct Scsi_Host *shost, int node) {}
+#endif
+
 /* legacy interfaces */
 extern struct Scsi_Host *scsi_register(struct scsi_host_template *, int);
 extern void scsi_unregister(struct Scsi_Host *);
-- 
1.7.1

[patch,v3,repost 02/10] scsi: make __scsi_alloc_queue numa-aware

[Jeff Moyer]

Pass the numa node id set in the Scsi_Host on to blk_init_queue_node
in order to keep all allocations local to the numa node the device is
closest to.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/scsi_lib.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index da36a3a..ebad5e8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1664,7 +1664,8 @@ struct request_queue *__scsi_alloc_queue(struct Scsi_Host *shost,
 	struct request_queue *q;
 	struct device *dev = shost->dma_dev;
 
-	q = blk_init_queue(request_fn, NULL);
+	q = blk_init_queue_node(request_fn, NULL,
+				scsi_host_get_numa_node(shost));
 	if (!q)
 		return NULL;
 
-- 
1.7.1

[patch,v3,repost 03/10] scsi: make scsi_alloc_sdev numa-aware

[Jeff Moyer]

Use the numa node id set in the Scsi_Host to allocate the sdev structure
on the device-local numa node.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/scsi_scan.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 3e58b22..d91749d 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -232,8 +232,8 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	extern void scsi_evt_thread(struct work_struct *work);
 	extern void scsi_requeue_run_queue(struct work_struct *work);
 
-	sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
-		       GFP_ATOMIC);
+	sdev = kzalloc_node(sizeof(*sdev) + shost->transportt->device_size,
+			    GFP_ATOMIC, scsi_host_get_numa_node(shost));
 	if (!sdev)
 		goto out;
 
-- 
1.7.1

[patch,v3,repost 04/10] scsi: allocate scsi_cmnd-s from the device's local numa node

[Jeff Moyer]

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/scsi.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 2936b44..1750702 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -173,16 +173,19 @@ static DEFINE_MUTEX(host_cmd_pool_mutex);
  * NULL on failure
  */
 static struct scsi_cmnd *
-scsi_pool_alloc_command(struct scsi_host_cmd_pool *pool, gfp_t gfp_mask)
+scsi_pool_alloc_command(struct scsi_host_cmd_pool *pool, gfp_t gfp_mask,
+			int node)
 {
 	struct scsi_cmnd *cmd;
 
-	cmd = kmem_cache_zalloc(pool->cmd_slab, gfp_mask | pool->gfp_mask);
+	cmd = kmem_cache_alloc_node(pool->cmd_slab,
+				    gfp_mask | pool->gfp_mask | __GFP_ZERO,
+				    node);
 	if (!cmd)
 		return NULL;
 
-	cmd->sense_buffer = kmem_cache_alloc(pool->sense_slab,
-					     gfp_mask | pool->gfp_mask);
+	cmd->sense_buffer = kmem_cache_alloc_node(pool->sense_slab,
+					gfp_mask | pool->gfp_mask, node);
 	if (!cmd->sense_buffer) {
 		kmem_cache_free(pool->cmd_slab, cmd);
 		return NULL;
@@ -223,7 +226,8 @@ scsi_host_alloc_command(struct Scsi_Host *shost, gfp_t gfp_mask)
 {
 	struct scsi_cmnd *cmd;
 
-	cmd = scsi_pool_alloc_command(shost->cmd_pool, gfp_mask);
+	cmd = scsi_pool_alloc_command(shost->cmd_pool, gfp_mask,
+				      scsi_host_get_numa_node(shost));
 	if (!cmd)
 		return NULL;
 
@@ -435,7 +439,7 @@ struct scsi_cmnd *scsi_allocate_command(gfp_t gfp_mask)
 	if (!pool)
 		return NULL;
 
-	return scsi_pool_alloc_command(pool, gfp_mask);
+	return scsi_pool_alloc_command(pool, gfp_mask, NUMA_NO_NODE);
 }
 EXPORT_SYMBOL(scsi_allocate_command);
 
-- 
1.7.1

[patch,v3,repost 05/10] sd: use alloc_disk_node

[Jeff Moyer]

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/sd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 12f6fdf..a5dae6b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2714,7 +2714,7 @@ static int sd_probe(struct device *dev)
 	if (!sdkp)
 		goto out;
 
-	gd = alloc_disk(SD_MINORS);
+	gd = alloc_disk_node(SD_MINORS, scsi_host_get_numa_node(sdp->host));
 	if (!gd)
 		goto out_free;
 
-- 
1.7.1

[patch,v3,repost 06/10] ata: use scsi_host_alloc_node

[Jeff Moyer]

Acked-by: Jeff Garzik <jgarzik@redhat.com>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/ata/libata-scsi.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e3bda07..9d5dd09 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3586,7 +3586,8 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		struct Scsi_Host *shost;
 
 		rc = -ENOMEM;
-		shost = scsi_host_alloc(sht, sizeof(struct ata_port *));
+		shost = scsi_host_alloc_node(sht, sizeof(struct ata_port *),
+					     dev_to_node(host->dev));
 		if (!shost)
 			goto err_alloc;
 
-- 
1.7.1

[patch,v3,repost 07/10] megaraid_sas: use scsi_host_alloc_node

[Jeff Moyer]

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index d2c5366..707a6cd 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4020,8 +4020,9 @@ megasas_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (megasas_set_dma_mask(pdev))
 		goto fail_set_dma_mask;
 
-	host = scsi_host_alloc(&megasas_template,
-			       sizeof(struct megasas_instance));
+	host = scsi_host_alloc_node(&megasas_template,
+				    sizeof(struct megasas_instance),
+				    dev_to_node(&pdev->dev));
 
 	if (!host) {
 		printk(KERN_DEBUG "megasas: scsi_host_alloc failed\n");
-- 
1.7.1

[patch,v3,repost 08/10] mpt2sas: use scsi_host_alloc_node

[Jeff Moyer]

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/mpt2sas/mpt2sas_scsih.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index af4e6c4..a4d6b36 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -8011,8 +8011,8 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct MPT2SAS_ADAPTER *ioc;
 	struct Scsi_Host *shost;
 
-	shost = scsi_host_alloc(&scsih_driver_template,
-	    sizeof(struct MPT2SAS_ADAPTER));
+	shost = scsi_host_alloc_node(&scsih_driver_template,
+	    sizeof(struct MPT2SAS_ADAPTER), dev_to_node(&pdev->dev));
 	if (!shost)
 		return -ENODEV;
 
-- 
1.7.1

[patch,v3,repost 09/10] lpfc: use scsi_host_alloc_node

[Jeff Moyer]

Acked-By: James Smart  <james.smart@emulex.com>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/scsi/lpfc/lpfc_init.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 7dc4218..65956d3 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -3051,11 +3051,13 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, struct device *dev)
 	int error = 0;
 
 	if (dev != &phba->pcidev->dev)
-		shost = scsi_host_alloc(&lpfc_vport_template,
-					sizeof(struct lpfc_vport));
+		shost = scsi_host_alloc_node(&lpfc_vport_template,
+					     sizeof(struct lpfc_vport),
+					     dev_to_node(&phba->pcidev->dev));
 	else
-		shost = scsi_host_alloc(&lpfc_template,
-					sizeof(struct lpfc_vport));
+		shost = scsi_host_alloc_node(&lpfc_template,
+					     sizeof(struct lpfc_vport),
+					     dev_to_node(&phba->pcidev->dev));
 	if (!shost)
 		goto out;
 
-- 
1.7.1

[patch,v3,repost 10/10] cciss: use blk_init_queue_node

[Jeff Moyer]

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 drivers/block/cciss.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index b0f553b..5fe5546 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -1930,7 +1930,8 @@ static void cciss_get_serial_no(ctlr_info_t *h, int logvol,
 static int cciss_add_disk(ctlr_info_t *h, struct gendisk *disk,
 				int drv_index)
 {
-	disk->queue = blk_init_queue(do_cciss_request, &h->lock);
+	disk->queue = blk_init_queue_node(do_cciss_request, &h->lock,
+					  dev_to_node(&h->dev));
 	if (!disk->queue)
 		goto init_queue_failure;
 	sprintf(disk->disk_name, "cciss/c%dd%d", h->ctlr, drv_index);
-- 
1.7.1

Re: [patch,v3,repost 07/10] megaraid_sas: use scsi_host_alloc_node

[adam radford]

On Tue, Nov 27, 2012 at 8:46 AM, Jeff Moyer <jmoyer@redhat.com> wrote:
>
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index d2c5366..707a6cd 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -4020,8 +4020,9 @@ megasas_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
>         if (megasas_set_dma_mask(pdev))
>                 goto fail_set_dma_mask;
>
> -       host = scsi_host_alloc(&megasas_template,
> -                              sizeof(struct megasas_instance));
> +       host = scsi_host_alloc_node(&megasas_template,
> +                                   sizeof(struct megasas_instance),
> +                                   dev_to_node(&pdev->dev));
>
>         if (!host) {
>                 printk(KERN_DEBUG "megasas: scsi_host_alloc failed\n");

Acked-by: Adam Radford <aradford@gmail.com>

Re: [patch,v3,repost 00/10] make I/O path allocations more numa-friendly

[Jeff Moyer]

Jeff Moyer <jmoyer@redhat.com> writes:

> Hi,
>
> This patch set makes memory allocations for data structures used in
> the I/O path more numa friendly by allocating them from the same numa
> node as the storage device.  I've only converted a handful of drivers
> at this point.  My testing is limited by the hardware I have on hand.
> Using these patches, I was able to max out the bandwidth of the storage
> controller when issuing I/O from any node on my 4 node system.  Without
> the patch, I/O from nodes remote to the storage device would suffer a
> penalty ranging from 6-12%.  Given my relatively low-end setup[1], I
> wouldn't be surprised if others could show a more significant performance
> advantage.
>
> This is a repost of the last posting.  The only changes are additional
> reviewed-by/acked-by tags.  I think this version is ready for inclusion.
> James, would you mind taking a look?

James?  Do you have any objections to including this for 3.8?

-Jeff

Re: [patch,v3,repost 00/10] make I/O path allocations more numa-friendly

[James Bottomley]

On Mon, 2012-12-10 at 12:59 -0500, Jeff Moyer wrote:
> Jeff Moyer <jmoyer@redhat.com> writes:
> 
> > Hi,
> >
> > This patch set makes memory allocations for data structures used in
> > the I/O path more numa friendly by allocating them from the same numa
> > node as the storage device.  I've only converted a handful of drivers
> > at this point.  My testing is limited by the hardware I have on hand.
> > Using these patches, I was able to max out the bandwidth of the storage
> > controller when issuing I/O from any node on my 4 node system.  Without
> > the patch, I/O from nodes remote to the storage device would suffer a
> > penalty ranging from 6-12%.  Given my relatively low-end setup[1], I
> > wouldn't be surprised if others could show a more significant performance
> > advantage.
> >
> > This is a repost of the last posting.  The only changes are additional
> > reviewed-by/acked-by tags.  I think this version is ready for inclusion.
> > James, would you mind taking a look?
> 
> James?  Do you have any objections to including this for 3.8?

Probably for 3.9 since the 3.8 merge window is upon us.

Do we actually have any performance numbers from the big system people?
That's really a must for this type of work.

It's missing acks from the affected drivers; that's not a show stopper
but it would be better to have them.

James

Re: [patch,v3,repost 00/10] make I/O path allocations more numa-friendly

[Jeff Moyer]

James Bottomley <James.Bottomley@HansenPartnership.com> writes:

> On Mon, 2012-12-10 at 12:59 -0500, Jeff Moyer wrote:
>> Jeff Moyer <jmoyer@redhat.com> writes:
>> 
>> > Hi,
>> >
>> > This patch set makes memory allocations for data structures used in
>> > the I/O path more numa friendly by allocating them from the same numa
>> > node as the storage device.  I've only converted a handful of drivers
>> > at this point.  My testing is limited by the hardware I have on hand.
>> > Using these patches, I was able to max out the bandwidth of the storage
>> > controller when issuing I/O from any node on my 4 node system.  Without
>> > the patch, I/O from nodes remote to the storage device would suffer a
>> > penalty ranging from 6-12%.  Given my relatively low-end setup[1], I
>> > wouldn't be surprised if others could show a more significant performance
>> > advantage.
>> >
>> > This is a repost of the last posting.  The only changes are additional
>> > reviewed-by/acked-by tags.  I think this version is ready for inclusion.
>> > James, would you mind taking a look?
>> 
>> James?  Do you have any objections to including this for 3.8?
>
> Probably for 3.9 since the 3.8 merge window is upon us.

OK.

> Do we actually have any performance numbers from the big system people?
> That's really a must for this type of work.

I'm working on getting some numbers.

> It's missing acks from the affected drivers; that's not a show stopper
> but it would be better to have them.

Well, we could trim the driver list to those that have ACKs, but the
driver changes are trivial.

Thanks,
Jeff