diff options
author | Ming Lei <ming.lei@redhat.com> | 2018-03-13 05:42:39 -0400 |
---|---|---|
committer | Martin K. Petersen <martin.petersen@oracle.com> | 2018-03-14 23:31:13 -0400 |
commit | 8b834bff1b73dce46f4e9f5e84af6f73fed8b0ef (patch) | |
tree | 8eb55c8bed7a25767bed13dbaaa4b9021e79f83a | |
parent | 14bc1dff74277408f08661d03e785710a46e0699 (diff) |
scsi: hpsa: fix selection of reply queue
Since commit 84676c1f21e8 ("genirq/affinity: assign vectors to all
possible CPUs") we could end up with an MSI-X vector that did not have
any online CPUs mapped. This would lead to I/O hangs since there was no
CPU to receive the completion.
Retrieve IRQ affinity information using pci_irq_get_affinity() and use
this mapping to choose a reply queue.
[mkp: tweaked commit desc]
Cc: Hannes Reinecke <hare@suse.de>
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: Laurence Oberman <loberman@redhat.com>
Cc: Meelis Roos <mroos@linux.ee>
Cc: Artem Bityutskiy <artem.bityutskiy@intel.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Fixes: 84676c1f21e8 ("genirq/affinity: assign vectors to all possible CPUs")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
Tested-by: Laurence Oberman <loberman@redhat.com>
Tested-by: Don Brace <don.brace@microsemi.com>
Tested-by: Artem Bityutskiy <artem.bityutskiy@intel.com>
Acked-by: Don Brace <don.brace@microsemi.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
-rw-r--r-- | drivers/scsi/hpsa.c | 73 | ||||
-rw-r--r-- | drivers/scsi/hpsa.h | 1 |
2 files changed, 55 insertions, 19 deletions
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 87b260e403ec..31423b6dc26d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c | |||
@@ -1045,11 +1045,7 @@ static void set_performant_mode(struct ctlr_info *h, struct CommandList *c, | |||
1045 | c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1); | 1045 | c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1); |
1046 | if (unlikely(!h->msix_vectors)) | 1046 | if (unlikely(!h->msix_vectors)) |
1047 | return; | 1047 | return; |
1048 | if (likely(reply_queue == DEFAULT_REPLY_QUEUE)) | 1048 | c->Header.ReplyQueue = reply_queue; |
1049 | c->Header.ReplyQueue = | ||
1050 | raw_smp_processor_id() % h->nreply_queues; | ||
1051 | else | ||
1052 | c->Header.ReplyQueue = reply_queue % h->nreply_queues; | ||
1053 | } | 1049 | } |
1054 | } | 1050 | } |
1055 | 1051 | ||
@@ -1063,10 +1059,7 @@ static void set_ioaccel1_performant_mode(struct ctlr_info *h, | |||
1063 | * Tell the controller to post the reply to the queue for this | 1059 | * Tell the controller to post the reply to the queue for this |
1064 | * processor. This seems to give the best I/O throughput. | 1060 | * processor. This seems to give the best I/O throughput. |
1065 | */ | 1061 | */ |
1066 | if (likely(reply_queue == DEFAULT_REPLY_QUEUE)) | 1062 | cp->ReplyQueue = reply_queue; |
1067 | cp->ReplyQueue = smp_processor_id() % h->nreply_queues; | ||
1068 | else | ||
1069 | cp->ReplyQueue = reply_queue % h->nreply_queues; | ||
1070 | /* | 1063 | /* |
1071 | * Set the bits in the address sent down to include: | 1064 | * Set the bits in the address sent down to include: |
1072 | * - performant mode bit (bit 0) | 1065 | * - performant mode bit (bit 0) |
@@ -1087,10 +1080,7 @@ static void set_ioaccel2_tmf_performant_mode(struct ctlr_info *h, | |||
1087 | /* Tell the controller to post the reply to the queue for this | 1080 | /* Tell the controller to post the reply to the queue for this |
1088 | * processor. This seems to give the best I/O throughput. | 1081 | * processor. This seems to give the best I/O throughput. |
1089 | */ | 1082 | */ |
1090 | if (likely(reply_queue == DEFAULT_REPLY_QUEUE)) | 1083 | cp->reply_queue = reply_queue; |
1091 | cp->reply_queue = smp_processor_id() % h->nreply_queues; | ||
1092 | else | ||
1093 | cp->reply_queue = reply_queue % h->nreply_queues; | ||
1094 | /* Set the bits in the address sent down to include: | 1084 | /* Set the bits in the address sent down to include: |
1095 | * - performant mode bit not used in ioaccel mode 2 | 1085 | * - performant mode bit not used in ioaccel mode 2 |
1096 | * - pull count (bits 0-3) | 1086 | * - pull count (bits 0-3) |
@@ -1109,10 +1099,7 @@ static void set_ioaccel2_performant_mode(struct ctlr_info *h, | |||
1109 | * Tell the controller to post the reply to the queue for this | 1099 | * Tell the controller to post the reply to the queue for this |
1110 | * processor. This seems to give the best I/O throughput. | 1100 | * processor. This seems to give the best I/O throughput. |
1111 | */ | 1101 | */ |
1112 | if (likely(reply_queue == DEFAULT_REPLY_QUEUE)) | 1102 | cp->reply_queue = reply_queue; |
1113 | cp->reply_queue = smp_processor_id() % h->nreply_queues; | ||
1114 | else | ||
1115 | cp->reply_queue = reply_queue % h->nreply_queues; | ||
1116 | /* | 1103 | /* |
1117 | * Set the bits in the address sent down to include: | 1104 | * Set the bits in the address sent down to include: |
1118 | * - performant mode bit not used in ioaccel mode 2 | 1105 | * - performant mode bit not used in ioaccel mode 2 |
@@ -1157,6 +1144,8 @@ static void __enqueue_cmd_and_start_io(struct ctlr_info *h, | |||
1157 | { | 1144 | { |
1158 | dial_down_lockup_detection_during_fw_flash(h, c); | 1145 | dial_down_lockup_detection_during_fw_flash(h, c); |
1159 | atomic_inc(&h->commands_outstanding); | 1146 | atomic_inc(&h->commands_outstanding); |
1147 | |||
1148 | reply_queue = h->reply_map[raw_smp_processor_id()]; | ||
1160 | switch (c->cmd_type) { | 1149 | switch (c->cmd_type) { |
1161 | case CMD_IOACCEL1: | 1150 | case CMD_IOACCEL1: |
1162 | set_ioaccel1_performant_mode(h, c, reply_queue); | 1151 | set_ioaccel1_performant_mode(h, c, reply_queue); |
@@ -7376,6 +7365,26 @@ static void hpsa_disable_interrupt_mode(struct ctlr_info *h) | |||
7376 | h->msix_vectors = 0; | 7365 | h->msix_vectors = 0; |
7377 | } | 7366 | } |
7378 | 7367 | ||
7368 | static void hpsa_setup_reply_map(struct ctlr_info *h) | ||
7369 | { | ||
7370 | const struct cpumask *mask; | ||
7371 | unsigned int queue, cpu; | ||
7372 | |||
7373 | for (queue = 0; queue < h->msix_vectors; queue++) { | ||
7374 | mask = pci_irq_get_affinity(h->pdev, queue); | ||
7375 | if (!mask) | ||
7376 | goto fallback; | ||
7377 | |||
7378 | for_each_cpu(cpu, mask) | ||
7379 | h->reply_map[cpu] = queue; | ||
7380 | } | ||
7381 | return; | ||
7382 | |||
7383 | fallback: | ||
7384 | for_each_possible_cpu(cpu) | ||
7385 | h->reply_map[cpu] = 0; | ||
7386 | } | ||
7387 | |||
7379 | /* If MSI/MSI-X is supported by the kernel we will try to enable it on | 7388 | /* If MSI/MSI-X is supported by the kernel we will try to enable it on |
7380 | * controllers that are capable. If not, we use legacy INTx mode. | 7389 | * controllers that are capable. If not, we use legacy INTx mode. |
7381 | */ | 7390 | */ |
@@ -7771,6 +7780,10 @@ static int hpsa_pci_init(struct ctlr_info *h) | |||
7771 | err = hpsa_interrupt_mode(h); | 7780 | err = hpsa_interrupt_mode(h); |
7772 | if (err) | 7781 | if (err) |
7773 | goto clean1; | 7782 | goto clean1; |
7783 | |||
7784 | /* setup mapping between CPU and reply queue */ | ||
7785 | hpsa_setup_reply_map(h); | ||
7786 | |||
7774 | err = hpsa_pci_find_memory_BAR(h->pdev, &h->paddr); | 7787 | err = hpsa_pci_find_memory_BAR(h->pdev, &h->paddr); |
7775 | if (err) | 7788 | if (err) |
7776 | goto clean2; /* intmode+region, pci */ | 7789 | goto clean2; /* intmode+region, pci */ |
@@ -8480,6 +8493,28 @@ static struct workqueue_struct *hpsa_create_controller_wq(struct ctlr_info *h, | |||
8480 | return wq; | 8493 | return wq; |
8481 | } | 8494 | } |
8482 | 8495 | ||
8496 | static void hpda_free_ctlr_info(struct ctlr_info *h) | ||
8497 | { | ||
8498 | kfree(h->reply_map); | ||
8499 | kfree(h); | ||
8500 | } | ||
8501 | |||
8502 | static struct ctlr_info *hpda_alloc_ctlr_info(void) | ||
8503 | { | ||
8504 | struct ctlr_info *h; | ||
8505 | |||
8506 | h = kzalloc(sizeof(*h), GFP_KERNEL); | ||
8507 | if (!h) | ||
8508 | return NULL; | ||
8509 | |||
8510 | h->reply_map = kzalloc(sizeof(*h->reply_map) * nr_cpu_ids, GFP_KERNEL); | ||
8511 | if (!h->reply_map) { | ||
8512 | kfree(h); | ||
8513 | return NULL; | ||
8514 | } | ||
8515 | return h; | ||
8516 | } | ||
8517 | |||
8483 | static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) | 8518 | static int hpsa_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) |
8484 | { | 8519 | { |
8485 | int dac, rc; | 8520 | int dac, rc; |
@@ -8517,7 +8552,7 @@ reinit_after_soft_reset: | |||
8517 | * the driver. See comments in hpsa.h for more info. | 8552 | * the driver. See comments in hpsa.h for more info. |
8518 | */ | 8553 | */ |
8519 | BUILD_BUG_ON(sizeof(struct CommandList) % COMMANDLIST_ALIGNMENT); | 8554 | BUILD_BUG_ON(sizeof(struct CommandList) % COMMANDLIST_ALIGNMENT); |
8520 | h = kzalloc(sizeof(*h), GFP_KERNEL); | 8555 | h = hpda_alloc_ctlr_info(); |
8521 | if (!h) { | 8556 | if (!h) { |
8522 | dev_err(&pdev->dev, "Failed to allocate controller head\n"); | 8557 | dev_err(&pdev->dev, "Failed to allocate controller head\n"); |
8523 | return -ENOMEM; | 8558 | return -ENOMEM; |
@@ -8916,7 +8951,7 @@ static void hpsa_remove_one(struct pci_dev *pdev) | |||
8916 | h->lockup_detected = NULL; /* init_one 2 */ | 8951 | h->lockup_detected = NULL; /* init_one 2 */ |
8917 | /* (void) pci_disable_pcie_error_reporting(pdev); */ /* init_one 1 */ | 8952 | /* (void) pci_disable_pcie_error_reporting(pdev); */ /* init_one 1 */ |
8918 | 8953 | ||
8919 | kfree(h); /* init_one 1 */ | 8954 | hpda_free_ctlr_info(h); /* init_one 1 */ |
8920 | } | 8955 | } |
8921 | 8956 | ||
8922 | static int hpsa_suspend(__attribute__((unused)) struct pci_dev *pdev, | 8957 | static int hpsa_suspend(__attribute__((unused)) struct pci_dev *pdev, |
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 018f980a701c..fb9f5e7f8209 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h | |||
@@ -158,6 +158,7 @@ struct bmic_controller_parameters { | |||
158 | #pragma pack() | 158 | #pragma pack() |
159 | 159 | ||
160 | struct ctlr_info { | 160 | struct ctlr_info { |
161 | unsigned int *reply_map; | ||
161 | int ctlr; | 162 | int ctlr; |
162 | char devname[8]; | 163 | char devname[8]; |
163 | char *product_name; | 164 | char *product_name; |