Re: [PATCH 1/2] MAINTAINERS: ibmvscsi driver maintainer change
On 01/12/2015 06:31 PM, Tyrel Datwyler wrote: Change maintainer of ibmvscsi driver to Tyrel Datwyler. Signed-off-by: Tyrel Datwyler tyr...@linux.vnet.ibm.com Cc: Nathan Fontenot nf...@linux.vnet.ibm.com Cc: Brian King brk...@linux.vnet.ibm.com Acked-by: Nathan Fontenot nf...@linux.vnet.ibm.com --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 79b2e4b..a646b94 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4754,7 +4754,7 @@ S: Supported F: drivers/net/ethernet/ibm/ibmveth.* IBM Power Virtual SCSI Device Drivers -M: Nathan Fontenot nf...@linux.vnet.ibm.com +M: Tyrel Datwyler tyr...@linux.vnet.ibm.com L: linux-scsi@vger.kernel.org S: Supported F: drivers/scsi/ibmvscsi/ibmvscsi* -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dm + blk-mq soft lockup complaint
On Wed, Jan 14 2015 at 4:16am -0500, Bart Van Assche bart.vanass...@sandisk.com wrote: On 01/13/15 17:21, Mike Snitzer wrote: OK, I assume you specified the mpath device for the test that failed. Yes, of course ... This test works fine on my 100MB scsi_debug device with 4 paths exported over virtio-blk to a guest that assembles the mpath device. Could be a hang that is unique to scsi-mq. Any chance you'd be willing to provide a HOWTO for setting up your SRP/iscsi configuration? Are you carrying any related changes that are not upstream? (I can hunt down the email in this thread where you describe your kernel tree...) I'll try to reproduce but this info could be useful to others that are more scsi-mq inclined who might need to chase this too. The four patches I had used in my tests at the initiator side and that are not yet in v3.19-rc4 have been attached to this e-mail (I have not yet had the time to post all of these patches for review). This is how my I had configured the initiator system: * If the version of the srptools package supplied by your distro is lower than 1.0.2, build and install the latest version from the source code available at git://git.openfabrics.org/~bvanassche/srptools.git/.git. * Install the latest version of lsscsi (http://sg.danny.cz/scsi/lsscsi.html). This version has SRP transport support but is not yet in any distro AFAIK. * Build and install a kernel = v3.19-rc4 that includes the dm patches at the start of this e-mail thread. * Check whether the IB links are up (should display State: Active): ibstat | grep State: * Spread completion interrupts statically over CPU cores, e.g. via the attached script (spread-mlx4-ib-interrupts). * Check whether the SRP target system is visible from the SRP initiator system - the command below should print at least one line: ibsrpdm -c * Enable blk-mq: echo Y /sys/module/scsi_mod/parameters/use_blk_mq * Configure the SRP kernel module parameters as follows: echo 'options ib_srp cmd_sg_entries=255 dev_loss_tmo=60 ch_count=6' /etc/modprobe.d/ib_srp.conf * Unload and reload the SRP initiator kernel module to apply these parameters: rmmod ib_srp; modprobe ib_srp * Start srpd and wait until SRP login has finished: systemctl start srpd while ! lsscsi -t | grep -q srp:; do sleep 1; done * Start multipathd and check the table it has built: systemctl start multipathd dmsetup table /dev/dm-0 * Set the I/O scheduler to noop, disable add_random and set rq_affinity to 2 for all SRP and dm block devices. * Run the I/O load of your preference. Thanks for all this info. But I don't have an IB setup readily available to test with. We are setting up an IB testbed in the lab and can hopefully work through your setup in the coming weeks. IB aside, I haven't been following along close enough on scsi-mq developments, but does a regular iscsi initiator have support for scsi-mq? I'd like to validate scsi-mq devices with the dm-mpath changes. Mike -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: Avoid crashing if device uses DIX but adapter does not support it
From: Ewan D. Milne emi...@redhat.com This can happen if a multipathed device uses DIX and another path is added via an adapter that does not support it. Multipath should not allow this path to be added, but we should not depend upon that to avoid crashing. Signed-off-by: Ewan D. Milne emi...@redhat.com --- drivers/scsi/scsi_lib.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6d5c0b8..4f14f4a 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1143,7 +1143,17 @@ int scsi_init_io(struct scsi_cmnd *cmd) struct scsi_data_buffer *prot_sdb = cmd-prot_sdb; int ivecs, count; - BUG_ON(prot_sdb == NULL); + if (prot_sdb == NULL) { + /* +* This can happen if someone (e.g. multipath) +* queues a command to a device on an adapter +* that does not support T10 PI. +*/ + WARN_ON_ONCE(1); + error = BLKPREP_KILL; + goto err_exit; + } + ivecs = blk_rq_count_integrity_sg(rq-q, rq-bio); if (scsi_alloc_sgtable(prot_sdb, ivecs, is_mq)) { -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/48] hpsa: refactor hpsa_find_board_params() to encapsulate legacy test
From: Webb Scales web...@hp.com Encapsulate the conditional predicate which tests for legacy controllers in a separate function and rework the code comments. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Webb Scales web...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index ec2503a..08d96a9 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3107,7 +3107,8 @@ out: kfree(logdev_list); } -/* hpsa_scatter_gather takes a struct scsi_cmnd, (cmd), and does the pci +/* + * hpsa_scatter_gather takes a struct scsi_cmnd, (cmd), and does the pci * dma mapping and fills in the scatter gather entries of the * hpsa command, cp. */ @@ -3165,7 +3166,7 @@ static int hpsa_scatter_gather(struct ctlr_info *h, sglist_finished: cp-Header.SGList = (u8) use_sg; /* no. SGs contig in this cmd */ - cp-Header.SGTotal = cpu_to_le16(use_sg); /* total sgs in this cmd list */ + cp-Header.SGTotal = cpu_to_le16(use_sg); /* total sgs in cmd list */ return 0; } @@ -6162,6 +6163,15 @@ static void hpsa_get_max_perf_mode_cmds(struct ctlr_info *h) } } +/* If the controller reports that the total max sg entries is greater than 512, + * then we know that chained SG blocks work. (Original smart arrays did not + * support chained SG blocks and would return zero for max sg entries.) + */ +static int hpsa_supports_chained_sg_blocks(struct ctlr_info *h) +{ + return h-maxsgentries 512; +} + /* Interrogate the hardware for some limits: * max commands, max SG elements without chaining, and with chaining, * SG chain block size, etc. @@ -6172,18 +6182,20 @@ static void hpsa_find_board_params(struct ctlr_info *h) h-nr_cmds = h-max_commands - 4; /* Allow room for some ioctls */ h-maxsgentries = readl((h-cfgtable-MaxScatterGatherElements)); h-fw_support = readl((h-cfgtable-misc_fw_support)); - /* -* Limit in-command s/g elements to 32 save dma'able memory. -* Howvever spec says if 0, use 31 -*/ - h-max_cmd_sg_entries = 31; - if (h-maxsgentries 512) { + if (hpsa_supports_chained_sg_blocks(h)) { + /* Limit in-command s/g elements to 32 save dma'able memory. */ h-max_cmd_sg_entries = 32; h-chainsize = h-maxsgentries - h-max_cmd_sg_entries; h-maxsgentries--; /* save one for chain pointer */ } else { - h-chainsize = 0; + /* +* Original smart arrays supported at most 31 s/g entries +* embedded inline in the command (trying to use more +* would lock up the controller) +*/ + h-max_cmd_sg_entries = 31; h-maxsgentries = 31; /* default to traditional values */ + h-chainsize = 0; } /* Find out what task management functions are supported and cache */ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/48] hpsa: remove 0x from queue depth print which is in decimal
From: Robert Elliott elli...@hp.com The queue depth printed at startup is in decimal, so shouldn't have a 0x prefix. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 97bb718..fbeef5b 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5931,7 +5931,7 @@ static void print_cfg_table(struct device *dev, struct CfgTable __iomem *tb) readl((tb-HostWrite.CoalIntDelay))); dev_info(dev,Coalesce Interrupt Count = 0x%x\n, readl((tb-HostWrite.CoalIntCount))); - dev_info(dev,Max outstanding commands = 0x%d\n, + dev_info(dev,Max outstanding commands = %d\n, readl((tb-CmdsOutMax))); dev_info(dev,Bus Types = 0x%x\n, readl((tb-BusTypes))); for (i = 0; i 16; i++) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/48] hpsa: propagate hard_reset failures in reset_devices mode
From: Robert Elliott elli...@hp.com Return the real reason for kdump_hard_reset failure rather than change them all to -ENODEV. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index fbeef5b..92ac76a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6416,11 +6416,8 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) * performant mode. Or, it might be 640x, which can't reset * due to concerns about shared bbwc between 6402/6404 pair. */ - if (rc) { - if (rc != -ENOTSUPP) /* just try to do the kdump anyhow. */ - rc = -ENODEV; + if (rc) goto out_disable; - } /* Now try to get the controller to respond to a no-op */ dev_warn(pdev-dev, Waiting for controller to respond to no-op\n); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/48] hpsa: change how SA controllers are reset
Change how SA controllers are reset by changing PCI power levels. The hpsa driver was finding the PCI_PM_CTRL_STATE_MASK offset then reading/writing a bitmask to change the power state. There are kernel functions that do the same operations. Better to use the kernel functions. Signed-off-by: Don Brace don.br...@pmcs.com Reviewed-by: Scott Teel scott.t...@pmcs.com Reviewed-by: Webb Scales web...@hp.com --- drivers/scsi/hpsa.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 9edacff..371d0a8 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5689,30 +5689,22 @@ static int hpsa_controller_hard_reset(struct pci_dev *pdev, * the controller, place the interface device in D3 then to D0, * this causes a secondary PCI reset which will reset the * controller. */ - int pos; - u16 pmcsr; - - pos = pci_find_capability(pdev, PCI_CAP_ID_PM); - if (pos == 0) { - dev_err(pdev-dev, - hpsa_reset_controller: - PCI PM not supported\n); - return -ENODEV; - } + + int rc = 0; + dev_info(pdev-dev, using PCI PM to reset controller\n); + /* enter the D3hot power management state */ - pci_read_config_word(pdev, pos + PCI_PM_CTRL, - (__force u16 *)pmcsr); - pmcsr = ~PCI_PM_CTRL_STATE_MASK; - pmcsr |= (__force u16) PCI_D3hot; - pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr); + rc = pci_set_power_state(pdev, PCI_D3hot); + if (rc) + return rc; msleep(500); /* enter the D0 power management state */ - pmcsr = ~PCI_PM_CTRL_STATE_MASK; - pmcsr |= (__force u16) PCI_D0; - pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr); + rc = pci_set_power_state(pdev, PCI_D0); + if (rc) + return rc; /* * The P600 requires a small delay when changing states. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/48] hpsa: correct change_queue_depth
scsi_adjust_queue_depth was changed to scsi_change_queue_depth Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 371d0a8..7dfe829 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -217,6 +217,7 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd); static void hpsa_scan_start(struct Scsi_Host *); static int hpsa_scan_finished(struct Scsi_Host *sh, unsigned long elapsed_time); +static int hpsa_change_queue_depth(struct scsi_device *sdev, int qdepth); static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd); static int hpsa_eh_abort_handler(struct scsi_cmnd *scsicmd); @@ -672,7 +673,7 @@ static struct scsi_host_template hpsa_driver_template = { .queuecommand = hpsa_scsi_queue_command, .scan_start = hpsa_scan_start, .scan_finished = hpsa_scan_finished, - .change_queue_depth = scsi_change_queue_depth, + .change_queue_depth = hpsa_change_queue_depth, .this_id= -1, .use_clustering = ENABLE_CLUSTERING, .eh_abort_handler = hpsa_eh_abort_handler, @@ -4007,6 +4008,19 @@ static void hpsa_scan_start(struct Scsi_Host *sh) spin_unlock_irqrestore(h-scan_lock, flags); } +static int hpsa_change_queue_depth(struct scsi_device *sdev, int qdepth) +{ + struct ctlr_info *h = sdev_to_hba(sdev); + + if (qdepth 1) + qdepth = 1; + else + if (qdepth h-nr_cmds) + qdepth = h-nr_cmds; + scsi_change_queue_depth(sdev, qdepth); + return sdev-queue_depth; +} + static int hpsa_scan_finished(struct Scsi_Host *sh, unsigned long elapsed_time) { -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/48] hpsa: adjust RAID-1, RAID-1ADM, and RAID-6 names
From: Robert Elliott elli...@hp.com HP now uses RAID-6 rather than RAID-ADG (Advanced Data Guarding) as the marketing name for our implementation of RAID-6. The driver considers RAID-1 and RAID-1+0 to be the same level, and considers RAID-1ADM and RAID-1+0ADM to be the same level. Parenthesis can be used to reflect the optional +0 portion of both those RAID levels. Rename: RAID-ADG to RAID-6 RAID-1(1+0) to RAID-1(+0) RAID-1(ADM) to RAID-1(+0)ADM Also, add another const after the pointer type as suggested by checkpatch.pl so the array is: static const char * const raid_label[] Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 7dfe829..4252b63 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -507,8 +507,8 @@ static inline int is_logical_dev_addr_mode(unsigned char scsi3addr[]) return (scsi3addr[3] 0xC0) == 0x40; } -static const char *raid_label[] = { 0, 4, 1(1+0), 5, 5+1, ADG, - 1(ADM), UNKNOWN +static const char * const raid_label[] = { 0, 4, 1(+0), 5, 5+1, 6, + 1(+0)ADM, UNKNOWN }; #define HPSA_RAID_00 #define HPSA_RAID_41 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/48] hpsa: propagate return value from board ID lookup
From: Robert Elliott elli...@hp.com If the board ID lookup function fails, return the return code rather than return -ENODEV. The only board ID failure reason right now is -ENODEV, so this just provides more informative prints in kdump and adapts to future changes. Tested with error injection while booting with reset_devices on the kernel command line: [ 62.804324] injecting error in inj_hpsa_lookup_board_id: 1 11 [ 62.804423] hpsa :04:00.0: Board ID not found (the pci probe layer does not print an additional message if -ENODEV is the reason) Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 92ac76a..85b3d73 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5812,8 +5812,12 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) */ rc = hpsa_lookup_board_id(pdev, board_id); - if (rc 0 || !ctlr_is_resettable(board_id)) { - dev_warn(pdev-dev, Not resetting device.\n); + if (rc 0) { + dev_warn(pdev-dev, Board ID not found\n); + return rc; + } + if (!ctlr_is_resettable(board_id)) { + dev_warn(pdev-dev, Controller not resettable\n); return -ENODEV; } @@ -6295,7 +6299,7 @@ static int hpsa_pci_init(struct ctlr_info *h) prod_index = hpsa_lookup_board_id(h-pdev, h-board_id); if (prod_index 0) - return -ENODEV; + return prod_index; h-product_name = products[prod_index].product_name; h-access = *(products[prod_index].access); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/48] hpsa: downgrade the Waiting for no-op print to dev_info
From: Robert Elliott elli...@hp.com There is nothing worrisome about the Waiting for controller to respond to no-op print, so use dev_info rather than dev_warn. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 85b3d73..ec2503a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6424,7 +6424,7 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) goto out_disable; /* Now try to get the controller to respond to a no-op */ - dev_warn(pdev-dev, Waiting for controller to respond to no-op\n); + dev_info(pdev-dev, Waiting for controller to respond to no-op\n); for (i = 0; i HPSA_POST_RESET_NOOP_RETRIES; i++) { if (hpsa_noop(pdev) == 0) break; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/48] hpsa: notice all request_irq errors
From: Robert Elliott elli...@hp.com In MSI and MSI-X mode, where hpsa asks for more than one interrupt, hpsa_request_irqs forgets if the first request_irq call failed if later ones succeed. It needs to exit the loop on any failure rather than continue, freeing all irqs that were requested until that point. Also, it needs to clear out the q numbers up to MAX_REPLY_QUEUES. The same is true for the general hpsa_free_irqs function. Tested with error injection of -ENOSYS on the 4th call: [9.277691] injecting error in inj_request_irq: 1 4 [9.277780] hpsa :02:00.0: failed to get irq 35 for hpsa1 [ 10.711623] scsi host1: Error handler scsi_eh_1 exiting [ 10.739170] hpsa: probe of :02:00.0 failed with error -38 Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 11d21ef..97bb718 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6508,6 +6508,8 @@ static void hpsa_free_irqs(struct ctlr_info *h) irq_set_affinity_hint(h-intr[i], NULL); free_irq(h-intr[i], h-q[i]); } + for (; i MAX_REPLY_QUEUES; i++) + h-q[i] = 0; } static int hpsa_request_irq(struct ctlr_info *h, @@ -6525,10 +6527,25 @@ static int hpsa_request_irq(struct ctlr_info *h, if (h-intr_mode == PERF_MODE_INT h-msix_vector 0) { /* If performant mode and MSI-X, use multiple reply queues */ - for (i = 0; i h-msix_vector; i++) + for (i = 0; i h-msix_vector; i++) { rc = request_irq(h-intr[i], msixhandler, 0, h-devname, h-q[i]); + if (rc) { + int j; + + dev_err(h-pdev-dev, + failed to get irq %d for %s\n, + h-intr[i], h-devname); + for (j = 0; j i; j++) { + free_irq(h-intr[j], h-q[j]); + h-q[j] = 0; + } + for (; j MAX_REPLY_QUEUES; j++) + h-q[j] = 0; + return rc; + } + } hpsa_irq_affinity_hints(h); } else { /* Use single reply pool */ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/48] hpsa: correct endian sparse warnings
Correct endiness issues reported by sparse. SA controllers are little endian. This patch ensures endiness correctness. Signed-off-by: Don Brace don.br...@pmcs.com Reviewed-by: Scott Teel scott.t...@pmcs.com Reviewed-by: Webb Scales web...@hp.com --- drivers/scsi/hpsa.c | 275 +++ drivers/scsi/hpsa_cmd.h | 172 +++-- 2 files changed, 198 insertions(+), 249 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 6bb4611..2b9baea 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -50,6 +50,7 @@ #include linux/jiffies.h #include linux/percpu-defs.h #include linux/percpu.h +#include asm/unaligned.h #include asm/div64.h #include hpsa_cmd.h #include hpsa.h @@ -229,7 +230,7 @@ static void check_ioctl_unit_attention(struct ctlr_info *h, struct CommandList *c); /* performant mode helper functions */ static void calc_bucket_map(int *bucket, int num_buckets, - int nsgs, int min_blocks, int *bucket_map); + int nsgs, int min_blocks, u32 *bucket_map); static void hpsa_put_ctlr_into_performant_mode(struct ctlr_info *h); static inline u32 next_command(struct ctlr_info *h, u8 q); static int hpsa_find_cfg_addrs(struct pci_dev *pdev, void __iomem *vaddr, @@ -919,7 +920,7 @@ static int hpsa_scsi_add_entry(struct ctlr_info *h, int hostno, /* If this device a non-zero lun of a multi-lun device * byte 4 of the 8-byte LUN addr will contain the logical -* unit no, zero otherise. +* unit no, zero otherwise. */ if (device-scsi3addr[4] == 0) { /* This is not a non-zero lun of a multi-lun device */ @@ -1504,7 +1505,7 @@ static int hpsa_map_sg_chain_block(struct ctlr_info *h, chain_block = h-cmd_sg_list[c-cmdindex]; chain_sg-Ext = cpu_to_le32(HPSA_SG_CHAIN); chain_len = sizeof(*chain_sg) * - (c-Header.SGTotal - h-max_cmd_sg_entries); + (le16_to_cpu(c-Header.SGTotal) - h-max_cmd_sg_entries); chain_sg-Len = cpu_to_le32(chain_len); temp64 = pci_map_single(h-pdev, chain_block, chain_len, PCI_DMA_TODEVICE); @@ -1693,7 +1694,7 @@ static void complete_scsi_command(struct CommandList *cp) scsi_dma_unmap(cmd); /* undo the DMA mappings */ if ((cp-cmd_type == CMD_SCSI) - (cp-Header.SGTotal h-max_cmd_sg_entries)) + (le16_to_cpu(cp-Header.SGTotal) h-max_cmd_sg_entries)) hpsa_unmap_sg_chain_block(h, cp); cmd-result = (DID_OK 16); /* host byte */ @@ -1726,8 +1727,10 @@ static void complete_scsi_command(struct CommandList *cp) */ if (cp-cmd_type == CMD_IOACCEL1) { struct io_accel1_cmd *c = h-ioaccel_cmd_pool[cp-cmdindex]; - cp-Header.SGList = cp-Header.SGTotal = scsi_sg_count(cmd); - cp-Request.CDBLen = c-io_flags IOACCEL1_IOFLAGS_CDBLEN_MASK; + cp-Header.SGList = scsi_sg_count(cmd); + cp-Header.SGTotal = cpu_to_le16(cp-Header.SGList); + cp-Request.CDBLen = le16_to_cpu(c-io_flags) + IOACCEL1_IOFLAGS_CDBLEN_MASK; cp-Header.tag = c-tag; memcpy(cp-Header.LUN.LunAddrBytes, c-CISS_LUN, 8); memcpy(cp-Request.CDB, c-CDB, cp-Request.CDBLen); @@ -2191,15 +2194,13 @@ static void hpsa_debug_map_buff(struct ctlr_info *h, int rc, le16_to_cpu(map_buff-row_cnt)); dev_info(h-pdev-dev, layout_map_count = %u\n, le16_to_cpu(map_buff-layout_map_count)); - dev_info(h-pdev-dev, flags = %u\n, + dev_info(h-pdev-dev, flags = 0x%x\n, le16_to_cpu(map_buff-flags)); - if (map_buff-flags RAID_MAP_FLAG_ENCRYPT_ON) - dev_info(h-pdev-dev, encrypytion = ON\n); - else - dev_info(h-pdev-dev, encrypytion = OFF\n); + dev_info(h-pdev-dev, encrypytion = %s\n, + le16_to_cpu(map_buff-flags) + RAID_MAP_FLAG_ENCRYPT_ON ? ON : OFF); dev_info(h-pdev-dev, dekindex = %u\n, le16_to_cpu(map_buff-dekindex)); - map_cnt = le16_to_cpu(map_buff-layout_map_count); for (map = 0; map map_cnt; map++) { dev_info(h-pdev-dev, Map%u:\n, map); @@ -2741,8 +2742,8 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_info *h, struct scsi_cmnd *scmd; /* scsi command within request being aborted */ struct hpsa_scsi_dev_t *d; /* device of request being aborted */ struct io_accel2_cmd *c2a; /* ioaccel2 command to abort */ - u32 it_nexus; /* 4 byte device handle for the ioaccel2 cmd */ - u32 scsi_nexus; /* 4 byte device handle for the ioaccel2 cmd */ + __le32 it_nexus;/* 4 byte device handle for the ioaccel2 cmd */ + __le32 scsi_nexus; /* 4
[PATCH 02/48] hpsa: fix memory leak in kdump hard reset
From: Tomas Henzl the...@redhat.com There is a potential memory leak in hpsa_kdump_hard_reset_controller. Reviewed-by: Don Brace don.br...@pmcs.com Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Tomas Henzl the...@redhat.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 2b9baea..d97f455 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5840,7 +5840,7 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) } rc = write_driver_ver_to_cfgtable(cfgtable); if (rc) - goto unmap_vaddr; + goto unmap_cfgtable; /* If reset via doorbell register is supported, use that. * There are two such methods. Favor the newest method. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/48] hpsa driver updates
This patch set is based on Linus's tree. The changes are: - correct sparse warnings - correct memory leaks - correct kdump issues - correct queue depth updates - cleanup function return values - cleanup and enhance messages - cleanup and enhance error handling - minor code refactoring - performance enhancements. - removal of command queueing in driver - elimination of race conditions around aborts - change to internal driver workqueues --- Don Brace (11): hpsa: correct endian sparse warnings hpsa: change how SA controllers are reset hpsa: correct change_queue_depth hpsa: do not queue commands internally in driver hpsa: use workqueue to resubmit failed ioaccel commands hpsa: honor queue depth of physical devices hpsa: count passthru cmds with atomics, not a spin locked int hpsa: slightly optimize SA5_performant_completed hpsa: return failed from device reset/abort handlers hpsa: add in gen9 controller model names hpsa: add in P840ar controller model name Fabian Frederick (1): hpsa: Fix -Wunused-but-set-variable warning Robert Elliott (16): hpsa: adjust RAID-1, RAID-1ADM, and RAID-6 names hpsa: rename free_irqs to hpsa_free_irqs hpsa: notice all request_irq errors hpsa: remove 0x from queue depth print which is in decimal hpsa: propagate hard_reset failures in reset_devices mode hpsa: propagate return value from board ID lookup hpsa: downgrade the Waiting for no-op print to dev_info hpsa: report failure to ioremap config table hpsa: rename hpsa_request_irq to hpsa_request_irqs hpsa: pass error from pci_set_consistent_dma_mask from hpsa_message hpsa: report allocation failures while allocating SG chain blocks hpsa: fix memory leak in hpsa_alloc_cmd_pool hpsa: avoid unneccesary calls to resource freeing functions hpsa: optimize cmd_alloc function by remembering last allocation hpsa: shorten the wait for the CISS doorbell mode change ack hpsa: detect and report failures changing controller transport modes Stephen Cameron (13): hpsa: trivial message and comment clean ups hpsa: reserve some commands for use by driver hpsa: get rid of cmd_special_alloc and cmd_special_free hpsa: do not request device rescan on every ioaccel path error hpsa: factor out hpsa_ciss_submit function hpsa: do not check for msi(x) in interrupt_pending hpsa: remove incorrect BUG_ONs checking for raid offload enable hpsa: do not ack controller events on controllers that do not support it hpsa: guard against overflowing raid map array hpsa: check for ctlr lockup after command allocation in main io path hpsa: do not use a void pointer for scsi_cmd field of struct CommandList hpsa: print CDBs instead of kernel virtual addresses for uncommon errors hpsa: do not use function pointers in fast path command submission Tomas Henzl (2): hpsa: fix memory leak in kdump hard reset hpsa: turn off interrupts when kdump starts Webb Scales (5): hpsa: refactor hpsa_find_board_params() to encapsulate legacy test hpsa: use per-controller work queue hpsa: fix race between abort handler and main i/o path hpsa: move SG descriptor set-up out of hpsa_scatter_gather() hpsa: refactor duplicated scan completion code into a new routine drivers/scsi/hpsa.c | 1796 +++ drivers/scsi/hpsa.h | 59 +- drivers/scsi/hpsa_cmd.h | 334 ++--- 3 files changed, 1150 insertions(+), 1039 deletions(-) -- Signature -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/48] hpsa: turn off interrupts when kdump starts
From: Tomas Henzl the...@redhat.com Sometimes when the card is restarted it may cause - irq 16: nobody cared (try booting with the irqpoll option) that is likely caused so, that the card, after the hard reset finishes, pulls on the irq. Disabling the ints before or after the hpsa_kdump_hard_reset_controller fixes it. At this point we can't know in which state the card is, so using SA5_INTR_OFF + SA5_REPLY_INTR_MASK_OFFSET defines directly, instead of the function the drivers provides, seems to be apropriate. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index d97f455..9edacff 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6370,6 +6370,7 @@ static void hpsa_hba_inquiry(struct ctlr_info *h) static int hpsa_init_reset_devices(struct pci_dev *pdev) { int rc, i; + void __iomem *vaddr; if (!reset_devices) return 0; @@ -6393,6 +6394,14 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) pci_set_master(pdev); + vaddr = pci_ioremap_bar(pdev, 0); + if (vaddr == NULL) { + rc = -ENOMEM; + goto out_disable; + } + writel(SA5_INTR_OFF, vaddr + SA5_REPLY_INTR_MASK_OFFSET); + iounmap(vaddr); + /* Reset the controller with a PCI power-cycle or via doorbell */ rc = hpsa_kdump_hard_reset_controller(pdev); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/48] hpsa: rename free_irqs to hpsa_free_irqs
From: Robert Elliott elli...@hp.com Change the function names to have hpsa prefix. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 43 ++- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 4252b63..dc81d88 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6491,6 +6491,25 @@ static void hpsa_irq_affinity_hints(struct ctlr_info *h) } } +/* clear affinity hints and free MSI-X, MSI, or legacy INTx vectors */ +static void hpsa_free_irqs(struct ctlr_info *h) +{ + int i; + + if (!h-msix_vector || h-intr_mode != PERF_MODE_INT) { + /* Single reply queue, only one irq to free */ + i = h-intr_mode; + irq_set_affinity_hint(h-intr[i], NULL); + free_irq(h-intr[i], h-q[i]); + return; + } + + for (i = 0; i h-msix_vector; i++) { + irq_set_affinity_hint(h-intr[i], NULL); + free_irq(h-intr[i], h-q[i]); + } +} + static int hpsa_request_irq(struct ctlr_info *h, irqreturn_t (*msixhandler)(int, void *), irqreturn_t (*intxhandler)(int, void *)) @@ -6555,27 +6574,9 @@ static int hpsa_kdump_soft_reset(struct ctlr_info *h) return 0; } -static void free_irqs(struct ctlr_info *h) -{ - int i; - - if (!h-msix_vector || h-intr_mode != PERF_MODE_INT) { - /* Single reply queue, only one irq to free */ - i = h-intr_mode; - irq_set_affinity_hint(h-intr[i], NULL); - free_irq(h-intr[i], h-q[i]); - return; - } - - for (i = 0; i h-msix_vector; i++) { - irq_set_affinity_hint(h-intr[i], NULL); - free_irq(h-intr[i], h-q[i]); - } -} - static void hpsa_free_irqs_and_disable_msix(struct ctlr_info *h) { - free_irqs(h); + hpsa_free_irqs(h); #ifdef CONFIG_PCI_MSI if (h-msix_vector) { if (h-pdev-msix_enabled) @@ -6937,7 +6938,7 @@ reinit_after_soft_reset: spin_lock_irqsave(h-lock, flags); h-access.set_intr_mask(h, HPSA_INTR_OFF); spin_unlock_irqrestore(h-lock, flags); - free_irqs(h); + hpsa_free_irqs(h); rc = hpsa_request_irq(h, hpsa_msix_discard_completions, hpsa_intx_discard_completions); if (rc) { @@ -6997,7 +6998,7 @@ reinit_after_soft_reset: clean4: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); - free_irqs(h); + hpsa_free_irqs(h); clean2: clean1: if (h-lockup_detected) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 15/48] hpsa: trivial message and comment clean ups
From: Stephen Cameron stephenmcame...@gmail.com Cleanup comments to be more specific. Make messages more informational. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 08d96a9..5973018 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5863,8 +5863,8 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) } else { use_doorbell = misc_fw_support MISC_FW_DOORBELL_RESET; if (use_doorbell) { - dev_warn(pdev-dev, Soft reset not supported. - Firmware update is required.\n); + dev_warn(pdev-dev, + Soft reset not supported. Firmware update is required.\n); rc = -ENOTSUPP; /* try soft reset */ goto unmap_cfgtable; } @@ -5884,8 +5884,7 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) rc = hpsa_wait_for_board_state(pdev, vaddr, BOARD_READY); if (rc) { dev_warn(pdev-dev, - failed waiting for board to become ready - after hard reset\n); + Failed waiting for board to become ready after hard reset\n); goto unmap_cfgtable; } @@ -5984,7 +5983,7 @@ static int find_PCI_BAR_index(struct pci_dev *pdev, unsigned long pci_bar_addr) } /* If MSI/MSI-X is supported by the kernel we will try to enable it on - * controllers that are capable. If not, we use IO-APIC mode. + * controllers that are capable. If not, we use legacy INTx mode. */ static void hpsa_interrupt_mode(struct ctlr_info *h) @@ -6003,7 +6002,7 @@ static void hpsa_interrupt_mode(struct ctlr_info *h) (h-board_id == 0x40820E11) || (h-board_id == 0x40830E11)) goto default_int_mode; if (pci_find_capability(h-pdev, PCI_CAP_ID_MSIX)) { - dev_info(h-pdev-dev, MSIX\n); + dev_info(h-pdev-dev, MSI-X capable controller\n); h-msix_vector = MAX_REPLY_QUEUES; if (h-msix_vector num_online_cpus()) h-msix_vector = num_online_cpus(); @@ -6024,7 +6023,7 @@ static void hpsa_interrupt_mode(struct ctlr_info *h) } single_msi_mode: if (pci_find_capability(h-pdev, PCI_CAP_ID_MSI)) { - dev_info(h-pdev-dev, MSI\n); + dev_info(h-pdev-dev, MSI capable controller\n); if (!pci_enable_msi(h-pdev)) h-msi_vector = 1; else @@ -6209,7 +6208,7 @@ static void hpsa_find_board_params(struct ctlr_info *h) static inline bool hpsa_CISS_signature_present(struct ctlr_info *h) { if (!check_signature(h-cfgtable-Signature, CISS, 4)) { - dev_warn(h-pdev-dev, not a valid CISS config table\n); + dev_err(h-pdev-dev, not a valid CISS config table\n); return false; } return true; @@ -6301,7 +6300,7 @@ static int hpsa_enter_simple_mode(struct ctlr_info *h) h-transMethod = CFGTBL_Trans_Simple; return 0; error: - dev_warn(h-pdev-dev, unable to get board into simple mode\n); + dev_err(h-pdev-dev, failed to enter simple mode\n); return -ENODEV; } @@ -7282,8 +7281,8 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) hpsa_wait_for_mode_change_ack(h); register_value = readl((h-cfgtable-TransportActive)); if (!(register_value CFGTBL_Trans_Performant)) { - dev_warn(h-pdev-dev, unable to get board into -performant mode\n); + dev_err(h-pdev-dev, + performant mode problem - transport not active\n); return; } /* Change the access methods to the performant access methods */ -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 17/48] hpsa: rename hpsa_request_irq to hpsa_request_irqs
From: Robert Elliott elli...@hp.com Make the function name more descriptive. We use more than one interrupt. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 354e7f8..f29f569 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6526,7 +6526,8 @@ static void hpsa_free_irqs(struct ctlr_info *h) h-q[i] = 0; } -static int hpsa_request_irq(struct ctlr_info *h, +/* returns 0 on success; cleans up and returns -Enn on error */ +static int hpsa_request_irqs(struct ctlr_info *h, irqreturn_t (*msixhandler)(int, void *), irqreturn_t (*intxhandler)(int, void *)) { @@ -6934,7 +6935,7 @@ reinit_after_soft_reset: /* make sure the board interrupts are off */ h-access.set_intr_mask(h, HPSA_INTR_OFF); - if (hpsa_request_irq(h, do_hpsa_intr_msi, do_hpsa_intr_intx)) + if (hpsa_request_irqs(h, do_hpsa_intr_msi, do_hpsa_intr_intx)) goto clean2; dev_info(pdev-dev, %s: 0x%x at IRQ %d%s using DAC\n, h-devname, pdev-device, @@ -6970,11 +6971,11 @@ reinit_after_soft_reset: h-access.set_intr_mask(h, HPSA_INTR_OFF); spin_unlock_irqrestore(h-lock, flags); hpsa_free_irqs(h); - rc = hpsa_request_irq(h, hpsa_msix_discard_completions, + rc = hpsa_request_irqs(h, hpsa_msix_discard_completions, hpsa_intx_discard_completions); if (rc) { - dev_warn(h-pdev-dev, Failed to request_irq after - soft reset.\n); + dev_warn(h-pdev-dev, + Failed to request_irq after soft reset.\n); goto clean4; } -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/48] hpsa: Fix -Wunused-but-set-variable warning
From: Fabian Frederick f...@skynet.be Remove unused variable in hpsa_free_cmd_pool. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Fabian Frederick f...@skynet.be Acked-by: Don Brace don.br...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index dc81d88..11d21ef 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6482,11 +6482,11 @@ static void hpsa_free_cmd_pool(struct ctlr_info *h) static void hpsa_irq_affinity_hints(struct ctlr_info *h) { - int i, cpu, rc; + int i, cpu; cpu = cpumask_first(cpu_online_mask); for (i = 0; i h-msix_vector; i++) { - rc = irq_set_affinity_hint(h-intr[i], get_cpu_mask(cpu)); + irq_set_affinity_hint(h-intr[i], get_cpu_mask(cpu)); cpu = cpumask_next(cpu, cpu_online_mask); } } -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 20/48] hpsa: fix memory leak in hpsa_alloc_cmd_pool
From: Robert Elliott elli...@hp.com Partial allocation failure wasn't handled correctly Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index dc328ce..a66a50e 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -211,6 +211,7 @@ static struct CommandList *cmd_special_alloc(struct ctlr_info *h); static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h, void *buff, size_t size, u16 page_code, unsigned char *scsi3addr, int cmd_type); +static void hpsa_free_cmd_pool(struct ctlr_info *h); #define VPD_PAGE (1 8) static int hpsa_scsi_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd); @@ -6471,9 +6472,12 @@ static int hpsa_allocate_cmd_pool(struct ctlr_info *h) || (h-cmd_pool == NULL) || (h-errinfo_pool == NULL)) { dev_err(h-pdev-dev, out of memory in %s, __func__); - return -ENOMEM; + goto clean_up; } return 0; +clean_up: + hpsa_free_cmd_pool(h); + return -ENOMEM; } static void hpsa_free_cmd_pool(struct ctlr_info *h) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/48] hpsa: report allocation failures while allocating SG chain blocks
From: Robert Elliott elli...@hp.com Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 64d17d1..dc328ce 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1480,13 +1480,17 @@ static int hpsa_allocate_sg_chain_blocks(struct ctlr_info *h) h-cmd_sg_list = kzalloc(sizeof(*h-cmd_sg_list) * h-nr_cmds, GFP_KERNEL); - if (!h-cmd_sg_list) + if (!h-cmd_sg_list) { + dev_err(h-pdev-dev, Failed to allocate SG list\n); return -ENOMEM; + } for (i = 0; i h-nr_cmds; i++) { h-cmd_sg_list[i] = kmalloc(sizeof(*h-cmd_sg_list[i]) * h-chainsize, GFP_KERNEL); - if (!h-cmd_sg_list[i]) + if (!h-cmd_sg_list[i]) { + dev_err(h-pdev-dev, Failed to allocate cmd SG\n); goto clean; + } } return 0; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 16/48] hpsa: report failure to ioremap config table
From: Robert Elliott elli...@hp.com Enhance error reporting. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 5973018..354e7f8 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6130,8 +6130,10 @@ static int hpsa_find_cfgtables(struct ctlr_info *h) return rc; h-cfgtable = remap_pci_mem(pci_resource_start(h-pdev, cfg_base_addr_index) + cfg_offset, sizeof(*h-cfgtable)); - if (!h-cfgtable) + if (!h-cfgtable) { + dev_err(h-pdev-dev, Failed mapping cfgtable\n); return -ENOMEM; + } rc = write_driver_ver_to_cfgtable(h-cfgtable); if (rc) return rc; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/48] hpsa: pass error from pci_set_consistent_dma_mask from hpsa_message
From: Robert Elliott elli...@hp.com Return the actual error code instead of a generic error code. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index f29f569..64d17d1 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5610,7 +5610,7 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode, err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32)); if (err) { iounmap(vaddr); - return -ENOMEM; + return err; } cmd = pci_alloc_consistent(pdev, cmd_sz, paddr64); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [LSF/MM TOPIC] Unifying the LIO and SCST target drivers
On Wed, 2015-01-14 at 11:05 +0100, Bart Van Assche wrote: The LIO and SCST SCSI target subsystems consist of the following components: * A core that processes SCSI commands and that provides common functionality like persistent reservations, LUN masking and an interface that allows configuration from user space. * Device handlers that allow this core to access SCSI devices, block devices and files uniformly as SCSI devices. * Target drivers that implement a storage protocol (iSCSI, FC, SRP, iSER, FCoE, ...) and that realize the SCSI request and response communication between the target system and an initiator system. A significant amount of code is shared between several LIO target drivers and the SCST target drivers that implement the same storage protocol. Since there are two sets of these drivers this means that each set has to be maintained, extended and tested separately. This means a lot of redundant work. The main difference between these two sets of drivers is the interface between the target drivers and the SCSI target core. Hence the proposal to discuss the unification of the API between SCSI target core and SCSI target drivers. Implementing a single unified API would have the following advantages: * A single set of target drivers works for both projects which means a reduction of the maintenance effort for those who maintain target drivers for target driver developers and target driver users. * This would increase the size of the user base for the unified target drivers. * This would reduce the workload for the storage target maintainers. * This would motivate the SCST target driver maintainers to contribute to the upstream target drivers and to bring the upstream SRP and FCoE target drivers to the same feature and stability level as their SCST counterparts. In other words, the LIO users would also benefit from this work. * This effort would also help SCST users by ensuring that all latest target driver features are also available to SCST users. Some time ago (but no longer today) the LIO QLogic target driver was ahead of the SCST QLogic target driver. This motivated an SCST user to port the LIO QLogic target driver to SCST. See also Greg Wettstein, New release of SCST/Qlogic target interface driver, linux-scsi, April 2014, http://marc.info/?l=linux-scsim=139649571807085). During the first phase of this initiative the focus will be on the QLogic FC, SRP and FCoE target drivers since a significant part of the code of these drivers is shared between the two target frameworks. As always, I'm open to discussion, but if the main selling point for unification is a bag of out-of-tree drivers that LIO already has vendor and enterprise distro support for in upstream code, the net result will not be to bring in more developers, vendors, and users to contribute to a single target-core subsystem. Contributing to a single target-core is after all, my main interest as target subsystem maintainer. If anything, this proposal gives certain vendors continued incentive to avoid supporting and improving upstream target-core. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 46/48] hpsa: detect and report failures changing controller transport modes
From: Robert Elliott elli...@hp.com Detect failues when attempting to change controller to use simple or performant transport modes (mode change ack) rather than just proceeding ahead after timeouts. Return values are added to: hpsa_put_ctlr_into_performant_mode hpsa_wait_for_mode_change_ack and all their callers check/propagate the result. More consistency in printing errors and whether dev_err is used. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 40 +--- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 89744a1..c1f4a95 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -243,7 +243,7 @@ static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id); static int hpsa_wait_for_board_state(struct pci_dev *pdev, void __iomem *vaddr, int wait_for_ready); static inline void finish_cmd(struct CommandList *c); -static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h); +static int hpsa_wait_for_mode_change_ack(struct ctlr_info *h); #define BOARD_NOT_READY 0 #define BOARD_READY 1 static void hpsa_drain_accel_commands(struct ctlr_info *h); @@ -6191,7 +6191,7 @@ static inline void hpsa_p600_dma_prefetch_quirk(struct ctlr_info *h) writel(dma_prefetch, h-vaddr + I2O_DMA1_CFG); } -static void hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) +static int hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) { int i; u32 doorbell_value; @@ -6202,13 +6202,16 @@ static void hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) doorbell_value = readl(h-vaddr + SA5_DOORBELL); spin_unlock_irqrestore(h-lock, flags); if (!(doorbell_value DOORBELL_CLEAR_EVENTS)) - break; + goto done; /* delay and try again */ msleep(CLEAR_EVENT_WAIT_INTERVAL); } + return -ENODEV; +done: + return 0; } -static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h) +static int hpsa_wait_for_mode_change_ack(struct ctlr_info *h) { int i; u32 doorbell_value; @@ -6223,12 +6226,16 @@ static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h) doorbell_value = readl(h-vaddr + SA5_DOORBELL); spin_unlock_irqrestore(h-lock, flags); if (!(doorbell_value CFGTBL_ChangeReq)) - break; + goto done; /* delay and try again */ msleep(MODE_CHANGE_WAIT_INTERVAL); } + return -ENODEV; +done: + return 0; } +/* return -ENODEV or other reason on error, 0 on success */ static int hpsa_enter_simple_mode(struct ctlr_info *h) { u32 trans_support; @@ -6243,7 +6250,8 @@ static int hpsa_enter_simple_mode(struct ctlr_info *h) writel(CFGTBL_Trans_Simple, (h-cfgtable-HostWrite.TransportRequest)); writel(0, h-cfgtable-HostWrite.command_pool_addr_hi); writel(CFGTBL_ChangeReq, h-vaddr + SA5_DOORBELL); - hpsa_wait_for_mode_change_ack(h); + if (hpsa_wait_for_mode_change_ack(h)) + goto error; print_cfg_table(h-pdev-dev, h-cfgtable); if (!(readl((h-cfgtable-TransportActive)) CFGTBL_Trans_Simple)) goto error; @@ -7144,7 +7152,8 @@ static void calc_bucket_map(int bucket[], int num_buckets, } } -static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) +/* return -ENODEV or other reason on error, 0 on success */ +static int hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) { int i; unsigned long register_value; @@ -7236,12 +7245,16 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) } } writel(CFGTBL_ChangeReq, h-vaddr + SA5_DOORBELL); - hpsa_wait_for_mode_change_ack(h); + if (hpsa_wait_for_mode_change_ack(h)) { + dev_err(h-pdev-dev, + performant mode problem - doorbell timeout\n); + return -ENODEV; + } register_value = readl((h-cfgtable-TransportActive)); if (!(register_value CFGTBL_Trans_Performant)) { dev_err(h-pdev-dev, performant mode problem - transport not active\n); - return; + return -ENODEV; } /* Change the access methods to the performant access methods */ h-access = access; @@ -7249,7 +7262,7 @@ static void hpsa_enter_performant_mode(struct ctlr_info *h, u32 trans_support) if (!((trans_support CFGTBL_Trans_io_accel1) || (trans_support CFGTBL_Trans_io_accel2))) - return; + return
[PATCH 47/48] hpsa: add in gen9 controller model names
Add in gen9 controller model names Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index c1f4a95..6568da5 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -168,24 +168,24 @@ static struct board_type products[] = { {0x1926103C, Smart Array P731m, SA5_access}, {0x1928103C, Smart Array P230i, SA5_access}, {0x1929103C, Smart Array P530, SA5_access}, - {0x21BD103C, Smart Array, SA5_access}, - {0x21BE103C, Smart Array, SA5_access}, - {0x21BF103C, Smart Array, SA5_access}, - {0x21C0103C, Smart Array, SA5_access}, + {0x21BD103C, Smart Array P244br, SA5_access}, + {0x21BE103C, Smart Array P741m, SA5_access}, + {0x21BF103C, Smart HBA H240ar, SA5_access}, + {0x21C0103C, Smart Array P440ar, SA5_access}, {0x21C1103C, Smart Array, SA5_access}, - {0x21C2103C, Smart Array, SA5_access}, - {0x21C3103C, Smart Array, SA5_access}, + {0x21C2103C, Smart Array P440, SA5_access}, + {0x21C3103C, Smart Array P441, SA5_access}, {0x21C4103C, Smart Array, SA5_access}, - {0x21C5103C, Smart Array, SA5_access}, - {0x21C6103C, Smart Array, SA5_access}, - {0x21C7103C, Smart Array, SA5_access}, - {0x21C8103C, Smart Array, SA5_access}, + {0x21C5103C, Smart Array P841, SA5_access}, + {0x21C6103C, Smart HBA H244br, SA5_access}, + {0x21C7103C, Smart HBA H240, SA5_access}, + {0x21C8103C, Smart HBA H241, SA5_access}, {0x21C9103C, Smart Array, SA5_access}, - {0x21CA103C, Smart Array, SA5_access}, - {0x21CB103C, Smart Array, SA5_access}, + {0x21CA103C, Smart Array P246br, SA5_access}, + {0x21CB103C, Smart Array P840, SA5_access}, {0x21CC103C, Smart Array, SA5_access}, {0x21CD103C, Smart Array, SA5_access}, - {0x21CE103C, Smart Array, SA5_access}, + {0x21CE103C, Smart HBA, SA5_access}, {0x00761590, HP Storage P1224 Array Controller, SA5_access}, {0x00871590, HP Storage P1224e Array Controller, SA5_access}, {0x007D1590, HP Storage P1228 Array Controller, SA5_access}, -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 45/48] hpsa: shorten the wait for the CISS doorbell mode change ack
From: Robert Elliott elli...@hp.com Shorten the wait for the CISS configuration table doorbell mode change acknowledgment from 300-600 s to 20 s, which is the value specified in the CISS specification that should be honored by all controllers. Wait using interruptible msleep() rather than uninterruptible usleep_range(), which triggers rt_sched timeout errors if the wait is long. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a92653a..89744a1 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -60,8 +60,11 @@ #define DRIVER_NAME HP HPSA Driver (v HPSA_DRIVER_VERSION ) #define HPSA hpsa -/* How long to wait (in milliseconds) for board to go into simple mode */ -#define MAX_CONFIG_WAIT 3 +/* How long to wait for CISS doorbell communication */ +#define CLEAR_EVENT_WAIT_INTERVAL 20 /* ms for each msleep() call */ +#define MODE_CHANGE_WAIT_INTERVAL 10 /* ms for each msleep() call */ +#define MAX_CLEAR_EVENT_WAIT 3 /* times 20 ms = 600 s */ +#define MAX_MODE_CHANGE_WAIT 2000 /* times 10 ms = 20 s */ #define MAX_IOCTL_CONFIG_WAIT 1000 /*define how many times we will try a command because of bus resets */ @@ -6194,14 +6197,14 @@ static void hpsa_wait_for_clear_event_notify_ack(struct ctlr_info *h) u32 doorbell_value; unsigned long flags; /* wait until the clear_event_notify bit 6 is cleared by controller. */ - for (i = 0; i MAX_CONFIG_WAIT; i++) { + for (i = 0; i MAX_CLEAR_EVENT_WAIT; i++) { spin_lock_irqsave(h-lock, flags); doorbell_value = readl(h-vaddr + SA5_DOORBELL); spin_unlock_irqrestore(h-lock, flags); if (!(doorbell_value DOORBELL_CLEAR_EVENTS)) break; /* delay and try again */ - msleep(20); + msleep(CLEAR_EVENT_WAIT_INTERVAL); } } @@ -6215,14 +6218,14 @@ static void hpsa_wait_for_mode_change_ack(struct ctlr_info *h) * (e.g.: hot replace a failed 144GB drive in a RAID 5 set right * as we enter this code.) */ - for (i = 0; i MAX_CONFIG_WAIT; i++) { + for (i = 0; i MAX_MODE_CHANGE_WAIT; i++) { spin_lock_irqsave(h-lock, flags); doorbell_value = readl(h-vaddr + SA5_DOORBELL); spin_unlock_irqrestore(h-lock, flags); if (!(doorbell_value CFGTBL_ChangeReq)) break; /* delay and try again */ - usleep_range(1, 2); + msleep(MODE_CHANGE_WAIT_INTERVAL); } } -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 43/48] hpsa: move SG descriptor set-up out of hpsa_scatter_gather()
From: Webb Scales web...@hp.com Move the code which sets up the SG descriptor out of hpsa_scatter_gather() and into a subroutine where it can be reused (in the next patch). The Ext field is now assigned unconditionally: this makes the refactor much simpler, but more importantly it removes a conditional operation from inside the loop. The case for which the conditional formerly tested is now executed (unconditionally) after the loop is exited. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Webb Scales web...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a23277d..7915dc4 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3236,6 +3236,17 @@ out: kfree(id_phys); } +static void hpsa_set_sg_descriptor(struct SGDescriptor *desc, + struct scatterlist *sg) +{ + u64 addr64 = (u64) sg_dma_address(sg); + unsigned int len = sg_dma_len(sg); + + desc-Addr = cpu_to_le64(addr64); + desc-Len = cpu_to_le32(len); + desc-Ext = 0; +} + /* * hpsa_scatter_gather takes a struct scsi_cmnd, (cmd), and does the pci * dma mapping and fills in the scatter gather entries of the @@ -3245,9 +3256,7 @@ static int hpsa_scatter_gather(struct ctlr_info *h, struct CommandList *cp, struct scsi_cmnd *cmd) { - unsigned int len; struct scatterlist *sg; - u64 addr64; int use_sg, i, sg_index, chained; struct SGDescriptor *curr_sg; @@ -3270,13 +3279,11 @@ static int hpsa_scatter_gather(struct ctlr_info *h, curr_sg = h-cmd_sg_list[cp-cmdindex]; sg_index = 0; } - addr64 = (u64) sg_dma_address(sg); - len = sg_dma_len(sg); - curr_sg-Addr = cpu_to_le64(addr64); - curr_sg-Len = cpu_to_le32(len); - curr_sg-Ext = cpu_to_le32(0); + hpsa_set_sg_descriptor(curr_sg, sg); curr_sg++; } + + /* Back the pointer up to the last entry and mark it as last. */ (--curr_sg)-Ext = cpu_to_le32(HPSA_SG_LAST); if (use_sg + chained h-maxSG) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 48/48] hpsa: add in P840ar controller model name
Add in P840ar model name for gen9 Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 6568da5..15ef65c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -172,7 +172,7 @@ static struct board_type products[] = { {0x21BE103C, Smart Array P741m, SA5_access}, {0x21BF103C, Smart HBA H240ar, SA5_access}, {0x21C0103C, Smart Array P440ar, SA5_access}, - {0x21C1103C, Smart Array, SA5_access}, + {0x21C1103C, Smart Array P840ar, SA5_access}, {0x21C2103C, Smart Array P440, SA5_access}, {0x21C3103C, Smart Array P441, SA5_access}, {0x21C4103C, Smart Array, SA5_access}, -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 42/48] hpsa: do not use function pointers in fast path command submission
From: Stephen Cameron stephenmcame...@gmail.com Performance tweak, avoid unnecessary function calls. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |8 +--- drivers/scsi/hpsa.h |5 + 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 18bcba9..a23277d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -821,19 +821,21 @@ static void dial_up_lockup_detection_on_fw_flash_complete(struct ctlr_info *h, static void enqueue_cmd_and_start_io(struct ctlr_info *h, struct CommandList *c) { + dial_down_lockup_detection_during_fw_flash(h, c); + atomic_inc(h-commands_outstanding); switch (c-cmd_type) { case CMD_IOACCEL1: set_ioaccel1_performant_mode(h, c); + writel(c-busaddr, h-vaddr + SA5_REQUEST_PORT_OFFSET); break; case CMD_IOACCEL2: set_ioaccel2_performant_mode(h, c); + writel(c-busaddr, h-vaddr + IOACCEL2_INBOUND_POSTQ_32); break; default: set_performant_mode(h, c); + h-access.submit_command(h, c); } - dial_down_lockup_detection_during_fw_flash(h, c); - atomic_inc(h-commands_outstanding); - h-access.submit_command(h, c); } static inline int is_hba_lunid(unsigned char scsi3addr[]) diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 239ecea..62c50c3 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -367,10 +367,7 @@ static void SA5_submit_command_no_read(struct ctlr_info *h, static void SA5_submit_command_ioaccel2(struct ctlr_info *h, struct CommandList *c) { - if (c-cmd_type == CMD_IOACCEL2) - writel(c-busaddr, h-vaddr + IOACCEL2_INBOUND_POSTQ_32); - else - writel(c-busaddr, h-vaddr + SA5_REQUEST_PORT_OFFSET); + writel(c-busaddr, h-vaddr + SA5_REQUEST_PORT_OFFSET); } /* -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [LSF/MM TOPIC] Unifying the LIO and SCST target drivers
On 1/14/15, 2:05 AM, Bart Van Assche bart.vanass...@sandisk.com wrote: The LIO and SCST SCSI target subsystems consist of the following components: * A core that processes SCSI commands and that provides common functionality like persistent reservations, LUN masking and an interface that allows configuration from user space. * Device handlers that allow this core to access SCSI devices, block devices and files uniformly as SCSI devices. * Target drivers that implement a storage protocol (iSCSI, FC, SRP, iSER, FCoE, ...) and that realize the SCSI request and response communication between the target system and an initiator system. A significant amount of code is shared between several LIO target drivers and the SCST target drivers that implement the same storage protocol. Since there are two sets of these drivers this means that each set has to be maintained, extended and tested separately. This means a lot of redundant work. The main difference between these two sets of drivers is the interface between the target drivers and the SCSI target core. Hence the proposal to discuss the unification of the API between SCSI target core and SCSI target drivers. Implementing a single unified API would have the following advantages: * A single set of target drivers works for both projects which means a reduction of the maintenance effort for those who maintain target drivers for target driver developers and target driver users. * This would increase the size of the user base for the unified target drivers. * This would reduce the workload for the storage target maintainers. * This would motivate the SCST target driver maintainers to contribute to the upstream target drivers and to bring the upstream SRP and FCoE target drivers to the same feature and stability level as their SCST counterparts. In other words, the LIO users would also benefit from this work. * This effort would also help SCST users by ensuring that all latest target driver features are also available to SCST users. Some time ago (but no longer today) the LIO QLogic target driver was ahead of the SCST QLogic target driver. This motivated an SCST user to port the LIO QLogic target driver to SCST. See also Greg Wettstein, New release of SCST/Qlogic target interface driver, linux-scsi, April 2014, http://marc.info/?l=linux-scsim=139649571807085). During the first phase of this initiative the focus will be on the QLogic FC, SRP and FCoE target drivers since a significant part of the code of these drivers is shared between the two target frameworks. For those who are not following the SCST project: I'm maintaining the SCST SRP and FCoE target drivers. Nic, in case it was not yet clear, you would be more than welcome during this session :-) Bart. QT +1. This would be a plus for Qlogic to have 2 stacks under a unify API. Test resource devlopment cycles are limited. A lot of cycles are loss in keeping patches in sync. Would like to listen in at LSF to hear the discussion. attachment: winmail.dat
Re: [PATCH] scsi: Avoid crashing if device uses DIX but adapter does not support it
Ewan == Ewan D Milne emi...@redhat.com writes: Ewan This can happen if a multipathed device uses DIX and another path Ewan is added via an adapter that does not support it. Multipath Ewan should not allow this path to be added, No it shouldn't :) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6d5c0b8..4f14f4a 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1143,7 +1143,17 @@ int scsi_init_io(struct scsi_cmnd *cmd) struct scsi_data_buffer *prot_sdb = cmd-prot_sdb; int ivecs, count; - BUG_ON(prot_sdb == NULL); + if (prot_sdb == NULL) { + /* +* This can happen if someone (e.g. multipath) +* queues a command to a device on an adapter +* that does not support T10 PI. s/T10 PI/DIX/ +*/ + WARN_ON_ONCE(1); + error = BLKPREP_KILL; + goto err_exit; + } + ivecs = blk_rq_count_integrity_sg(rq-q, rq-bio); if (scsi_alloc_sgtable(prot_sdb, ivecs, is_mq)) { Failing more gracefully is OK with me. Acked-by: Martin K. Petersen martin.peter...@oracle.com -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 28/48] hpsa: use per-controller work queue
From: Webb Scales web...@hp.com There is a possibility of deadlock if we use the system work queue for command resubmission since something in the queue may be depending on the I/O that gets resubmitted, and the resubmitted I/O will be behind the thing that depends on it in the queue. Using a driver specific, per-controller work queue avoids this. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Webb Scales web...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 16 +--- drivers/scsi/hpsa.h |1 + 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index c1166a5..dcacb29 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1651,7 +1651,7 @@ static void process_ioaccel2_completion(struct ctlr_info *h, retry_cmd: INIT_WORK(c-work, hpsa_command_resubmit_worker); - schedule_work_on(raw_smp_processor_id(), c-work); + queue_work_on(raw_smp_processor_id(), h-resubmit_wq, c-work); } static void complete_scsi_command(struct CommandList *cp) @@ -1722,7 +1722,8 @@ static void complete_scsi_command(struct CommandList *cp) if (ei-CommandStatus == CMD_IOACCEL_DISABLED) dev-offload_enabled = 0; INIT_WORK(cp-work, hpsa_command_resubmit_worker); - schedule_work_on(raw_smp_processor_id(), cp-work); + queue_work_on(raw_smp_processor_id(), + h-resubmit_wq, cp-work); return; } } @@ -6418,6 +6419,7 @@ static void fail_all_outstanding_cmds(struct ctlr_info *h) int i; struct CommandList *c = NULL; + flush_workqueue(h-resubmit_wq); /* ensure all cmds are fully built */ for (i = 0; i h-nr_cmds; i++) { if (!test_bit(i (BITS_PER_LONG - 1), h-cmd_pool_bits + (i / BITS_PER_LONG))) @@ -6653,6 +6655,12 @@ reinit_after_soft_reset: spin_lock_init(h-scan_lock); spin_lock_init(h-passthru_count_lock); + h-resubmit_wq = alloc_workqueue(hpsa, WQ_MEM_RECLAIM, 0); + if (!h-resubmit_wq) { + dev_err(h-pdev-dev, Failed to allocate work queue\n); + rc = -ENOMEM; + goto clean1; + } /* Allocate and clear per-cpu variable lockup_detected */ h-lockup_detected = alloc_percpu(u32); if (!h-lockup_detected) { @@ -6785,6 +6793,8 @@ clean2_and_free_irqs: hpsa_free_irqs(h); clean2: clean1: + if (h-resubmit_wq) + destroy_workqueue(h-resubmit_wq); if (h-lockup_detected) free_percpu(h-lockup_detected); kfree(h); @@ -6860,9 +6870,9 @@ static void hpsa_remove_one(struct pci_dev *pdev) h-remove_in_progress = 1; cancel_delayed_work(h-monitor_ctlr_work); spin_unlock_irqrestore(h-lock, flags); - hpsa_unregister_scsi(h);/* unhook from SCSI subsystem */ hpsa_shutdown(pdev); + destroy_workqueue(h-resubmit_wq); iounmap(h-vaddr); iounmap(h-transtable); iounmap(h-cfgtable); diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 06a3e81..a0f4268 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -236,6 +236,7 @@ struct ctlr_info { struct list_head offline_device_list; int acciopath_status; int raid_offload_debug; + struct workqueue_struct *resubmit_wq; }; struct offline_device_entry { -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 24/48] hpsa: do not queue commands internally in driver
By not doing maintaining a list of queued commands, we can eliminate some spin locking in the main i/o path and gain significant improvement in IOPS. Remove the queuing code and the code that calls it; remove now-unused interrupt code; remove DIRECT_LOOKUP_BIT. Now that the passthru commands share the same command pool as the main i/o path, and the total size of the pool is less than or equal to the number of commands that will fit in the hardware fifo, there is no need to check to see if we are exceeding the hardware fifo's depth. Reviewed-by: Scott Teel scott.t...@pmcs.com Reviewed-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 306 +-- drivers/scsi/hpsa.h | 17 --- drivers/scsi/hpsa_cmd.h | 10 -- 3 files changed, 38 insertions(+), 295 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 26e3e5b..70f07af 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -196,8 +196,6 @@ static int number_of_controllers; static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id); static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id); static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg); -static void lock_and_start_io(struct ctlr_info *h); -static void start_io(struct ctlr_info *h, unsigned long *flags); #ifdef CONFIG_COMPAT static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, @@ -689,13 +687,6 @@ static struct scsi_host_template hpsa_driver_template = { .no_write_same = 1, }; - -/* Enqueuing and dequeuing functions for cmdlists. */ -static inline void addQ(struct list_head *list, struct CommandList *c) -{ - list_add_tail(c-list, list); -} - static inline u32 next_command(struct ctlr_info *h, u8 q) { u32 a; @@ -829,8 +820,6 @@ static void dial_up_lockup_detection_on_fw_flash_complete(struct ctlr_info *h, static void enqueue_cmd_and_start_io(struct ctlr_info *h, struct CommandList *c) { - unsigned long flags; - switch (c-cmd_type) { case CMD_IOACCEL1: set_ioaccel1_performant_mode(h, c); @@ -842,18 +831,8 @@ static void enqueue_cmd_and_start_io(struct ctlr_info *h, set_performant_mode(h, c); } dial_down_lockup_detection_during_fw_flash(h, c); - spin_lock_irqsave(h-lock, flags); - addQ(h-reqQ, c); - h-Qdepth++; - start_io(h, flags); - spin_unlock_irqrestore(h-lock, flags); -} - -static inline void removeQ(struct CommandList *c) -{ - if (WARN_ON(list_empty(c-list))) - return; - list_del_init(c-list); + atomic_inc(h-commands_outstanding); + h-access.submit_command(h, c); } static inline int is_hba_lunid(unsigned char scsi3addr[]) @@ -3449,8 +3428,7 @@ static int hpsa_scsi_ioaccel2_queue_command(struct ctlr_info *h, set_encrypt_ioaccel2(h, c, cp); cp-scsi_nexus = cpu_to_le32(ioaccel_handle); - cp-Tag = cpu_to_le32(c-cmdindex DIRECT_LOOKUP_SHIFT | - DIRECT_LOOKUP_BIT); + cp-Tag = cpu_to_le32(c-cmdindex DIRECT_LOOKUP_SHIFT); memcpy(cp-cdb, cdb, sizeof(cp-cdb)); /* fill in sg elements */ @@ -3831,10 +3809,7 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, dev-scsi3addr); } -/* - * Running in struct Scsi_Host-host_lock less mode using LLD internal - * struct ctlr_info *h-lock w/ spin_lock_irqsave() protection. - */ +/* Running in struct Scsi_Host-host_lock less mode */ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) { struct ctlr_info *h; @@ -3898,8 +3873,7 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) c-Header.ReplyQueue = 0; /* unused in simple mode */ memcpy(c-Header.LUN.LunAddrBytes[0], scsi3addr[0], 8); - c-Header.tag = cpu_to_le64((c-cmdindex DIRECT_LOOKUP_SHIFT) | - DIRECT_LOOKUP_BIT); + c-Header.tag = cpu_to_le64((c-cmdindex DIRECT_LOOKUP_SHIFT)); /* Fill in the request block... */ @@ -4264,56 +4238,6 @@ static int hpsa_send_abort(struct ctlr_info *h, unsigned char *scsi3addr, return rc; } -/* - * hpsa_find_cmd_in_queue - * - * Used to determine whether a command (find) is still present - * in queue_head. Optionally excludes the last element of queue_head. - * - * This is used to avoid unnecessary aborts. Commands in h-reqQ have - * not yet been submitted, and so can be aborted by the driver without - * sending an abort to the hardware. - * - * Returns pointer to command if found in queue, NULL otherwise. - */ -static struct CommandList *hpsa_find_cmd_in_queue(struct ctlr_info *h, - struct scsi_cmnd *find, struct list_head *queue_head) -{ - unsigned long flags; - struct CommandList *c = NULL; /* ptr
[PATCH 29/48] hpsa: honor queue depth of physical devices
When using the ioaccel submission methods, requests destined for RAID volumes are sometimes diverted to physical devices. The OS has no or limited knowledge of these physical devices, so it is up to the driver to avoid pushing the device too hard. It is better to honor the physical device queue limit rather than making the device spew zillions of TASK SET FULL responses. This is so that hpsa based devices support /sys/block/sdNN/device/queue_type of simple, which lets the SCSI midlayer automatically adjust the queue_depth based on TASK SET FULL and GOOD status. Adjust the queue depth for a new device after it is created based on the maximum queue depths of the physical devices that constitute the device. This drops the maximum queue depth from .can_queue of 1024 to something like 174 for single-drive RAID-0, 348 for two-drive RAID-1, etc. It also adjusts for the ratio of data to parity drives. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Webb Scales web...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 318 +-- drivers/scsi/hpsa.h | 14 ++ drivers/scsi/hpsa_cmd.h | 148 ++ 3 files changed, 412 insertions(+), 68 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index dcacb29..60f5734 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -247,7 +247,7 @@ static void hpsa_drain_accel_commands(struct ctlr_info *h); static void hpsa_flush_cache(struct ctlr_info *h); static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info *h, struct CommandList *c, u32 ioaccel_handle, u8 *cdb, int cdb_len, - u8 *scsi3addr); + u8 *scsi3addr, struct hpsa_scsi_dev_t *phys_disk); static void hpsa_command_resubmit_worker(struct work_struct *work); static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) @@ -965,12 +965,24 @@ static void hpsa_scsi_update_entry(struct ctlr_info *h, int hostno, /* Raid level changed. */ h-dev[entry]-raid_level = new_entry-raid_level; - /* Raid offload parameters changed. */ + /* Raid offload parameters changed. Careful about the ordering. */ + if (new_entry-offload_config new_entry-offload_enabled) { + /* +* if drive is newly offload_enabled, we want to copy the +* raid map data first. If previously offload_enabled and +* offload_config were set, raid map data had better be +* the same as it was before. if raid map data is changed +* then it had better be the case that +* h-dev[entry]-offload_enabled is currently 0. +*/ + h-dev[entry]-raid_map = new_entry-raid_map; + h-dev[entry]-ioaccel_handle = new_entry-ioaccel_handle; + wmb(); /* ensure raid map updated prior to -offload_enabled */ + } h-dev[entry]-offload_config = new_entry-offload_config; - h-dev[entry]-offload_enabled = new_entry-offload_enabled; - h-dev[entry]-ioaccel_handle = new_entry-ioaccel_handle; h-dev[entry]-offload_to_mirror = new_entry-offload_to_mirror; - h-dev[entry]-raid_map = new_entry-raid_map; + h-dev[entry]-offload_enabled = new_entry-offload_enabled; + h-dev[entry]-queue_depth = new_entry-queue_depth; dev_info(h-pdev-dev, %s device c%db%dt%dl%d updated.\n, scsi_device_type(new_entry-devtype), hostno, new_entry-bus, @@ -1096,6 +1108,8 @@ static inline int device_updated(struct hpsa_scsi_dev_t *dev1, return 1; if (dev1-offload_enabled != dev2-offload_enabled) return 1; + if (dev1-queue_depth != dev2-queue_depth) + return 1; return 0; } @@ -1241,6 +1255,85 @@ static void hpsa_show_volume_status(struct ctlr_info *h, } } +/* + * Figure the list of physical drive pointers for a logical drive with + * raid offload configured. + */ +static void hpsa_figure_phys_disk_ptrs(struct ctlr_info *h, + struct hpsa_scsi_dev_t *dev[], int ndevices, + struct hpsa_scsi_dev_t *logical_drive) +{ + struct raid_map_data *map = logical_drive-raid_map; + struct raid_map_disk_data *dd = map-data[0]; + int i, j; + int total_disks_per_row = le16_to_cpu(map-data_disks_per_row) + + le16_to_cpu(map-metadata_disks_per_row); + int nraid_map_entries = le16_to_cpu(map-row_cnt) * + le16_to_cpu(map-layout_map_count) * + total_disks_per_row; + int nphys_disk = le16_to_cpu(map-layout_map_count) * + total_disks_per_row; + int qdepth; + + if (nraid_map_entries RAID_MAP_MAX_ENTRIES) + nraid_map_entries = RAID_MAP_MAX_ENTRIES; + + qdepth = 0; + for (i = 0; i
[PATCH 25/48] hpsa: do not request device rescan on every ioaccel path error
From: Stephen Cameron stephenmcame...@gmail.com The original reasoning behind doing this was faulty. An error of some sort would be encountered, accelerated i/o would be disabled for that logical drive, the command would be kicked back out to the SCSI midlayer for a retry, and since i/o accelerator mode was disabled, it would get retried down the RAID path. However, something needs to turn ioaccellerator mode back on, and this rescan request was what did that. However, it was racy, and extremely bad for performance to rescan all devices, so, don't do that. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 14 ++ drivers/scsi/hpsa.h |1 - 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 70f07af..94a82e3 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1637,21 +1637,19 @@ static void process_ioaccel2_completion(struct ctlr_info *h, c2-error_data.serv_response == IOACCEL2_SERV_RESPONSE_FAILURE) { dev-offload_enabled = 0; - h-drv_req_rescan = 1; /* schedule controller for a rescan */ cmd-result = DID_SOFT_ERROR 16; cmd_free(h, c); cmd-scsi_done(cmd); return; } raid_retry = handle_ioaccel_mode2_error(h, c, cmd, c2); - /* If error found, disable Smart Path, schedule a rescan, -* and force a retry on the standard path. + /* If error found, disable Smart Path, +* force a retry on the standard path. */ if (raid_retry) { dev_warn(h-pdev-dev, %s: Retrying on standard path.\n, HP SSD Smart Path); dev-offload_enabled = 0; /* Disable Smart Path */ - h-drv_req_rescan = 1;/* schedule controller rescan */ cmd-result = DID_SOFT_ERROR 16; } cmd_free(h, c); @@ -6478,9 +6476,6 @@ static void hpsa_ack_ctlr_events(struct ctlr_info *h) int i; char *event_type; - /* Clear the driver-requested rescan flag */ - h-drv_req_rescan = 0; - /* Ask the controller to clear the events we're handling. */ if ((h-transMethod (CFGTBL_Trans_io_accel1 | CFGTBL_Trans_io_accel2)) @@ -6526,9 +6521,6 @@ static void hpsa_ack_ctlr_events(struct ctlr_info *h) */ static int hpsa_ctlr_needs_rescan(struct ctlr_info *h) { - if (h-drv_req_rescan) - return 1; - if (!(h-fw_support MISC_FW_EVENT_NOTIFY)) return 0; @@ -6574,7 +6566,6 @@ static void hpsa_monitor_ctlr_worker(struct work_struct *work) if (hpsa_ctlr_needs_rescan(h) || hpsa_offline_devices_ready(h)) { scsi_host_get(h-scsi_host); - h-drv_req_rescan = 0; hpsa_ack_ctlr_events(h); hpsa_scan_start(h-scsi_host); scsi_host_put(h-scsi_host); @@ -6743,7 +6734,6 @@ reinit_after_soft_reset: /* Enable Accelerated IO path at driver layer */ h-acciopath_status = 1; - h-drv_req_rescan = 0; /* Turn the interrupts on so we can service requests */ h-access.set_intr_mask(h, HPSA_INTR_ON); diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index e7d7eaa..06a3e81 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -235,7 +235,6 @@ struct ctlr_info { spinlock_t offline_device_lock; struct list_head offline_device_list; int acciopath_status; - int drv_req_rescan; /* flag for driver to request rescan event */ int raid_offload_debug; }; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 23/48] hpsa: get rid of cmd_special_alloc and cmd_special_free
From: Stephen Cameron stephenmcame...@gmail.com We have commands reserved for internal use. This is laying the groundwork for removing the internal queue of commands from the driver so that the locks that protect that queue may be removed. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 106 ++- drivers/scsi/hpsa.h |2 - 2 files changed, 31 insertions(+), 77 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 99c32a0..26e3e5b 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -205,9 +205,7 @@ static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, #endif static void cmd_free(struct ctlr_info *h, struct CommandList *c); -static void cmd_special_free(struct ctlr_info *h, struct CommandList *c); static struct CommandList *cmd_alloc(struct ctlr_info *h); -static struct CommandList *cmd_special_alloc(struct ctlr_info *h); static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h, void *buff, size_t size, u16 page_code, unsigned char *scsi3addr, int cmd_type); @@ -2057,10 +2055,10 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_warn(h-pdev-dev, cmd_special_alloc returned NULL!\n); + dev_warn(h-pdev-dev, cmd_alloc returned NULL!\n); return -ENOMEM; } @@ -2076,7 +2074,7 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr, rc = -1; } out: - cmd_special_free(h, c); + cmd_free(h, c); return rc; } @@ -2088,10 +2086,9 @@ static int hpsa_bmic_ctrl_mode_sense(struct ctlr_info *h, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); - + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_warn(h-pdev-dev, cmd_special_alloc returned NULL!\n); + dev_warn(h-pdev-dev, cmd_alloc returned NULL!\n); return -ENOMEM; } @@ -2107,7 +2104,7 @@ static int hpsa_bmic_ctrl_mode_sense(struct ctlr_info *h, rc = -1; } out: - cmd_special_free(h, c); + cmd_free(h, c); return rc; } @@ -2118,10 +2115,10 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_warn(h-pdev-dev, cmd_special_alloc returned NULL!\n); + dev_warn(h-pdev-dev, cmd_alloc returned NULL!\n); return -ENOMEM; } @@ -2137,7 +2134,7 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr, hpsa_scsi_interpret_error(h, c); rc = -1; } - cmd_special_free(h, c); + cmd_free(h, c); return rc; } @@ -2245,26 +2242,26 @@ static int hpsa_get_raid_map(struct ctlr_info *h, struct CommandList *c; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) { - dev_warn(h-pdev-dev, cmd_special_alloc returned NULL!\n); + dev_warn(h-pdev-dev, cmd_alloc returned NULL!\n); return -ENOMEM; } if (fill_cmd(c, HPSA_GET_RAID_MAP, h, this_device-raid_map, sizeof(this_device-raid_map), 0, scsi3addr, TYPE_CMD)) { dev_warn(h-pdev-dev, Out of memory in hpsa_get_raid_map()\n); - cmd_special_free(h, c); + cmd_free(h, c); return -ENOMEM; } hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE); ei = c-err_info; if (ei-CommandStatus != 0 ei-CommandStatus != CMD_DATA_UNDERRUN) { hpsa_scsi_interpret_error(h, c); - cmd_special_free(h, c); + cmd_free(h, c); return -1; } - cmd_special_free(h, c); + cmd_free(h, c); /* @todo in the future, dynamically allocate RAID map memory */ if (le32_to_cpu(this_device-raid_map.structure_size) @@ -2384,9 +2381,9 @@ static int hpsa_scsi_do_report_luns(struct ctlr_info *h, int logical, unsigned char scsi3addr[8]; struct ErrorInfo *ei; - c = cmd_special_alloc(h); + c = cmd_alloc(h); if (c == NULL) {/* trouble... */ - dev_err(h-pdev-dev, cmd_special_alloc returned NULL!\n); + dev_err(h-pdev-dev, cmd_alloc returned NULL!\n);
[PATCH 22/48] hpsa: reserve some commands for use by driver
From: Stephen Cameron stephenmcame...@gmail.com We need to reserve some commands for device rescans, aborts, and the pass through ioctls, etc. so we cannot give them all to the scsi mid layer. This is in preparation for removing cmd_special_alloc and cmd_special_free so that we can stop queuing commands internally in the driver so that we can remove the locks thta protect the queue that we will no longer have. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |7 +-- drivers/scsi/hpsa.h |2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index bee24b2..99c32a0 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4064,11 +4064,14 @@ static int hpsa_register_scsi(struct ctlr_info *h) sh-max_cmd_len = MAX_COMMAND_SIZE; sh-max_lun = HPSA_MAX_LUN; sh-max_id = HPSA_MAX_LUN; - sh-can_queue = h-nr_cmds; + sh-can_queue = h-nr_cmds - + HPSA_CMDS_RESERVED_FOR_ABORTS - + HPSA_CMDS_RESERVED_FOR_DRIVER - + HPSA_MAX_CONCURRENT_PASSTHRUS; if (h-hba_mode_enabled) sh-cmd_per_lun = 7; else - sh-cmd_per_lun = h-nr_cmds; + sh-cmd_per_lun = sh-can_queue; sh-sg_tablesize = h-maxsgentries; h-scsi_host = sh; sh-hostdata[0] = (unsigned long) h; diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 8e06d9e..5ee6c6a 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -115,6 +115,8 @@ struct ctlr_info { void __iomem *vaddr; unsigned long paddr; int nr_cmds; /* Number of commands allowed on this controller */ +#define HPSA_CMDS_RESERVED_FOR_ABORTS 2 +#define HPSA_CMDS_RESERVED_FOR_DRIVER 1 struct CfgTable __iomem *cfgtable; int interrupts_enabled; int max_commands; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 27/48] hpsa: use workqueue to resubmit failed ioaccel commands
Instead of kicking the commands all the way back to the mid layer, use a work queue. This enables having a mechanism for the driver to be able to resubmit the commands down the normal raid path without turning off the ioaccel feature entirely whenever an error is encountered on the ioaccel path, and prevent excessive rescanning of devices. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 60 --- drivers/scsi/hpsa_cmd.h |1 + 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index cc3128f..c1166a5 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -248,6 +248,7 @@ static void hpsa_flush_cache(struct ctlr_info *h); static int hpsa_scsi_ioaccel_queue_command(struct ctlr_info *h, struct CommandList *c, u32 ioaccel_handle, u8 *cdb, int cdb_len, u8 *scsi3addr); +static void hpsa_command_resubmit_worker(struct work_struct *work); static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) { @@ -1619,7 +1620,6 @@ static void process_ioaccel2_completion(struct ctlr_info *h, struct hpsa_scsi_dev_t *dev) { struct io_accel2_cmd *c2 = h-ioaccel2_cmd_pool[c-cmdindex]; - int raid_retry = 0; /* check for good status */ if (likely(c2-error_data.serv_response == 0 @@ -1636,24 +1636,22 @@ static void process_ioaccel2_completion(struct ctlr_info *h, if (is_logical_dev_addr_mode(dev-scsi3addr) c2-error_data.serv_response == IOACCEL2_SERV_RESPONSE_FAILURE) { - dev-offload_enabled = 0; - cmd-result = DID_SOFT_ERROR 16; - cmd_free(h, c); - cmd-scsi_done(cmd); - return; - } - raid_retry = handle_ioaccel_mode2_error(h, c, cmd, c2); - /* If error found, disable Smart Path, -* force a retry on the standard path. -*/ - if (raid_retry) { - dev_warn(h-pdev-dev, %s: Retrying on standard path.\n, - HP SSD Smart Path); - dev-offload_enabled = 0; /* Disable Smart Path */ - cmd-result = DID_SOFT_ERROR 16; + if (c2-error_data.status == + IOACCEL2_STATUS_SR_IOACCEL_DISABLED) + dev-offload_enabled = 0; + goto retry_cmd; } + + if (handle_ioaccel_mode2_error(h, c, cmd, c2)) + goto retry_cmd; + cmd_free(h, c); cmd-scsi_done(cmd); + return; + +retry_cmd: + INIT_WORK(c-work, hpsa_command_resubmit_worker); + schedule_work_on(raw_smp_processor_id(), c-work); } static void complete_scsi_command(struct CommandList *cp) @@ -1723,9 +1721,8 @@ static void complete_scsi_command(struct CommandList *cp) if (is_logical_dev_addr_mode(dev-scsi3addr)) { if (ei-CommandStatus == CMD_IOACCEL_DISABLED) dev-offload_enabled = 0; - cmd-result = DID_SOFT_ERROR 16; - cmd_free(h, cp); - cmd-scsi_done(cmd); + INIT_WORK(cp-work, hpsa_command_resubmit_worker); + schedule_work_on(raw_smp_processor_id(), cp-work); return; } } @@ -3873,6 +3870,31 @@ static int hpsa_ciss_submit(struct ctlr_info *h, return 0; } +static void hpsa_command_resubmit_worker(struct work_struct *work) +{ + struct scsi_cmnd *cmd; + struct hpsa_scsi_dev_t *dev; + struct CommandList *c = + container_of(work, struct CommandList, work); + + cmd = c-scsi_cmd; + dev = cmd-device-hostdata; + if (!dev) { + cmd-result = DID_NO_CONNECT 16; + cmd-scsi_done(cmd); + return; + } + if (hpsa_ciss_submit(c-h, c, cmd, dev-scsi3addr)) { + /* +* If we get here, it means dma mapping failed. Try +* again via scsi mid layer, which will then get +* SCSI_MLQUEUE_HOST_BUSY. +*/ + cmd-result = DID_IMM_RETRY 16; + cmd-scsi_done(cmd); + } +} + /* Running in struct Scsi_Host-host_lock less mode */ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) { diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h index d78e666..3f2f0af 100644 --- a/drivers/scsi/hpsa_cmd.h +++ b/drivers/scsi/hpsa_cmd.h @@ -404,6 +404,7 @@ struct CommandList { long cmdindex; struct completion *waiting; void *scsi_cmd; + struct work_struct work; } __aligned(COMMANDLIST_ALIGNMENT); /* Max S/G elements in I/O accelerator command */ -- To unsubscribe from this list: send the line
[PATCH 26/48] hpsa: factor out hpsa_ciss_submit function
From: Stephen Cameron stephenmcame...@gmail.com Factor out the bottom part of the queuecommand function which is the part that builds commands for submitting down the normal' RAID stack path of a Smart Array. Need to factor this out to improve how commands that were initially sent down one of the ioaccellerated paths but which have some sort of error condition are retried down the normal path. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 126 +++ 1 file changed, 67 insertions(+), 59 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 94a82e3..cc3128f 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -2034,7 +2034,7 @@ static int hpsa_scsi_do_inquiry(struct ctlr_info *h, unsigned char *scsi3addr, c = cmd_alloc(h); - if (c == NULL) {/* trouble... */ + if (c == NULL) { dev_warn(h-pdev-dev, cmd_alloc returned NULL!\n); return -ENOMEM; } @@ -3807,68 +3807,14 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, dev-scsi3addr); } -/* Running in struct Scsi_Host-host_lock less mode */ -static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) +/* Submit commands down the normal RAID stack path */ +static int hpsa_ciss_submit(struct ctlr_info *h, + struct CommandList *c, struct scsi_cmnd *cmd, + unsigned char scsi3addr[]) { - struct ctlr_info *h; - struct hpsa_scsi_dev_t *dev; - unsigned char scsi3addr[8]; - struct CommandList *c; - int rc = 0; - - /* Get the ptr to our adapter structure out of cmd-host. */ - h = sdev_to_hba(cmd-device); - dev = cmd-device-hostdata; - if (!dev) { - cmd-result = DID_NO_CONNECT 16; - cmd-scsi_done(cmd); - return 0; - } - memcpy(scsi3addr, dev-scsi3addr, sizeof(scsi3addr)); - - if (unlikely(lockup_detected(h))) { - cmd-result = DID_ERROR 16; - cmd-scsi_done(cmd); - return 0; - } - c = cmd_alloc(h); - if (c == NULL) {/* trouble... */ - dev_err(h-pdev-dev, cmd_alloc returned NULL!\n); - return SCSI_MLQUEUE_HOST_BUSY; - } - - /* Fill in the command list header */ - /* save c in case we have to abort it */ cmd-host_scribble = (unsigned char *) c; - c-cmd_type = CMD_SCSI; c-scsi_cmd = cmd; - - /* Call alternate submit routine for I/O accelerated commands. -* Retries always go down the normal I/O path. -*/ - if (likely(cmd-retries == 0 - cmd-request-cmd_type == REQ_TYPE_FS - h-acciopath_status)) { - if (dev-offload_enabled) { - rc = hpsa_scsi_ioaccel_raid_map(h, c); - if (rc == 0) - return 0; /* Sent on ioaccel path */ - if (rc 0) { /* scsi_dma_map failed. */ - cmd_free(h, c); - return SCSI_MLQUEUE_HOST_BUSY; - } - } else if (dev-ioaccel_handle) { - rc = hpsa_scsi_ioaccel_direct_map(h, c); - if (rc == 0) - return 0; /* Sent on direct map path */ - if (rc 0) { /* scsi_dma_map failed. */ - cmd_free(h, c); - return SCSI_MLQUEUE_HOST_BUSY; - } - } - } - c-Header.ReplyQueue = 0; /* unused in simple mode */ memcpy(c-Header.LUN.LunAddrBytes[0], scsi3addr[0], 8); c-Header.tag = cpu_to_le64((c-cmdindex DIRECT_LOOKUP_SHIFT)); @@ -3927,6 +3873,68 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) return 0; } +/* Running in struct Scsi_Host-host_lock less mode */ +static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) +{ + struct ctlr_info *h; + struct hpsa_scsi_dev_t *dev; + unsigned char scsi3addr[8]; + struct CommandList *c; + int rc = 0; + + /* Get the ptr to our adapter structure out of cmd-host. */ + h = sdev_to_hba(cmd-device); + dev = cmd-device-hostdata; + if (!dev) { + cmd-result = DID_NO_CONNECT 16; + cmd-scsi_done(cmd); + return 0; + } + memcpy(scsi3addr, dev-scsi3addr, sizeof(scsi3addr)); + + if (unlikely(lockup_detected(h))) { + cmd-result = DID_ERROR 16; + cmd-scsi_done(cmd); + return 0; + } + c = cmd_alloc(h); + if (c == NULL) {/*
[PATCH 34/48] hpsa: do not check for msi(x) in interrupt_pending
From: Stephen Cameron stephenmcame...@gmail.com No need to check whether interrupt pending for MSI(X) and conversely, no need to check whether MSI(X) interrupts are being used when checking if interrupts are pending. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.h |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index aa6cb0b..239ecea 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -479,9 +479,6 @@ static bool SA5_performant_intr_pending(struct ctlr_info *h) if (!register_value) return false; - if (h-msi_vector || h-msix_vector) - return true; - /* Read outbound doorbell to flush */ register_value = readl(h-vaddr + SA5_OUTDB_STATUS); return register_value SA5_OUTDB_STATUS_PERF_BIT; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 31/48] hpsa: optimize cmd_alloc function by remembering last allocation
From: Robert Elliott elli...@hp.com Empirically, this improves performance slightly (~2% max IOPS) by allowing cmd_alloc to remember where it left off searching for free commands between calls instead of always starting its search at command 0. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |7 +-- drivers/scsi/hpsa.h |1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index c95a20c..72abcf3 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4649,9 +4649,10 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) union u64bit temp64; dma_addr_t cmd_dma_handle, err_dma_handle; int refcount; - unsigned long offset = 0; + unsigned long offset; - /* There is some *extremely* small but non-zero chance that that + /* +* There is some *extremely* small but non-zero chance that that * multiple threads could get in here, and one thread could * be scanning through the list of bits looking for a free * one, but the free ones are always behind him, and other @@ -4662,6 +4663,7 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) * infrequently as to be indistinguishable from never. */ + offset = h-last_allocation; /* benignly racy */ for (;;) { i = find_next_zero_bit(h-cmd_pool_bits, h-nr_cmds, offset); if (unlikely(i == h-nr_cmds)) { @@ -4679,6 +4681,7 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) h-cmd_pool_bits + (i / BITS_PER_LONG)); break; /* it's ours now. */ } + h-last_allocation = i; /* benignly racy */ /* Zero out all of commandlist except the last field, refcount */ memset(c, 0, offsetof(struct CommandList, refcount)); diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 679e4d2..981479a 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -133,6 +133,7 @@ struct ctlr_info { struct CfgTable __iomem *cfgtable; int interrupts_enabled; int max_commands; + int last_allocation; atomic_t commands_outstanding; # define PERF_MODE_INT0 # define DOORBELL_INT 1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 21/48] hpsa: avoid unneccesary calls to resource freeing functions
From: Robert Elliott elli...@hp.com If hpsa_allocate_cmd_pool failed, we were calling two functions unnecessarily: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); This didn't cause any problem, as those functions can tolerate being called when what they free hasn't been allocated (relevant pointers would be NULL) but it is potentially confusing. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Robert Elliott elli...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a66a50e..bee24b2 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6948,8 +6948,9 @@ reinit_after_soft_reset: dev_info(pdev-dev, %s: 0x%x at IRQ %d%s using DAC\n, h-devname, pdev-device, h-intr[h-intr_mode], dac ? : not); - if (hpsa_allocate_cmd_pool(h)) - goto clean4; + rc = hpsa_allocate_cmd_pool(h); + if (rc) + goto clean2_and_free_irqs; if (hpsa_allocate_sg_chain_blocks(h)) goto clean4; init_waitqueue_head(h-scan_wait_queue); @@ -7038,6 +7039,7 @@ reinit_after_soft_reset: clean4: hpsa_free_sg_chain_blocks(h); hpsa_free_cmd_pool(h); +clean2_and_free_irqs: hpsa_free_irqs(h); clean2: clean1: -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 30/48] hpsa: fix race between abort handler and main i/o path
From: Webb Scales web...@hp.com This means changing the allocator to reference count commands. The reference count is now the authoritative indicator of whether a command is allocated or not. The h-cmd_pool_bits bitmap is now only a heuristic hint to speed up the allocation process, it is no longer the authoritative record of allocated commands. Since we changed the command allocator to use reference counting as the authoritative indicator of whether a command is allocated, fail_all_outstanding_cmds needs to use the reference count not h-cmd_pool_bits for this purpose. Fix hpsa_drain_accel_commands to use the reference count as the authoritative indicator of whether a command is allocated instead of the h-cmd_pool_bits bitmap. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 109 +++ drivers/scsi/hpsa.h |2 + drivers/scsi/hpsa_cmd.h |1 3 files changed, 65 insertions(+), 47 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 60f5734..c95a20c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4552,6 +4552,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) char msg[256]; /* For debug messaging. */ int ml = 0; __le32 tagupper, taglower; + int refcount; /* Find the controller of the command to be aborted */ h = sdev_to_hba(sc-device); @@ -4580,9 +4581,13 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) /* Get SCSI command to be aborted */ abort = (struct CommandList *) sc-host_scribble; if (abort == NULL) { - dev_err(h-pdev-dev, %s FAILED, Command to abort is NULL.\n, - msg); - return FAILED; + /* This can happen if the command already completed. */ + return SUCCESS; + } + refcount = atomic_inc_return(abort-refcount); + if (refcount == 1) { /* Command is done already. */ + cmd_free(h, abort); + return SUCCESS; } hpsa_get_tag(h, abort, taglower, tagupper); ml += sprintf(msg+ml, Tag:0x%08x:%08x , tagupper, taglower); @@ -4604,6 +4609,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) dev_warn(h-pdev-dev, FAILED abort on device C%d:B%d:T%d:L%d\n, h-scsi_host-host_no, dev-bus, dev-target, dev-lun); + cmd_free(h, abort); return FAILED; } dev_info(h-pdev-dev, %s REQUEST SUCCEEDED.\n, msg); @@ -4615,32 +4621,35 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) */ #define ABORT_COMPLETE_WAIT_SECS 30 for (i = 0; i ABORT_COMPLETE_WAIT_SECS * 10; i++) { - if (test_bit(abort-cmdindex (BITS_PER_LONG - 1), - h-cmd_pool_bits + - (abort-cmdindex / BITS_PER_LONG))) - msleep(100); - else + refcount = atomic_read(abort-refcount); + if (refcount 2) { + cmd_free(h, abort); return SUCCESS; + } else { + msleep(100); + } } dev_warn(h-pdev-dev, %s FAILED. Aborted command has not completed after %d seconds.\n, msg, ABORT_COMPLETE_WAIT_SECS); + cmd_free(h, abort); return FAILED; } - /* * For operations that cannot sleep, a command block is allocated at init, * and managed by cmd_alloc() and cmd_free() using a simple bitmap to track * which ones are free or in use. Lock must be held when calling this. * cmd_free() is the complement. */ + static struct CommandList *cmd_alloc(struct ctlr_info *h) { struct CommandList *c; int i; union u64bit temp64; dma_addr_t cmd_dma_handle, err_dma_handle; - int loopcount; + int refcount; + unsigned long offset = 0; /* There is some *extremely* small but non-zero chance that that * multiple threads could get in here, and one thread could @@ -4653,23 +4662,27 @@ static struct CommandList *cmd_alloc(struct ctlr_info *h) * infrequently as to be indistinguishable from never. */ - loopcount = 0; - do { - i = find_first_zero_bit(h-cmd_pool_bits, h-nr_cmds); - if (i == h-nr_cmds) - i = 0; - loopcount++; - } while (test_and_set_bit(i (BITS_PER_LONG - 1), - h-cmd_pool_bits + (i / BITS_PER_LONG)) != 0 - loopcount 10); - - /* Thread got starved? We do not expect this to ever happen. */ - if (loopcount = 10) - return NULL; - - c = h-cmd_pool + i; - memset(c, 0, sizeof(*c)); - c-Header.tag = cpu_to_le64((u64) i
[PATCH 37/48] hpsa: guard against overflowing raid map array
From: Stephen Cameron stephenmcame...@gmail.com In the code that translates logical drive LBAs to physical drive LBAs if we overflow the raid map disk data array we will get the wrong answers. We do not expect that to happen, but best to be on the safe side and guard against it anyway. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index afd192d..03fae8a 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3921,6 +3921,9 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, return IO_ACCEL_INELIGIBLE; } + if (unlikely(map_index = RAID_MAP_MAX_ENTRIES)) + return IO_ACCEL_INELIGIBLE; + c-phys_disk = dev-phys_disk[map_index]; disk_handle = dd[map_index].ioaccel_handle; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 40/48] hpsa: do not use a void pointer for scsi_cmd field of struct CommandList
From: Stephen Cameron stephenmcame...@gmail.com There's no reason for it to be a void *, it should be a struct scsi_cmnd * Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |6 +++--- drivers/scsi/hpsa_cmd.h |2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index a02ea7f..8f2569c 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1764,7 +1764,7 @@ static void complete_scsi_command(struct CommandList *cp) unsigned long sense_data_size; ei = cp-err_info; - cmd = (struct scsi_cmnd *) cp-scsi_cmd; + cmd = cp-scsi_cmd; h = cp-h; dev = cmd-device-hostdata; @@ -4466,7 +4466,7 @@ static int hpsa_send_reset_as_abort_ioaccel2(struct ctlr_info *h, unsigned char *psa = phys_scsi3addr[0]; /* Get a pointer to the hpsa logical device. */ - scmd = (struct scsi_cmnd *) abort-scsi_cmd; + scmd = abort-scsi_cmd; dev = (struct hpsa_scsi_dev_t *)(scmd-device-hostdata); if (dev == NULL) { dev_warn(h-pdev-dev, @@ -4604,7 +4604,7 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) } hpsa_get_tag(h, abort, taglower, tagupper); ml += sprintf(msg+ml, Tag:0x%08x:%08x , tagupper, taglower); - as = (struct scsi_cmnd *) abort-scsi_cmd; + as = abort-scsi_cmd; if (as != NULL) ml += sprintf(msg+ml, Command:0x%x SN:0x%lx , as-cmnd[0], as-serial_number); diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h index 071b64c..3a621c7 100644 --- a/drivers/scsi/hpsa_cmd.h +++ b/drivers/scsi/hpsa_cmd.h @@ -408,7 +408,7 @@ struct CommandList { intcmd_type; long cmdindex; struct completion *waiting; - void *scsi_cmd; + struct scsi_cmnd *scsi_cmd; struct work_struct work; /* -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 41/48] hpsa: print CDBs instead of kernel virtual addresses for uncommon errors
From: Stephen Cameron stephenmcame...@gmail.com Printing the address of the command pointer is of little value, change to print the CDB. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 8f2569c..18bcba9 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -1882,9 +1882,8 @@ static void complete_scsi_command(struct CommandList *cp) case CMD_DATA_UNDERRUN: /* let mid layer handle it. */ break; case CMD_DATA_OVERRUN: - dev_warn(h-pdev-dev, cp %p has -completed with data overrun - reported\n, cp); + dev_warn(h-pdev-dev, + CDB %16phN data overrun\n, cp-Request.CDB); break; case CMD_INVALID: { /* print_bytes(cp, sizeof(*cp), 1, 0); @@ -1900,34 +1899,38 @@ static void complete_scsi_command(struct CommandList *cp) break; case CMD_PROTOCOL_ERR: cmd-result = DID_ERROR 16; - dev_warn(h-pdev-dev, cp %p has - protocol error\n, cp); + dev_warn(h-pdev-dev, CDB %16phN : protocol error\n, + cp-Request.CDB); break; case CMD_HARDWARE_ERR: cmd-result = DID_ERROR 16; - dev_warn(h-pdev-dev, cp %p had hardware error\n, cp); + dev_warn(h-pdev-dev, CDB %16phN : hardware error\n, + cp-Request.CDB); break; case CMD_CONNECTION_LOST: cmd-result = DID_ERROR 16; - dev_warn(h-pdev-dev, cp %p had connection lost\n, cp); + dev_warn(h-pdev-dev, CDB %16phN : connection lost\n, + cp-Request.CDB); break; case CMD_ABORTED: cmd-result = DID_ABORT 16; - dev_warn(h-pdev-dev, cp %p was aborted with status 0x%x\n, - cp, ei-ScsiStatus); + dev_warn(h-pdev-dev, CDB %16phN was aborted with status 0x%x\n, + cp-Request.CDB, ei-ScsiStatus); break; case CMD_ABORT_FAILED: cmd-result = DID_ERROR 16; - dev_warn(h-pdev-dev, cp %p reports abort failed\n, cp); + dev_warn(h-pdev-dev, CDB %16phN : abort failed\n, + cp-Request.CDB); break; case CMD_UNSOLICITED_ABORT: cmd-result = DID_SOFT_ERROR 16; /* retry the command */ - dev_warn(h-pdev-dev, cp %p aborted due to an unsolicited - abort\n, cp); + dev_warn(h-pdev-dev, CDB %16phN : unsolicited abort\n, + cp-Request.CDB); break; case CMD_TIMEOUT: cmd-result = DID_TIME_OUT 16; - dev_warn(h-pdev-dev, cp %p timedout\n, cp); + dev_warn(h-pdev-dev, CDB %16phN timed out\n, + cp-Request.CDB); break; case CMD_UNABORTABLE: cmd-result = DID_ERROR 16; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 38/48] hpsa: check for ctlr lockup after command allocation in main io path
From: Stephen Cameron stephenmcame...@gmail.com Command allocation is the thing that takes the longest in the main i/o path, so check for controller lockup immediately after this to prevent submitting commands to locked up controller as much as possible. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 03fae8a..834ac78 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4097,8 +4097,15 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd) dev_err(h-pdev-dev, cmd_alloc returned NULL!\n); return SCSI_MLQUEUE_HOST_BUSY; } + if (unlikely(lockup_detected(h))) { + cmd-result = DID_ERROR 16; + cmd_free(h, c); + cmd-scsi_done(cmd); + return 0; + } - /* Call alternate submit routine for I/O accelerated commands. + /* +* Call alternate submit routine for I/O accelerated commands. * Retries always go down the normal I/O path. */ if (likely(cmd-retries == 0 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 36/48] hpsa: do not ack controller events on controllers that do not support it
From: Stephen Cameron stephenmcame...@gmail.com Acking controller events on controllers that do not support it can cause such controllers to lock up. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Joe Handzik joseph.t.hand...@hp.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 5abd49d..afd192d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -6674,6 +6674,9 @@ static void hpsa_ack_ctlr_events(struct ctlr_info *h) int i; char *event_type; + if (!(h-fw_support MISC_FW_EVENT_NOTIFY)) + return; + /* Ask the controller to clear the events we're handling. */ if ((h-transMethod (CFGTBL_Trans_io_accel1 | CFGTBL_Trans_io_accel2)) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 35/48] hpsa: remove incorrect BUG_ONs checking for raid offload enable
From: Stephen Cameron stephenmcame...@gmail.com In set_encrypt_ioaccel2() and in hpsa_scsi_ioaccel_raid_map there were BUG_ONs that looked like this: BUG_ON(!(dev-offload_config dev-offload_enabled)); But, In hpsa_ack_ctlr_events() we have this, /* Stop sending new RAID offload reqs via the IO accelerator */ scsi_block_requests(h-scsi_host); for (i = 0; i h-ndevices; i++) h-dev[i]-offload_enabled = 0; hpsa_drain_accel_commands(h); So, we set offload_enabled = 0 for all drives, then do this drain_accel_commands, so that means accel commands could still be in flight, ie. perhaps having just been submitted into hpsa_scsi_ioaccel_raid_map concurrent with -offload_enabled having just been set to zero. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |4 1 file changed, 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index bae3759..5abd49d 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3456,8 +3456,6 @@ static void set_encrypt_ioaccel2(struct ctlr_info *h, struct raid_map_data *map = dev-raid_map; u64 first_block; - BUG_ON(!(dev-offload_config dev-offload_enabled)); - /* Are we doing encryption on this device */ if (!(le16_to_cpu(map-flags) RAID_MAP_FLAG_ENCRYPT_ON)) return; @@ -3688,8 +3686,6 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, #endif int offload_to_mirror; - BUG_ON(!(dev-offload_config dev-offload_enabled)); - /* check for valid opcode, get LBA and block count */ switch (cmd-cmnd[0]) { case WRITE_6: -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 33/48] hpsa: slightly optimize SA5_performant_completed
Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.h |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 1856445..aa6cb0b 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -412,19 +412,19 @@ static unsigned long SA5_performant_completed(struct ctlr_info *h, u8 q) unsigned long register_value = FIFO_EMPTY; /* msi auto clears the interrupt pending bit. */ - if (!(h-msi_vector || h-msix_vector)) { + if (unlikely(!(h-msi_vector || h-msix_vector))) { /* flush the controller write of the reply queue by reading * outbound doorbell status register. */ - register_value = readl(h-vaddr + SA5_OUTDB_STATUS); + (void) readl(h-vaddr + SA5_OUTDB_STATUS); writel(SA5_OUTDB_CLEAR_PERF_BIT, h-vaddr + SA5_OUTDB_CLEAR); /* Do a read in order to flush the write to the controller * (as per spec.) */ - register_value = readl(h-vaddr + SA5_OUTDB_STATUS); + (void) readl(h-vaddr + SA5_OUTDB_STATUS); } - if ((rq-head[rq-current_entry] 1) == rq-wraparound) { + if u32) rq-head[rq-current_entry]) 1) == rq-wraparound) { register_value = rq-head[rq-current_entry]; rq-current_entry++; atomic_dec(h-commands_outstanding); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 32/48] hpsa: count passthru cmds with atomics, not a spin locked int
Performance enhancement. Remove spin_locks from the driver. Reviewed-by: Scott Teel scott.t...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c | 39 +-- drivers/scsi/hpsa.h |3 +-- 2 files changed, 6 insertions(+), 36 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 72abcf3..bae3759 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -5105,35 +5105,6 @@ static void check_ioctl_unit_attention(struct ctlr_info *h, (void) check_for_unit_attention(h, c); } -static int increment_passthru_count(struct ctlr_info *h) -{ - unsigned long flags; - - spin_lock_irqsave(h-passthru_count_lock, flags); - if (h-passthru_count = HPSA_MAX_CONCURRENT_PASSTHRUS) { - spin_unlock_irqrestore(h-passthru_count_lock, flags); - return -1; - } - h-passthru_count++; - spin_unlock_irqrestore(h-passthru_count_lock, flags); - return 0; -} - -static void decrement_passthru_count(struct ctlr_info *h) -{ - unsigned long flags; - - spin_lock_irqsave(h-passthru_count_lock, flags); - if (h-passthru_count = 0) { - spin_unlock_irqrestore(h-passthru_count_lock, flags); - /* not expecting to get here. */ - dev_warn(h-pdev-dev, Bug detected, passthru_count seems to be incorrect.\n); - return; - } - h-passthru_count--; - spin_unlock_irqrestore(h-passthru_count_lock, flags); -} - /* * ioctl */ @@ -5156,16 +5127,16 @@ static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg) case CCISS_GETDRIVVER: return hpsa_getdrivver_ioctl(h, argp); case CCISS_PASSTHRU: - if (increment_passthru_count(h)) + if (atomic_dec_if_positive(h-passthru_cmds_avail) 0) return -EAGAIN; rc = hpsa_passthru_ioctl(h, argp); - decrement_passthru_count(h); + atomic_inc(h-passthru_cmds_avail); return rc; case CCISS_BIG_PASSTHRU: - if (increment_passthru_count(h)) + if (atomic_dec_if_positive(h-passthru_cmds_avail) 0) return -EAGAIN; rc = hpsa_big_passthru_ioctl(h, argp); - decrement_passthru_count(h); + atomic_inc(h-passthru_cmds_avail); return rc; default: return -ENOTTY; @@ -6852,7 +6823,7 @@ reinit_after_soft_reset: spin_lock_init(h-lock); spin_lock_init(h-offline_device_lock); spin_lock_init(h-scan_lock); - spin_lock_init(h-passthru_count_lock); + atomic_set(h-passthru_cmds_avail, HPSA_MAX_CONCURRENT_PASSTHRUS); h-resubmit_wq = alloc_workqueue(hpsa, WQ_MEM_RECLAIM, 0); if (!h-resubmit_wq) { diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h index 981479a..1856445 100644 --- a/drivers/scsi/hpsa.h +++ b/drivers/scsi/hpsa.h @@ -183,8 +183,7 @@ struct ctlr_info { /* cap concurrent passthrus at some reasonable maximum */ #define HPSA_MAX_CONCURRENT_PASSTHRUS (10) - spinlock_t passthru_count_lock; /* protects passthru_count */ - int passthru_count; + atomic_t passthru_cmds_avail; /* * Performant mode completion buffers -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 39/48] hpsa: return failed from device reset/abort handlers
Returning failed from the device reset handler will get the device kicked offline, which is fine if the controller is locked up anyhow. Cannot abort a command from a failed controller. Reviewed-by: Scott Teel scott.t...@pmcs.com Reviewed-by: Justin Lindley justin.lind...@pmcs.com Signed-off-by: Don Brace don.br...@pmcs.com --- drivers/scsi/hpsa.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 834ac78..a02ea7f 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -4343,6 +4343,10 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd) h = sdev_to_hba(scsicmd-device); if (h == NULL) /* paranoia */ return FAILED; + + if (lockup_detected(h)) + return FAILED; + dev = scsicmd-device-hostdata; if (!dev) { dev_err(h-pdev-dev, hpsa_eh_device_reset_handler: @@ -4566,6 +4570,9 @@ static int hpsa_eh_abort_handler(struct scsi_cmnd *sc) ABORT REQUEST FAILED, Controller lookup failed.\n)) return FAILED; + if (lockup_detected(h)) + return FAILED; + /* Check that controller supports some kind of task abort */ if (!(HPSATMF_PHYS_TASK_ABORT h-TMFSupportFlags) !(HPSATMF_LOG_TASK_ABORT h-TMFSupportFlags)) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH for v3.19, v2] Avoid that sd_shutdown() triggers a kernel warning
On Wed, 14 Jan 2015, Christoph Hellwig wrote: On Mon, Jan 12, 2015 at 11:29:15AM -0500, Alan Stern wrote: This seems like a good idea and the obvious (once it has been pointed out!) approach. Perhaps not directly related to the issue at hand is this question: In scsi_rescan_device() we will now have: mutex_lock(shost-scan_mutex); if (dev-driver try_module_get(dev-driver-owner)) { struct scsi_driver *drv = to_scsi_driver(dev-driver); if (drv-rescan) drv-rescan(dev); module_put(dev-driver-owner); } mutex_unlock(shost-scan_mutex); What prevents the device from being unbound from its driver while the rescan runs? Evaluating the argument to the module_put() would then dereference a NULL pointer. Unbind events that happen through the normal scsi_remove_host() mechanism are fine, because scsi_remove_host() locks the scan_mutex. But what about writes to the driver's sysfs unbind attribute? Looks like we should still get an unconditional reference to the device using get_device in scsi_rescan_device at least. I'm not sure that's necessary. scsi_rescan_device is exposed by sysfs, and the kernfs core insures that the underlying device won't be deallocated while a sysfs method runs. (scsi_rescan_device is also EXPORTed, so in theory it could be called under less safe circumstances. Perhaps then the burden should fall on the caller to guarantee that the device won't be deallocated.) But this seems like a more generic problem, and at least a quick glance at the pci_driver methods seems like others don't have a good synchroniation of -remove against random driver methods. Can you give one or two examples? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] scsi: use external buffer for command logging
On Wed, 2015-01-14 at 10:36 +0100, h...@lst.de wrote: On Tue, Jan 13, 2015 at 06:56:17PM +, James Bottomley wrote: I really hate using an on-stack buffer here ... we're pretty deep in the call chain already. Since it's just required for printing a command: prefix, why not just use scsi_print_command()? Both the ch and sr callers are not really a problem in terms of stack usage, as they are directly in file operations of character or block devices, so the whole FS/MM stack stack problem isn't an issue. Trying to calculate minutely how much additional overhead there is isn't really the point. The point is we can just use a different function that doesn't consume the on stack buffer, so we should, just as a matter of best practise. I agree it would have been a more finely balanced problem if we definitely needed the allocation But using higher level functions might still be useful here. James
Re: [linux-devel:devel-lkp-ib03-powerpc-201501140043 30/31] drivers/scsi/scsi_logging.c:254:3: error: format not a string literal and no format arguments
On Wed, 2015-01-14 at 01:40 -0800, Christoph Hellwig wrote: On Tue, Jan 13, 2015 at 05:30:20PM +, James Bottomley wrote: Just for everyone's sake the problem is printk format strings (and all the things that indirect there, like pr_xxx and dev_printk). We must never pass a mutable string directly to printk because of the mayhem that would result if its contents were altered by the user (because some of the things we do in string format parsing are very dangerous), making this a potential security issue. Only ever pass static strings (in the ro section) to printk formats. So this is wrong: dev_printk(KERN_INFO, dev, logbuf); This is correct: dev_printk(KERN_INFO, dev, %s, logbuf); In this case the logbug comes from actually doing just that string formatting earlier in the function, so it practice it's harmles. However, it's a wrong pattern which we need to avoid. Otherwise we get one patch every few months fixing it and a couple of annoyed emails from security people who re-did the analysis. It would be useful to have a dev_puts to avoid that reinterpretation again, though. I'm fairly certain, given a lot of what has gone on in our string processors that dev_puts() would get implemented via dev_printk ... James N�r��yb�X��ǧv�^�){.n�+{{ay�ʇڙ�,j��f���h���z��w��� ���j:+v���w�j�mzZ+�ݢj��!�i
[PATCH RFC] qla2xxx: Fix qla27xx_write_reg()
The second argument passed to qla27xx_write_reg() is a byte offset. This means that this function must read data from (void *)reg + offset instead of (void *)reg + sizeof(*reg) * offset. Found this via source reading. Untested. --- drivers/scsi/qla2xxx/qla_tmpl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/qla2xxx/qla_tmpl.c b/drivers/scsi/qla2xxx/qla_tmpl.c index a8c0c73..7a92f60 100644 --- a/drivers/scsi/qla2xxx/qla_tmpl.c +++ b/drivers/scsi/qla2xxx/qla_tmpl.c @@ -190,7 +190,7 @@ static inline void qla27xx_write_reg(__iomem struct device_reg_24xx *reg, uint offset, uint32_t data, void *buf) { - __iomem void *window = reg + offset; + __iomem void *window = (void __iomem *)reg + offset; if (buf) { WRT_REG_DWORD(window, data); -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/16] virtio/scsi: verify device has config space
Some devices might not implement config space access (e.g. remoteproc used not to - before 3.9). virtio/scsi needs config space access so make it fail gracefully if not there. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- drivers/scsi/virtio_scsi.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index c52bb5d..f164f24 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -950,6 +950,12 @@ static int virtscsi_probe(struct virtio_device *vdev) u32 num_queues; struct scsi_host_template *hostt; + if (!vdev-config-get) { + dev_err(vdev-dev, %s failure: config access disabled\n, + __func__); + return -EINVAL; + } + /* We need to know how many queues before we allocate. */ num_queues = virtscsi_config_get(vdev, num_queues) ? : 1; -- MST -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dm + blk-mq soft lockup complaint
On 01/13/15 17:21, Mike Snitzer wrote: OK, I assume you specified the mpath device for the test that failed. Yes, of course ... This test works fine on my 100MB scsi_debug device with 4 paths exported over virtio-blk to a guest that assembles the mpath device. Could be a hang that is unique to scsi-mq. Any chance you'd be willing to provide a HOWTO for setting up your SRP/iscsi configuration? Are you carrying any related changes that are not upstream? (I can hunt down the email in this thread where you describe your kernel tree...) I'll try to reproduce but this info could be useful to others that are more scsi-mq inclined who might need to chase this too. The four patches I had used in my tests at the initiator side and that are not yet in v3.19-rc4 have been attached to this e-mail (I have not yet had the time to post all of these patches for review). This is how my I had configured the initiator system: * If the version of the srptools package supplied by your distro is lower than 1.0.2, build and install the latest version from the source code available at git://git.openfabrics.org/~bvanassche/srptools.git/.git. * Install the latest version of lsscsi (http://sg.danny.cz/scsi/lsscsi.html). This version has SRP transport support but is not yet in any distro AFAIK. * Build and install a kernel = v3.19-rc4 that includes the dm patches at the start of this e-mail thread. * Check whether the IB links are up (should display State: Active): ibstat | grep State: * Spread completion interrupts statically over CPU cores, e.g. via the attached script (spread-mlx4-ib-interrupts). * Check whether the SRP target system is visible from the SRP initiator system - the command below should print at least one line: ibsrpdm -c * Enable blk-mq: echo Y /sys/module/scsi_mod/parameters/use_blk_mq * Configure the SRP kernel module parameters as follows: echo 'options ib_srp cmd_sg_entries=255 dev_loss_tmo=60 ch_count=6' /etc/modprobe.d/ib_srp.conf * Unload and reload the SRP initiator kernel module to apply these parameters: rmmod ib_srp; modprobe ib_srp * Start srpd and wait until SRP login has finished: systemctl start srpd while ! lsscsi -t | grep -q srp:; do sleep 1; done * Start multipathd and check the table it has built: systemctl start multipathd dmsetup table /dev/dm-0 * Set the I/O scheduler to noop, disable add_random and set rq_affinity to 2 for all SRP and dm block devices. * Run the I/O load of your preference. Please let me know if you need any further information. Bart. From 664b7adce6c09b9c939b4983f7f32b7539497ef4 Mon Sep 17 00:00:00 2001 From: Bart Van Assche bvanass...@acm.org Date: Fri, 2 Jan 2015 14:52:07 +0100 Subject: [PATCH 1/4] e1000: Avoid that e1000_netpoll() triggers a kernel warning console_cont_flush(), which is called by console_unlock(), calls call_console_drivers() and hence also the netconsole function write_msg() with local interrupts disabled. This means that it is not allowed to call disable_irq() from inside a netpoll callback function. Hence eliminate the disable_irq() / enable_irq() pair from the e1000 netpoll function. This patch avoids that the e1000 networking driver triggers the following complaint: BUG: sleeping function called from invalid context at kernel/irq/manage.c:104 Call Trace: [814d1ec5] dump_stack+0x4c/0x65 [8107bcc5] ___might_sleep+0x175/0x230 [8107bdba] __might_sleep+0x3a/0xa0 [810a78c8] synchronize_irq+0x38/0xa0 [810a7a20] disable_irq+0x20/0x30 [a04b4442] e1000_netpoll+0x102/0x130 [e1000e] [8132] netpoll_poll_dev+0x72/0x350 [81400489] netpoll_send_skb_on_dev+0x1b9/0x2b0 [81400842] netpoll_send_udp+0x2c2/0x430 [a058187f] write_msg+0xcf/0x120 [netconsole] [810a4682] call_console_drivers.constprop.25+0xc2/0x250 [810a5588] console_unlock+0x328/0x4c0 [810a59f0] vprintk_emit+0x2d0/0x570 [810a5def] vprintk_default+0x1f/0x30 [814cf680] printk+0x46/0x48 See also [RFC PATCH net-next 00/11] net: remove disable_irq() from -ndo_poll_controller (http://thread.gmane.org/gmane.linux.network/342096). See also patch sched/wait: Add might_sleep() checks (kernel v3.19-rc1; commit e22b886a8a43). Reported-by: Sabrina Dubroca s...@queasysnail.net Signed-off-by: Bart Van Assche bvanass...@acm.org Cc: Thomas Gleixner t...@linutronix.de Cc: David S. Miller da...@davemloft.net Cc: Peter Zijlstra pet...@infradead.org --- drivers/net/ethernet/intel/e1000/e1000.h | 5 + drivers/net/ethernet/intel/e1000/e1000_main.c | 27 ++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h index 6970710..d85d19f 100644 --- a/drivers/net/ethernet/intel/e1000/e1000.h +++ b/drivers/net/ethernet/intel/e1000/e1000.h @@ -323,6 +323,11 @@ struct e1000_adapter { struct delayed_work watchdog_task; struct delayed_work
Re: [PATCH for v3.19, v2] Avoid that sd_shutdown() triggers a kernel warning
On Mon, Jan 12, 2015 at 11:29:15AM -0500, Alan Stern wrote: This seems like a good idea and the obvious (once it has been pointed out!) approach. Perhaps not directly related to the issue at hand is this question: In scsi_rescan_device() we will now have: mutex_lock(shost-scan_mutex); if (dev-driver try_module_get(dev-driver-owner)) { struct scsi_driver *drv = to_scsi_driver(dev-driver); if (drv-rescan) drv-rescan(dev); module_put(dev-driver-owner); } mutex_unlock(shost-scan_mutex); What prevents the device from being unbound from its driver while the rescan runs? Evaluating the argument to the module_put() would then dereference a NULL pointer. Unbind events that happen through the normal scsi_remove_host() mechanism are fine, because scsi_remove_host() locks the scan_mutex. But what about writes to the driver's sysfs unbind attribute? Looks like we should still get an unconditional reference to the device using get_device in scsi_rescan_device at least. But this seems like a more generic problem, and at least a quick glance at the pci_driver methods seems like others don't have a good synchroniation of -remove against random driver methods. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] scsi: use external buffer for command logging
On Tue, Jan 13, 2015 at 06:56:17PM +, James Bottomley wrote: I really hate using an on-stack buffer here ... we're pretty deep in the call chain already. Since it's just required for printing a command: prefix, why not just use scsi_print_command()? Both the ch and sr callers are not really a problem in terms of stack usage, as they are directly in file operations of character or block devices, so the whole FS/MM stack stack problem isn't an issue. But using higher level functions might still be useful here. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-devel:devel-lkp-ib03-powerpc-201501140043 30/31] drivers/scsi/scsi_logging.c:254:3: error: format not a string literal and no format arguments
On Tue, Jan 13, 2015 at 05:30:20PM +, James Bottomley wrote: Just for everyone's sake the problem is printk format strings (and all the things that indirect there, like pr_xxx and dev_printk). We must never pass a mutable string directly to printk because of the mayhem that would result if its contents were altered by the user (because some of the things we do in string format parsing are very dangerous), making this a potential security issue. Only ever pass static strings (in the ro section) to printk formats. So this is wrong: dev_printk(KERN_INFO, dev, logbuf); This is correct: dev_printk(KERN_INFO, dev, %s, logbuf); In this case the logbug comes from actually doing just that string formatting earlier in the function, so it practice it's harmles. It would be useful to have a dev_puts to avoid that reinterpretation again, though. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 09/10] megaraid_sas: swap whole register in megasas_register_aen
-Original Message- From: Christoph Hellwig [mailto:h...@lst.de] Sent: Saturday, January 10, 2015 10:41 PM To: sumit.sax...@avagotech.com; kashyap.de...@avagotech.com Cc: martin.peter...@oracle.com; linux-scsi@vger.kernel.org Subject: [PATCH 09/10] megaraid_sas: swap whole register in megasas_register_aen Swap the whole 32 bits we read from the hardware instead of swapping just the 16bits we care about in place later. Signed-off-by: Christoph Hellwig h...@lst.de --- drivers/scsi/megaraid/megaraid_sas_base.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 5350e18..3d4a080 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -4882,8 +4882,9 @@ megasas_register_aen(struct megasas_instance *instance, u32 seq_num, if (instance-aen_cmd) { - prev_aen.word = instance-aen_cmd-frame- dcmd.mbox.w[1]; - prev_aen.members.locale = le16_to_cpu(prev_aen.members.locale); + prev_aen.word = + le32_to_cpu(instance-aen_cmd-frame- dcmd.mbox.w[1]); + prev_aen.members.locale = prev_aen.members.locale; /* * A class whose enum value is smaller is inclusive of all Looks good to me. Acked-by: Sumit Saxena sumit.sax...@avagotech.com -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 08/10] megaraid_sas: fix megasas_fire_cmd_fusion calling convention
-Original Message- From: Christoph Hellwig [mailto:h...@lst.de] Sent: Saturday, January 10, 2015 10:41 PM To: sumit.sax...@avagotech.com; kashyap.de...@avagotech.com Cc: martin.peter...@oracle.com; linux-scsi@vger.kernel.org Subject: [PATCH 08/10] megaraid_sas: fix megasas_fire_cmd_fusion calling convention The fusion HBAs don't really use the instance template like the other variants, as it branches off at a much higher level. So instead of trying to squeeze megasas_fire_cmd_fusion into the wrong calling convention call it locally with argument data types that match what is passed. Signed-off-by: Christoph Hellwig h...@lst.de --- drivers/scsi/megaraid/megaraid_sas_fusion.c | 73 - 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index b1e053b..5a45764 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -230,6 +230,31 @@ inline void megasas_return_mfi_mpt_pthr(struct megasas_instance *instance, } /** + * megasas_fire_cmd_fusion - Sends command to the FW + */ +static void +megasas_fire_cmd_fusion(struct megasas_instance *instance, + union MEGASAS_REQUEST_DESCRIPTOR_UNION *req_desc) { #if +defined(writeq) defined(CONFIG_64BIT) + u64 req_data = (((u64)le32_to_cpu(req_desc-u.low) 32) | + le32_to_cpu(req_desc-u.high)); In req_data building, positions of req_desc-u.low and req_desc-u.high are swapped. req_data should be like below: u64 req_data = (((u64)le32_to_cpu(req_desc-u.high) 32) | le32_to_cpu(req_desc-u.low)); I will post new patch with this change. Thanks, Sumit + + writeq(req_data, instance-reg_set-inbound_low_queue_port); +#else + unsigned long flags; + + spin_lock_irqsave(instance-hba_lock, flags); + writel(le32_to_cpu(req_desc-u.low), + instance-reg_set-inbound_low_queue_port); + writel(le32_to_cpu(req_desc.u.high), + instance-reg_set-inbound_high_queue_port); + spin_unlock_irqrestore(instance-hba_lock, flags); #endif } + + +/** * megasas_teardown_frame_pool_fusion - Destroy the cmd frame DMA pool * @instance: Adapter soft state */ @@ -721,8 +746,7 @@ megasas_ioc_init_fusion(struct megasas_instance *instance) break; } - instance-instancet-fire_cmd(instance, req_desc.u.low, -req_desc.u.high, instance-reg_set); + megasas_fire_cmd_fusion(instance, req_desc); wait_and_poll(instance, cmd, MFI_POLL_TIMEOUT_SECS); @@ -1133,34 +1157,6 @@ fail_alloc_mfi_cmds: } /** - * megasas_fire_cmd_fusion - Sends command to the FW - * @frame_phys_addr : Physical address of cmd - * @frame_count : Number of frames for the command - * @regs :MFI register set - */ -void -megasas_fire_cmd_fusion(struct megasas_instance *instance, - dma_addr_t req_desc_lo, - u32 req_desc_hi, - struct megasas_register_set __iomem *regs) -{ -#if defined(writeq) defined(CONFIG_64BIT) - u64 req_data = (((u64)le32_to_cpu(req_desc_hi) 32) | - le32_to_cpu(req_desc_lo)); - - writeq(req_data, (regs)-inbound_low_queue_port); -#else - unsigned long flags; - - spin_lock_irqsave(instance-hba_lock, flags); - - writel(le32_to_cpu(req_desc_lo), (regs)- inbound_low_queue_port); - writel(le32_to_cpu(req_desc_hi), (regs)- inbound_high_queue_port); - spin_unlock_irqrestore(instance-hba_lock, flags); -#endif -} - -/** * map_cmd_status - Maps FW cmd status to OS cmd status * @cmd : Pointer to cmd * @status : status of cmd returned by FW @@ -1947,9 +1943,7 @@ megasas_build_and_issue_cmd_fusion(struct megasas_instance *instance, */ atomic_inc(instance-fw_outstanding); - instance-instancet-fire_cmd(instance, -req_desc-u.low, req_desc-u.high, -instance-reg_set); + megasas_fire_cmd_fusion(instance, req_desc); return 0; } @@ -2328,8 +2322,7 @@ megasas_issue_dcmd_fusion(struct megasas_instance *instance, return; } atomic_set(cmd-mfi_mpt_pthr, MFI_MPT_ATTACHED); - instance-instancet-fire_cmd(instance, req_desc-u.low, -req_desc-u.high, instance-reg_set); + megasas_fire_cmd_fusion(instance, req_desc); } /** @@ -2816,14 +2809,7 @@ int megasas_reset_fusion(struct Scsi_Host *shost, int iotimeout) frame */ megasas_return_cmd_fusion(instance, cmd_fusion); } else { - instance-instancet- -
RE: [PATCH 10/10] megaraid_sas: fix endianess for the crash dump state support
-Original Message- From: Christoph Hellwig [mailto:h...@lst.de] Sent: Saturday, January 10, 2015 10:41 PM To: sumit.sax...@avagotech.com; kashyap.de...@avagotech.com Cc: martin.peter...@oracle.com; linux-scsi@vger.kernel.org Subject: [PATCH 10/10] megaraid_sas: fix endianess for the crash dump state support Note that this one is a bit fishy: it seems like it should always be read and written using either words or bytes, but never using a mixture. Please verify against the hardware documentation. Signed-off-by: Christoph Hellwig h...@lst.de --- drivers/scsi/megaraid/megaraid_sas.h | 2 +- drivers/scsi/megaraid/megaraid_sas_base.c | 7 --- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 81484f1..465dd45 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -1676,7 +1676,7 @@ struct megasas_instance { u32 drv_buf_alloc; u32 crash_dump_fw_support; u32 crash_dump_drv_support; - u32 crash_dump_app_support; + __le32 crash_dump_app_support; u32 secure_jbod_support; spinlock_t crashdump_lock; diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 3d4a080..29a1b20 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -6010,7 +6010,7 @@ static int megasas_set_crash_dump_params_ioctl( { struct megasas_instance *local_instance; int i, error = 0; - int crash_support; + __le32 crash_support; crash_support = cmd-frame-dcmd.mbox.w[0]; @@ -6020,7 +6020,7 @@ static int megasas_set_crash_dump_params_ioctl( if ((local_instance-adprecovery == MEGASAS_HBA_OPERATIONAL) !megasas_set_crash_dump_params(local_instance, - crash_support)) { + le32_to_cpu(crash_support))) { local_instance-crash_dump_app_support = crash_support; dev_info(local_instance-pdev-dev, @@ -6084,7 +6084,8 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance, MFI_FRAME_SGL64 | MFI_FRAME_SENSE64)); - if (cmd-frame-dcmd.opcode == MR_DRIVER_SET_APP_CRASHDUMP_MODE) { + if (cmd-frame-dcmd.opcode == + cpu_to_le32(MR_DRIVER_SET_APP_CRASHDUMP_MODE)) { Application will not do any byte swapping for this DCMD frame(MR_DRIVER_SET_APP_CRASHDUMP_MODE) to convert it in LE format, since this DCMD will not be sent to firmware, driver will only process this DCMD and fire another DCMD- MR_DCMD_CTRL_SET_CRASH_DUMP_PARAMS to firmware. DCMD frame will always be in CPU format only. So this if condition does not need cpu_to_le32(MR_DRIVER_SET_APP_CRASHDUMP_MODE). error = megasas_set_crash_dump_params_ioctl(cmd); megasas_return_cmd(instance, cmd); return error; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[LSF/MM TOPIC] How SRP initiator performance has been improved via scsi-mq
Recently it has been proposed to discuss how to add scsi-mq support in the iSER initiator during the 2015 edition of the LSF/MM summit. In the discussion that followed that proposal several questions were asked about how scsi-mq support has been added in the SRP initiator driver, which design choices have been made and which lessons have been learned. Hence the proposal to add a topic to the LSF/MM schedule for reporting how scsi-mq support has been added in the SRP initiator. For the actual discussion see also Sagi Grimberg, [LSF/MM TOPIC] iSCSI MQ adoption via MCS discussion, linux-scsi mailing list, January 7, 2015 (http://thread.gmane.org/gmane.linux.scsi/98199). Christoph and Mike, if you would like to complement this topic with a presentation about the scsi-mq core itself and/or integration of multiqueue support in the device mapper multipath driver, I would welcome this. Bart. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[LSF/MM TOPIC] Unifying the LIO and SCST target drivers
The LIO and SCST SCSI target subsystems consist of the following components: * A core that processes SCSI commands and that provides common functionality like persistent reservations, LUN masking and an interface that allows configuration from user space. * Device handlers that allow this core to access SCSI devices, block devices and files uniformly as SCSI devices. * Target drivers that implement a storage protocol (iSCSI, FC, SRP, iSER, FCoE, ...) and that realize the SCSI request and response communication between the target system and an initiator system. A significant amount of code is shared between several LIO target drivers and the SCST target drivers that implement the same storage protocol. Since there are two sets of these drivers this means that each set has to be maintained, extended and tested separately. This means a lot of redundant work. The main difference between these two sets of drivers is the interface between the target drivers and the SCSI target core. Hence the proposal to discuss the unification of the API between SCSI target core and SCSI target drivers. Implementing a single unified API would have the following advantages: * A single set of target drivers works for both projects which means a reduction of the maintenance effort for those who maintain target drivers for target driver developers and target driver users. * This would increase the size of the user base for the unified target drivers. * This would reduce the workload for the storage target maintainers. * This would motivate the SCST target driver maintainers to contribute to the upstream target drivers and to bring the upstream SRP and FCoE target drivers to the same feature and stability level as their SCST counterparts. In other words, the LIO users would also benefit from this work. * This effort would also help SCST users by ensuring that all latest target driver features are also available to SCST users. Some time ago (but no longer today) the LIO QLogic target driver was ahead of the SCST QLogic target driver. This motivated an SCST user to port the LIO QLogic target driver to SCST. See also Greg Wettstein, New release of SCST/Qlogic target interface driver, linux-scsi, April 2014, http://marc.info/?l=linux-scsim=139649571807085). During the first phase of this initiative the focus will be on the QLogic FC, SRP and FCoE target drivers since a significant part of the code of these drivers is shared between the two target frameworks. For those who are not following the SCST project: I'm maintaining the SCST SRP and FCoE target drivers. Nic, in case it was not yet clear, you would be more than welcome during this session :-) Bart. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [LSF/MM TOPIC] Unifying the LIO and SCST target drivers
On 01/14/2015 11:05 AM, Bart Van Assche wrote: The LIO and SCST SCSI target subsystems consist of the following components: * A core that processes SCSI commands and that provides common functionality like persistent reservations, LUN masking and an interface that allows configuration from user space. * Device handlers that allow this core to access SCSI devices, block devices and files uniformly as SCSI devices. * Target drivers that implement a storage protocol (iSCSI, FC, SRP, iSER, FCoE, ...) and that realize the SCSI request and response communication between the target system and an initiator system. A significant amount of code is shared between several LIO target drivers and the SCST target drivers that implement the same storage protocol. Since there are two sets of these drivers this means that each set has to be maintained, extended and tested separately. This means a lot of redundant work. The main difference between these two sets of drivers is the interface between the target drivers and the SCSI target core. Hence the proposal to discuss the unification of the API between SCSI target core and SCSI target drivers. Implementing a single unified API would have the following advantages: * A single set of target drivers works for both projects which means a reduction of the maintenance effort for those who maintain target drivers for target driver developers and target driver users. * This would increase the size of the user base for the unified target drivers. * This would reduce the workload for the storage target maintainers. * This would motivate the SCST target driver maintainers to contribute to the upstream target drivers and to bring the upstream SRP and FCoE target drivers to the same feature and stability level as their SCST counterparts. In other words, the LIO users would also benefit from this work. * This effort would also help SCST users by ensuring that all latest target driver features are also available to SCST users. Some time ago (but no longer today) the LIO QLogic target driver was ahead of the SCST QLogic target driver. This motivated an SCST user to port the LIO QLogic target driver to SCST. See also Greg Wettstein, New release of SCST/Qlogic target interface driver, linux-scsi, April 2014, http://marc.info/?l=linux-scsim=139649571807085). During the first phase of this initiative the focus will be on the QLogic FC, SRP and FCoE target drivers since a significant part of the code of these drivers is shared between the two target frameworks. For those who are not following the SCST project: I'm maintaining the SCST SRP and FCoE target drivers. Nic, in case it was not yet clear, you would be more than welcome during this session :-) I'd like to have this discussion, too. It would be really good if we can make that work; after all, Linux should be about _choice_. So if both parties agree to have this discussion I'm all for it. I can even act as a moderator if required :-) Cheers, Hannes -- Dr. Hannes ReineckezSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: remove scsi_to_u32
On 01/10/15 11:10, Christoph Hellwig wrote: scsi_to_u32 is a non-optimized reimplementation of get_unaligned_be32, just use the real thing (TM). Reviewed-by: Bart Van Assche bart.vanass...@sandisk.com -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 07/10] megaraid_sas: add missing byte swaps to the sriov code
-Original Message- From: Christoph Hellwig [mailto:h...@lst.de] Sent: Saturday, January 10, 2015 10:41 PM To: sumit.sax...@avagotech.com; kashyap.de...@avagotech.com Cc: martin.peter...@oracle.com; linux-scsi@vger.kernel.org Subject: [PATCH 07/10] megaraid_sas: add missing byte swaps to the sriov code Signed-off-by: Christoph Hellwig h...@lst.de --- drivers/scsi/megaraid/megaraid_sas_base.c | 46 +--- --- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 548d033..5350e18 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -1924,20 +1924,22 @@ static int megasas_get_ld_vf_affiliation_111(struct megasas_instance *instance, dcmd-cmd = MFI_CMD_DCMD; dcmd-cmd_status = 0xFF; dcmd-sge_count = 1; - dcmd-flags = MFI_FRAME_DIR_BOTH; + dcmd-flags = cpu_to_le16(MFI_FRAME_DIR_BOTH); dcmd-timeout = 0; dcmd-pad_0 = 0; - dcmd-data_xfer_len = sizeof(struct MR_LD_VF_AFFILIATION_111); - dcmd-opcode = MR_DCMD_LD_VF_MAP_GET_ALL_LDS_111; + dcmd-data_xfer_len = + cpu_to_le32(sizeof(struct MR_LD_VF_AFFILIATION_111)); + dcmd-opcode = cpu_to_le32(MR_DCMD_LD_VF_MAP_GET_ALL_LDS_111); if (initial) dcmd-sgl.sge32[0].phys_addr = - instance-vf_affiliation_111_h; + cpu_to_le32(instance-vf_affiliation_111_h); else - dcmd-sgl.sge32[0].phys_addr = new_affiliation_111_h; + dcmd-sgl.sge32[0].phys_addr = + cpu_to_le32(new_affiliation_111_h); - dcmd-sgl.sge32[0].length = - sizeof(struct MR_LD_VF_AFFILIATION_111); + dcmd-sgl.sge32[0].length = cpu_to_le32( + sizeof(struct MR_LD_VF_AFFILIATION_111)); printk(KERN_WARNING megasas: SR-IOV: Getting LD/VF affiliation for scsi%d\n, instance-host-host_no); @@ -2039,20 +2041,22 @@ static int megasas_get_ld_vf_affiliation_12(struct megasas_instance *instance, dcmd-cmd = MFI_CMD_DCMD; dcmd-cmd_status = 0xFF; dcmd-sge_count = 1; - dcmd-flags = MFI_FRAME_DIR_BOTH; + dcmd-flags = cpu_to_le16(MFI_FRAME_DIR_BOTH); dcmd-timeout = 0; dcmd-pad_0 = 0; - dcmd-data_xfer_len = (MAX_LOGICAL_DRIVES + 1) * - sizeof(struct MR_LD_VF_AFFILIATION); - dcmd-opcode = MR_DCMD_LD_VF_MAP_GET_ALL_LDS; + dcmd-data_xfer_len = cpu_to_le32((MAX_LOGICAL_DRIVES + 1) * + sizeof(struct MR_LD_VF_AFFILIATION)); + dcmd-opcode = cpu_to_le32(MR_DCMD_LD_VF_MAP_GET_ALL_LDS); if (initial) - dcmd-sgl.sge32[0].phys_addr = instance-vf_affiliation_h; + dcmd-sgl.sge32[0].phys_addr = + cpu_to_le32(instance-vf_affiliation_h); else - dcmd-sgl.sge32[0].phys_addr = new_affiliation_h; + dcmd-sgl.sge32[0].phys_addr = + cpu_to_le32(new_affiliation_h);; - dcmd-sgl.sge32[0].length = (MAX_LOGICAL_DRIVES + 1) * - sizeof(struct MR_LD_VF_AFFILIATION); + dcmd-sgl.sge32[0].length = cpu_to_le32((MAX_LOGICAL_DRIVES + 1) * + sizeof(struct MR_LD_VF_AFFILIATION)); printk(KERN_WARNING megasas: SR-IOV: Getting LD/VF affiliation for scsi%d\n, instance-host-host_no); @@ -2204,17 +2208,17 @@ int megasas_sriov_start_heartbeat(struct megasas_instance *instance, memset(dcmd-mbox.b, 0, MFI_MBOX_SIZE); - dcmd-mbox.s[0] = sizeof(struct MR_CTRL_HB_HOST_MEM); + dcmd-mbox.s[0] = cpu_to_le16(sizeof(struct MR_CTRL_HB_HOST_MEM)); dcmd-cmd = MFI_CMD_DCMD; dcmd-cmd_status = 0xFF; dcmd-sge_count = 1; - dcmd-flags = MFI_FRAME_DIR_BOTH; + dcmd-flags = cpu_to_le16(MFI_FRAME_DIR_BOTH); dcmd-timeout = 0; dcmd-pad_0 = 0; - dcmd-data_xfer_len = sizeof(struct MR_CTRL_HB_HOST_MEM); - dcmd-opcode = MR_DCMD_CTRL_SHARED_HOST_MEM_ALLOC; - dcmd-sgl.sge32[0].phys_addr = instance-hb_host_mem_h; - dcmd-sgl.sge32[0].length = sizeof(struct MR_CTRL_HB_HOST_MEM); + dcmd-data_xfer_len = cpu_to_le32(sizeof(struct MR_CTRL_HB_HOST_MEM)); + dcmd-opcode = cpu_to_le32(MR_DCMD_CTRL_SHARED_HOST_MEM_ALLOC); + dcmd-sgl.sge32[0].phys_addr = cpu_to_le32(instance- hb_host_mem_h); + dcmd-sgl.sge32[0].length = cpu_to_le32(sizeof(struct +MR_CTRL_HB_HOST_MEM)); printk(KERN_WARNING megasas: SR-IOV: Starting heartbeat for scsi%d\n, instance-host-host_no); Acked-by: Sumit Saxena sumit.sax...@avagotech.com -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 06/10] megaraid_sas: bytewise or should be done on native endian variables
-Original Message- From: Christoph Hellwig [mailto:h...@lst.de] Sent: Saturday, January 10, 2015 10:41 PM To: sumit.sax...@avagotech.com; kashyap.de...@avagotech.com Cc: martin.peter...@oracle.com; linux-scsi@vger.kernel.org Subject: [PATCH 06/10] megaraid_sas: bytewise or should be done on native endian variables Signed-off-by: Christoph Hellwig h...@lst.de --- drivers/scsi/megaraid/megaraid_sas_fusion.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index 82439f9..b1e053b 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -673,7 +673,9 @@ megasas_ioc_init_fusion(struct megasas_instance *instance) frame_hdr = cmd-frame-hdr; frame_hdr-cmd_status = 0xFF; - frame_hdr-flags |= cpu_to_le16(MFI_FRAME_DONT_POST_IN_REPLY_QUEUE); + frame_hdr-flags = cpu_to_le16( + le16_to_cpu(frame_hdr-flags) | + MFI_FRAME_DONT_POST_IN_REPLY_QUEUE); init_frame-cmd = MFI_CMD_INIT; init_frame-cmd_status = 0xFF; Acked-by: Sumit Saxena sumit.sax...@avagotech.com -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html