Re: Re: Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print_command trace point instead of printk
Hi Hannes,
I tried to remove duplicated decoder of SCSI command, but the
output format of it in constants.c is different from it in traceevents.
I have two questions for it.
(Ex1)
traceevents: TEST_UNIT_READY
constants: Test Unit Ready
= Which of XXX_YYY_ZZZ and Xxx Yyy Zzz should the kernel output
strings? This difference comes from difference of implementation.
The decoder in traceevents are using macro. So, it cannot define
separated words. On the other hand, the decoder in constants are using
constant string array table. So, it can define separated words.
(Ex2)
traceevents: (nothing)
constants: Set limits(12)
= Should we merge those decoder?
I understand we use the decoder of constants, but we need to solve
these problems. Would you give me your comments?
Thanks,
Yoshihiro YUNOMAE
(2014/08/28 10:40), Yoshihiro YUNOMAE wrote:
(2014/08/27 23:16), Hannes Reinecke wrote:
On 08/08/2014 01:50 PM, Yoshihiro YUNOMAE wrote:
Previous printk messages of SCSI command can be mixed into other printk
messages because multiple printk messages are output for it. To avoid
the
problem, patch 4e64bb8d6 in Hannes' branch(*1) introduced a local
buffer.
But using local buffers can induce stack overflow, so we want to solve
the
problem without local buffer if possible.
trace_seq_printf can add log messages without local buffer, so we use
it.
Note:
We don't need constans.c any more.
(*1)
http://git.kernel.org/cgit/linux/kernel/git/hare/scsi-devel.git/log/?h=logging
- Result examples
Before (printk)
sd 2:0:0:0: [sda] CDB: Read(10)
After
scsi_print_command: host_no=2 channel=0 id=0 lun=0 [sda] CDB (Read(10))
Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
Cc: Hannes Reinecke h...@suse.de
Cc: Doug Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: Christoph Hellwig h...@lst.de
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Hidehiro Kawai hidehiro.kawai...@hitachi.com
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
---
drivers/scsi/Makefile |2
drivers/scsi/constants.c| 425
---
drivers/scsi/scsi_trace.c | 408
+
include/scsi/scsi.h |8 +
include/trace/events/scsi.h | 45 +
5 files changed, 461 insertions(+), 427 deletions(-)
delete mode 100644 drivers/scsi/constants.c
diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 5f0d299..c56f692 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -158,7 +158,7 @@ obj-$(CONFIG_SCSI_OSD_INITIATOR) += osd/
# This goes last, so that real scsi devices probe earlier
obj-$(CONFIG_SCSI_DEBUG)+= scsi_debug.o
-scsi_mod-y+= scsi.o hosts.o scsi_ioctl.o constants.o \
+scsi_mod-y+= scsi.o hosts.o scsi_ioctl.o \
scsicam.o scsi_error.o scsi_lib.o
scsi_mod-$(CONFIG_SCSI_DMA)+= scsi_lib_dma.o
scsi_mod-y+= scsi_scan.o scsi_sysfs.o scsi_devinfo.o
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
deleted file mode 100644
index ce9ceb8..000
--- a/drivers/scsi/constants.c
+++ /dev/null
@@ -1,425 +0,0 @@
-/*
- * ASCII values for a number of symbolic constants, printing functions,
- * etc.
- * Additions for SCSI 2 and Linux 2.2.x by D. Gilbert (990422)
- * Additions for SCSI 3+ (SPC-3 T10/1416-D Rev 07 3 May 2002)
- * by D. Gilbert and aeb (20020609)
- * Updated to SPC-4 T10/1713-D Rev 36g, D. Gilbert 20130701
- */
-
-#include linux/blkdev.h
-#include linux/module.h
-#include linux/kernel.h
-#include asm/unaligned.h
-
-#include scsi/scsi.h
-#include scsi/scsi_cmnd.h
-
-/* Commands with service actions that change the command name */
-#define SERVICE_ACTION_IN_12 0xab
-#define SERVICE_ACTION_OUT_12 0xa9
-#define SERVICE_ACTION_BIDIRECTIONAL 0x9d
-#define SERVICE_ACTION_IN_16 0x9e
-#define SERVICE_ACTION_OUT_16 0x9f
-#define THIRD_PARTY_COPY_OUT 0x83
-#define THIRD_PARTY_COPY_IN 0x84
-
-
-
-#ifdef CONFIG_SCSI_CONSTANTS
-static const char * cdb_byte0_names[] = {
-/* 00-03 */ Test Unit Ready, Rezero Unit/Rewind, NULL, Request
Sense,
-/* 04-07 */ Format Unit/Medium, Read Block Limits, NULL,
-Reassign Blocks,
-/* 08-0d */ Read(6), NULL, Write(6), Seek(6), NULL, NULL,
-/* 0e-12 */ NULL, Read Reverse, Write Filemarks, Space,
Inquiry,
-/* 13-16 */ Verify(6), Recover Buffered Data, Mode Select(6),
-Reserve(6),
-/* 17-1a */ Release(6), Copy, Erase, Mode Sense(6),
-/* 1b-1d */ Start/Stop Unit, Receive Diagnostic, Send Diagnostic,
-/* 1e-1f */ Prevent/Allow Medium Removal, NULL,
-/* 20-22 */ NULL, NULL, NULL,
-/* 23-28 */ Read Format Capacities, Set Window,
-Read Capacity(10), NULL, NULL, Read(10),
-/* 29-2d */ Read Generation, Write(10), Seek(10), Erase(10),
-Read updated block,
-/* 2e-31 */ Write Verify(10), Verify(10), Search High, Search
Equal,
-/* 32-34 */ Search Low, Set Limits, Prefetch/Read Position,
-/* 35-37 */ Synchronize Cache(10), Lock/Unlock
Re: [PATCH] fusion: fix excess parameter kernel-doc warning
On 08/27/14 17:55, Christoph Hellwig wrote: On Wed, Aug 27, 2014 at 04:47:24PM -0700, Randy Dunlap wrote: Goes with commit c9834c70efbaaa1461ec04289d97a842244fb294. Reviewed-by: Ewan D. Milne emi...@redhat.com Christoph, did you pick up this patch or should I merge it with my documentation patches? I did as mentioned in my reply to your other patch. Thanks. Sorry I missed that... -- ~Randy -- 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 v3 16/17] arcmsr: support new adapter ARC12x4 series
On Wed, 2014-08-27 at 16:00 +0200, Tomas Henzl wrote:
On 08/19/2014 09:25 AM, Ching Huang wrote:
From: Ching Huang ching2...@areca.com.tw
Add code for supporting Areca new Raid adapter ARC12x4 series.
Signed-off-by: Ching Huang ching2...@areca.com.tw
---
Hi Ching,
please look at the comments below -
}
@@ -1039,7 +1147,60 @@ static void arcmsr_done4abort_postqueue(
error = (flag_ccb ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ?
true : false;
arcmsr_drain_donequeue(acb, pCCB, error);
}
- }
+ }
+ break;
+ case ACB_ADAPTER_TYPE_D: {
+ struct MessageUnit_D *pmu = acb-pmuD;
+ uint32_t ccb_cdb_phy, outbound_write_pointer;
+ uint32_t doneq_index, index_stripped, addressLow, residual;
+ bool error;
+ struct CommandControlBlock *pCCB;
I have noticed this^ in this driver already before. Sometimes it makes sense
when a variable is declared in a 'case' block but often it is just
a waste of space. In this function this is the third 'bool error' declared.
Is there a reason for this style (this function is not the worst case) ?
Yea, there is many variable are duplicate declaration, I will clean it up.
+ outbound_write_pointer = pmu-done_qbuffer[0].addressLow + 1;
+ doneq_index = pmu-doneq_index;
+ residual = atomic_read(acb-ccboutstandingcount);
+ for (i = 0; i residual; i++) {
+ while ((doneq_index 0xFFF) !=
+ (outbound_write_pointer 0xFFF)) {
+ if (doneq_index 0x4000) {
+ index_stripped = doneq_index 0xFFF;
+ index_stripped += 1;
+ index_stripped %=
+ ARCMSR_MAX_ARC1214_DONEQUEUE;
+ pmu-doneq_index = index_stripped ?
+ (index_stripped | 0x4000) :
+ (index_stripped + 1);
+ } else {
+ index_stripped = doneq_index;
+ index_stripped += 1;
+ index_stripped %=
+ ARCMSR_MAX_ARC1214_DONEQUEUE;
+ pmu-doneq_index = index_stripped ?
+ index_stripped :
+ ((index_stripped | 0x4000) + 1);
+ }
+ doneq_index = pmu-doneq_index;
+ addressLow = pmu-done_qbuffer[doneq_index
+ 0xFFF].addressLow;
+ ccb_cdb_phy = (addressLow 0xFFF0);
+ pARCMSR_CDB = (struct ARCMSR_CDB *)
+ (acb-vir2phy_offset + ccb_cdb_phy);
+ pCCB = container_of(pARCMSR_CDB,
+ struct CommandControlBlock, arcmsr_cdb);
+ error = (addressLow
+ ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ?
+ true : false;
+ arcmsr_drain_donequeue(acb, pCCB, error);
+ writel(doneq_index,
pmu-outboundlist_read_pointer);
+ }
+ mdelay(10);
+ outbound_write_pointer =
+ pmu-done_qbuffer[0].addressLow + 1;
+ doneq_index = pmu-doneq_index;
+ }
+ pmu-postq_index = 0;
+ pmu-doneq_index = 0x40FF;
+ }
+ break;
}
}
...
@@ -1256,6 +1424,38 @@ static void arcmsr_post_ccb(struct Adapt
writel(ccb_post_stamp, phbcmu-inbound_queueport_low);
}
}
+ break;
+ case ACB_ADAPTER_TYPE_D: {
+ struct MessageUnit_D *pmu = acb-pmuD;
+ u16 index_stripped;
+ u16 postq_index;
u16 postq_index, toggle;
+ unsigned long flags;
+ struct InBound_SRB *pinbound_srb;
+
+ spin_lock_irqsave(acb-postq_lock, flags);
+ postq_index = pmu-postq_index;
+ pinbound_srb = (struct InBound_SRB
*)(pmu-post_qbuffer[postq_index 0xFF]);
+ pinbound_srb-addressHigh = dma_addr_hi32(cdb_phyaddr);
+ pinbound_srb-addressLow = dma_addr_lo32(cdb_phyaddr);
+ pinbound_srb-length = ccb-arc_cdb_size 2;
+ arcmsr_cdb-msgContext = dma_addr_lo32(cdb_phyaddr);
+ if (postq_index 0x4000) {
+ index_stripped = postq_index 0xFF;
+
[Bug 83391] Oops on sd_mod
https://bugzilla.kernel.org/show_bug.cgi?id=83391
--- Comment #2 from tomsun tomsunc...@gmail.com ---
static void sd_read_block_limits(struct scsi_disk *sdkp)
{
unsigned int sector_sz = sdkp-device-sector_size;
const int vpd_len = 32;
unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL);
if (!buffer ||
/* Block Limits VPD */
scsi_get_vpd_page(sdkp-device, 0xb0, buffer, vpd_len))
goto out;
blk_queue_io_min(sdkp-disk-queue,
get_unaligned_be16(buffer[6]) * sector_sz);
blk_queue_io_opt(sdkp-disk-queue,
get_unaligned_be32(buffer[12]) * sector_sz);
if (buffer[3] == 0x3c) {
unsigned int lba_count, desc_count;
sdkp-max_ws_blocks =
(u32) min_not_zero(get_unaligned_be64(buffer[36]),
(u64)0x);
if (!sdkp-lbpme)
goto out;
lba_count = get_unaligned_be32(buffer[20]);
desc_count = get_unaligned_be32(buffer[24]);
if (lba_count desc_count)
sdkp-max_unmap_blocks = lba_count;
sdkp-unmap_granularity = get_unaligned_be32(buffer[28]);
if (buffer[32] 0x80)
sdkp-unmap_alignment =
get_unaligned_be32(buffer[32]) ~(1 31);
if (!sdkp-lbpvpd) { /* LBP VPD page not provided */
if (sdkp-max_unmap_blocks)
sd_config_discard(sdkp, SD_LBP_UNMAP);
else
sd_config_discard(sdkp, SD_LBP_WS16);
} else {/* LBP VPD page tells us what to use */
if (sdkp-lbpu sdkp-max_unmap_blocks)
sd_config_discard(sdkp, SD_LBP_UNMAP);
else if (sdkp-lbpws)
sd_config_discard(sdkp, SD_LBP_WS16);
else if (sdkp-lbpws10)
sd_config_discard(sdkp, SD_LBP_WS10);
else
sd_config_discard(sdkp, SD_LBP_DISABLE);
}
}
out:
kfree(buffer);
}
first, the pointer of buffer is malloced 32 bytes memory, but the buffer be
misused as 64 bytes memory, ex. sdkp-max_ws_blocks =
(u32) min_not_zero(get_unaligned_be64(buffer[36]),
(u64)0x);
I don't know why, is it the bug for this oops?
thank you very much~
--
You are receiving this mail because:
You are watching the assignee of the bug.
--
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 v9 3/3] ahci_xgene: Fix the link down in first attempt for the APM X-Gene SoC AHCI SATA host controller driver.
Due to HW errata the APM X-Gene AHCI SATA host controller reports link
down even if the device presence is detected. This issue is due to speed
negotiation failure. This patch implements the algorithm to retry the
COMRESET if PxSTAT register reports device presence detected but
PHY communication not established. The maximum retry attempts are 3.
This patch also fixes the code to match the algorithm for the printing
a warning message if the disparity error still exists after link up.
Signed-off-by: Loc Ho l...@apm.com
Signed-off-by: Suman Tripathi stripa...@apm.com
---
drivers/ata/ahci_xgene.c | 48 ++--
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index c721887..40d0a76 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -78,6 +78,9 @@
#define CFG_MEM_RAM_SHUTDOWN 0x0070
#define BLOCK_MEM_RDY 0x0074
+/* Max retry for link down */
+#define MAX_LINK_DOWN_RETRY 3
+
struct xgene_ahci_context {
struct ahci_host_priv *hpriv;
struct device *dev;
@@ -237,8 +240,11 @@ static void xgene_ahci_set_phy_cfg(struct
xgene_ahci_context *ctx, int channel)
* and Gen1 (1.5Gbps). Otherwise during long IO stress test, the PHY will
* report disparity error and etc. In addition, during COMRESET, there can
* be error reported in the register PORT_SCR_ERR. For SERR_DISPARITY and
- * SERR_10B_8B_ERR, the PHY receiver line must be reseted. The following
- * algorithm is followed to proper configure the hardware PHY during COMRESET:
+ * SERR_10B_8B_ERR, the PHY receiver line must be reseted. Also during long
+ * reboot cycle regression, sometimes the PHY reports link down even if the
+ * device is present because of speed negotiation failure. so need to retry
+ * the COMRESET to get the link up. The following algorithm is followed to
+ * proper configure the hardware PHY during COMRESET:
*
* Alg Part 1:
* 1. Start the PHY at Gen3 speed (default setting)
@@ -254,9 +260,15 @@ static void xgene_ahci_set_phy_cfg(struct
xgene_ahci_context *ctx, int channel)
* Alg Part 2:
* 1. On link up, if there are any SERR_DISPARITY and SERR_10B_8B_ERR error
*reported in the register PORT_SCR_ERR, then reset the PHY receiver line
- * 2. Go to Alg Part 3
+ * 2. Go to Alg Part 4
*
* Alg Part 3:
+ * 1. Check the PORT_SCR_STAT to see whether device presence detected but PHY
+ *communication establishment failed and maximum link down attempts are
+ *less than Max attempts 3 then goto Alg Part 1.
+ * 2. Go to Alg Part 4.
+ *
+ * Alg Part 4:
* 1. Clear any pending from register PORT_SCR_ERR.
*
* NOTE: For the initial version, we will NOT support Gen1/Gen2. In addition
@@ -275,19 +287,27 @@ static int xgene_ahci_do_hardreset(struct ata_link *link,
u8 *d2h_fis = pp-rx_fis + RX_FIS_D2H_REG;
void __iomem *port_mmio = ahci_port_base(ap);
struct ata_taskfile tf;
+ int link_down_retry = 0;
int rc;
- u32 val;
-
- /* clear D2H reception area to properly wait for D2H FIS */
- ata_tf_init(link-device, tf);
- tf.command = ATA_BUSY;
- ata_tf_to_fis(tf, 0, 0, d2h_fis);
- rc = sata_link_hardreset(link, timing, deadline, online,
+ u32 val, sstatus;
+
+ do {
+ /* clear D2H reception area to properly wait for D2H FIS */
+ ata_tf_init(link-device, tf);
+ tf.command = ATA_BUSY;
+ ata_tf_to_fis(tf, 0, 0, d2h_fis);
+ rc = sata_link_hardreset(link, timing, deadline, online,
ahci_check_ready);
-
- val = readl(port_mmio + PORT_SCR_ERR);
- if (val (SERR_DISPARITY | SERR_10B_8B_ERR))
- dev_warn(ctx-dev, link has error\n);
+ if (*online) {
+ val = readl(port_mmio + PORT_SCR_ERR);
+ if (val (SERR_DISPARITY | SERR_10B_8B_ERR))
+ dev_warn(ctx-dev, link has error\n);
+ break;
+ }
+
+ sata_scr_read(link, SCR_STATUS, sstatus);
+ } while (link_down_retry++ MAX_LINK_DOWN_RETRY
+(sstatus 0xff) == 0x1);
/* clear all errors if any pending */
val = readl(port_mmio + PORT_SCR_ERR);
--
1.8.2.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 v9 0/3] ahci_xgene: Fixes related to APM X-Gene SATA host controller driver.
This patch set contains a couple of fixes related to APM X-Gene SATA controller driver. v2 Change: 1. Drop the Link down retry patch from this patch set. v4 Change: 1. Drop the patch to fix the csr-mask in dts for PHY clock node of SATA Host Controller 1. 2. Add the patch to correct the OOB tunning parameters for the COMINIT/COMWAKE parameters. 3. Add the patch to remove the NCQ support from the APM X-Gene AHCI SATA Host controller driver. 4. Add the patch to remove the clock and PHY reference nodes from the APM X-Gene Host controller dts node. v5 Change : 1. All the patches are based on 3.16.0-rc6/for-3.17 kernel. 2. Drop the patch to remove the clock and PHY reference nodes from the APM X-Gene Host controller dts node as it breaks with old firmware. 3. Add the patch to skip phy and clock initialisation if already done in the firmware. 4. Add the patch to fix the csr-mask in dts for PHY clock node of SATA Host Controller 1. 5. Add the patch to remove the NCQ support from the APM X-Gene AHCI SATA Host controller driver based on 3.16.0-rc6/ for-3.17 kernel. 6. Drop the patch to correct the OOB tunning parameters for the COMINIT/COMWAKE parameters as it is already applied to 3.16/for-3.17 branch by the maintainer. 7. Drop the patch to fix the watermark threshold as it is already applied to 3.16/for-3.17 branch by the maintainer. v6 change : 1. Drop the patch to skip phy and clock initialisation if already done in the firmware. 2. Drop the patch to fix the csr-mask in dts for PHY clock node of SATA Host Controller 1. 3. Add the remove the clock and PHY node patch as it fixes the dropped patches together. This patch works with old and new firmware as well. v7 change : 1. Drop the patch to remove the clock and PHY reference nodes from the APM X-Gene Host controller dts node as it breaks with old firmware if sata not initialised from the firmware. 2. Add the patch to skip phy and clock initialisation if already done in the firmware. 3. Add the patch to fix the csr-mask in dts for PHY clock node of SATA Host Controller 1. 4. Add the Link down retry patch with a proper comments and explanation. 5. Drop the patch to remove NCQ as it is applied to 3.16/for-3.17. v8 change : 1. Adding proper comments for link down retry patch. 2. changing the return type to bool for skip phy patch. v9 change: 1. Fixing the code to match the comment for link down retry patch. Signed-off-by: Loc Ho l...@apm.com Signed-off-by: Suman Tripathi stripa...@apm.com --- Suman Tripathi (3): arm64: Fix the csr-mask for APM X-Gene SoC AHCI SATA PHY clock DTS node. ahci_xgene: Skip the PHY and clock initialization if already configured by the firmware. ahci_xgene: Fix the link down in first attempt for the APM X-Gene SoC AHCI SATA host controller driver. arch/arm64/boot/dts/apm-storm.dtsi | 5 +-- drivers/ata/ahci_xgene.c | 63 +- 2 files changed, 50 insertions(+), 18 deletions(-) -- 1.8.2.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 v9 1/3] arm64: Fix the csr-mask for APM X-Gene SoC AHCI SATA PHY clock DTS node.
The value of the csr-mask of the SATA PHY clock DTS node has a wrong value resulting a kernel panic as the clock/reset is not proper for the PHY of the SATA host controller 1. This patch fixes the correct csr-mask value of the SATA PHY clock DTS node for the SATA Host controller 1. As the 'ok' is the default status of a device tree node, this patch removes the status of the PHY clock node of SATA Host Controller 1. The status of the clock node is handled from the firmware based on the controller enabled/disabled by the user. Signed-off-by: Loc Ho l...@apm.com Signed-off-by: Suman Tripathi stripa...@apm.com --- arch/arm64/boot/dts/apm-storm.dtsi | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi index 40aa96c..1bdaeda 100644 --- a/arch/arm64/boot/dts/apm-storm.dtsi +++ b/arch/arm64/boot/dts/apm-storm.dtsi @@ -184,9 +184,8 @@ reg = 0x0 0x1f21c000 0x0 0x1000; reg-names = csr-reg; clock-output-names = sataphy1clk; - status = disabled; csr-offset = 0x4; - csr-mask = 0x00; + csr-mask = 0x3a; enable-offset = 0x0; enable-mask = 0x06; }; @@ -198,7 +197,6 @@ reg = 0x0 0x1f22c000 0x0 0x1000; reg-names = csr-reg; clock-output-names = sataphy2clk; - status = ok; csr-offset = 0x4; csr-mask = 0x3a; enable-offset = 0x0; @@ -212,7 +210,6 @@ reg = 0x0 0x1f23c000 0x0 0x1000; reg-names = csr-reg; clock-output-names = sataphy3clk; - status = ok; csr-offset = 0x4; csr-mask = 0x3a; enable-offset = 0x0; -- 1.8.2.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 v9 2/3] ahci_xgene: Skip the PHY and clock initialization if already configured by the firmware.
This patch implements the feature to skip the PHY and clock
initialization if it is already configured by the firmware.
Signed-off-by: Loc Ho l...@apm.com
Signed-off-by: Suman Tripathi stripa...@apm.com
---
drivers/ata/ahci_xgene.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index f416495..c721887 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -145,6 +145,14 @@ static unsigned int xgene_ahci_qc_issue(struct
ata_queued_cmd *qc)
return rc;
}
+static bool xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
+{
+ void __iomem *diagcsr = ctx-csr_diag;
+
+ return (readl(diagcsr + CFG_MEM_RAM_SHUTDOWN) == 0
+ readl(diagcsr + BLOCK_MEM_RDY) == 0x);
+}
+
/**
* xgene_ahci_read_id - Read ID data from the specified device
* @dev: device
@@ -468,6 +476,11 @@ static int xgene_ahci_probe(struct platform_device *pdev)
return -ENODEV;
}
+ if (xgene_ahci_is_memram_inited(ctx)) {
+ dev_info(dev, skip clock and PHY initialization\n);
+ goto skip_clk_phy;
+ }
+
/* Due to errata, HW requires full toggle transition */
rc = ahci_platform_enable_clks(hpriv);
if (rc)
@@ -481,6 +494,8 @@ static int xgene_ahci_probe(struct platform_device *pdev)
/* Configure the host controller */
xgene_ahci_hw_init(hpriv);
+skip_clk_phy:
+
hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ;
rc = ahci_platform_init_host(pdev, hpriv, xgene_ahci_port_info,
--
1.8.2.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: Debugging scsi abort handling ?
On 08/25/2014 01:15 PM, Paolo Bonzini wrote: Il 25/08/2014 12:28, Bart Van Assche ha scritto: From SPC-4: 7.5.8 Control mode page [ ... ] A task aborted status (TAS) bit set to zero specifies that aborted commands shall be terminated by the device server without any response to the application client. A TAS bit set to one specifies that commands aborted by the actions of an I_T nexus other than the I_T nexus on which the command was received shall be completed with TASK ABORTED status (see SAM-5). Note the aborted by the actions of an I_T nexus other than the I_T nexus on which the command was received. In practice, this means that TASK ABORTED should only happen if you use the CLEAR TASK SET tmf and TST is not set to 001b (i.e. _not_ to per I_T nexus) in the Control mode page. It should never happen for a pen drive. Setting TASK ABORTED aside, the important part is that an abort can do one of two things: - complete the command, and then eh_abort should return after the driver has noticed the completion and called the -scsi_done callback for the Scsi_Cmnd*. - abort the command, and then the driver should never call the -scsi_done callback for the Scsi_Cmnd*. In practice we rely on the latter behaviour; when -scsi_done is called while the command is under eh_abort _really bad things_ will happen. As soon as eh_abort is called control is transferred back to the SCSI midlayer, so any LLDD should never send completions for these commands back to the midlayer. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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: Debugging scsi abort handling ?
On 08/26/2014 09:19 PM, Hans de Goede wrote: Hi, On 08/26/2014 08:34 PM, James Bottomley wrote: On Tue, 2014-08-26 at 10:13 +0200, Hans de Goede wrote: Hi, On 08/25/2014 05:41 PM, James Bottomley wrote: On Mon, 2014-08-25 at 13:39 +0200, Paolo Bonzini wrote: Il 25/08/2014 13:26, Hans de Goede ha scritto: Thanks Bart and Paolo, your insights into this are greatly appreciated. So with uas there are separate usb transaction for cmd, data in, data out and sense for each tag. At the time of abort, usually one of data in / data out and a sense usb transaction will be outstanding. There already is logic in the driver to kill the data in / out transactions if a sense gets returned (usually with an error) before they are done. So if I'm reading this correctly, then on a successful abort, the sense transaction (if not already completed by the target) should be cancelled as it will never complete, correct ? Yes. More precisely, once the response IU comes back for the abort, there should be no more IUs for that task. Figure 13 (Multiple Command Example) in the UAS spec actually shows that. At least, that's what it should be like in theory. I suspect firmware bugs will abound in this area, but at least you shouldn't be actively expecting an IU for an aborted task. Just a note on this. The abort area in all devices is where we have scope for spec compliance issues. Also in the old days abort itself could trigger a firmware crash on some devices (including arrays). The problem is actually that the system is usually groaning because it's out of memory within the controller. That actually means that sending in another task (the abort) causes greater pressure. For this reason, some device drivers choose to skip the abort step and head straight to reset. For reset, you guarantee that all outstanding tasks for the unit are killed. Hmm, I like this idea, given the finickiness of the abort path in the uas driver, and that: 1) We really have no proper way to test this 2) We already have some known issues there (we don't kill sense urbs atm, and I've a note somewhere about a double free on some corner case where an urb submit fails) 3) 3) In all the cases where I've managed to trip op an uas device the only thing which actually worked to recover things was doing a usb reset 4) Aborts should not happen in the first place, so using a big hammer when they do is not really a big problem, instead we should focus on figuring out why they happen and fix the cause I think that just dropping abort handling altogether is a good idea actually, so if there are no objections I'm just going to do that. I can simply not set eh_abort_handler (aka set it to NULL), right ? Just so you know, if you do this, error handling becomes much more painful. The abort handler can now handle aborting and retrying without pausing the host. The reset definitely stops the host and causes a big and noticeable burp in processing. However, there are hosts which have to do it. I understand, but shouldn't aborts be something which rarely happens. I guess that with a faulty drive they happen more often, but at this point in time the uas driver's error handling problems are mostly with dealing with timeouts during probing, which are likely caused by the device not grokking some command we've send, and responding to this in a bad manner (hanging mostly). So I'm tempted to just remove the abort handling, and first focus on getting uas stable under all conditions. Once it is stable we can look into making it deal more graciously with errors. Sounds like a reasonable plan. [ .. ] Wait, could this also be your abort problem? In the running abort handler, we could get a scenario where we abort a bunch of commands at the same time. I don't think so, the uas code does not return from eh_abort_handler until the abort has either succeeded or timedout (for which a 3 second timeout is used). AFAIK the scsi core will always issue multiple aborts sequentially, right. And if the abort succeeds then the tag is free again for the next abort. Only if an abort fails (times out, which is what I've seen in all the error conditions which I've encountered so far), then will subsequent aborts, and the logical unit reset, fail due to there not being a free tag to use for them. If the logic for command/task abort and logical unit reset is similar then there is no point in implementing both. So by what I've seen I would first implement target reset (and host reset :-), and worry about finer grained error control later on. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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
Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print_command trace point instead of printk
On 08/28/2014 08:19 AM, Yoshihiro YUNOMAE wrote: Hi Hannes, I tried to remove duplicated decoder of SCSI command, but the output format of it in constants.c is different from it in traceevents. I have two questions for it. (Ex1) traceevents: TEST_UNIT_READY constants: Test Unit Ready = Which of XXX_YYY_ZZZ and Xxx Yyy Zzz should the kernel output strings? This difference comes from difference of implementation. The decoder in traceevents are using macro. So, it cannot define separated words. On the other hand, the decoder in constants are using constant string array table. So, it can define separated words. I would go with the wording in constants.c, but set in double quotes like Test Unit Ready to avoid parsing errors later on. (Ex2) traceevents: (nothing) constants: Set limits(12) = Should we merge those decoder? Yes, I would prefer this. Again, we should be setting the strings in double quotes irrespective if there are spaces in the resulting string or not. I understand we use the decoder of constants, but we need to solve these problems. Would you give me your comments? As mentioned previously, my aim is to convert the logging system in several steps: a) convert all lone 'printk' statements into dev_printk() variants and ensure everything is printed in one statement to avoid linebreaks in the resulting logging message. This will eliminate the immediate problem we're having, namely that debugging is near to impossible under high load. The patchset is nearing completion, and I will be posting it later this week. b) Convert scsi_trace to use the functions from constants.c to avoid code duplication and update scsi_trace to log sense codes. c) Remove all logging functions in the hot path (ie SCSI_LOG_MLQUEUE and SCSI_LOG_MLCOMPLETE) and replace them with trace events. b) is what you're working on, right? For 'c)' the problem is that we should retain the existing functionality with scsi_logging_level here, at least initially. So we cannot just replace it, as this would leave SCSI_LOG_ML a dummy without any real functionality. I'll be posting my current patchset for review shortly. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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: Debugging scsi abort handling ?
Hi, On 08/28/2014 02:10 PM, Hannes Reinecke wrote: On 08/26/2014 09:19 PM, Hans de Goede wrote: Hi, On 08/26/2014 08:34 PM, James Bottomley wrote: On Tue, 2014-08-26 at 10:13 +0200, Hans de Goede wrote: Hi, On 08/25/2014 05:41 PM, James Bottomley wrote: On Mon, 2014-08-25 at 13:39 +0200, Paolo Bonzini wrote: Il 25/08/2014 13:26, Hans de Goede ha scritto: Thanks Bart and Paolo, your insights into this are greatly appreciated. So with uas there are separate usb transaction for cmd, data in, data out and sense for each tag. At the time of abort, usually one of data in / data out and a sense usb transaction will be outstanding. There already is logic in the driver to kill the data in / out transactions if a sense gets returned (usually with an error) before they are done. So if I'm reading this correctly, then on a successful abort, the sense transaction (if not already completed by the target) should be cancelled as it will never complete, correct ? Yes. More precisely, once the response IU comes back for the abort, there should be no more IUs for that task. Figure 13 (Multiple Command Example) in the UAS spec actually shows that. At least, that's what it should be like in theory. I suspect firmware bugs will abound in this area, but at least you shouldn't be actively expecting an IU for an aborted task. Just a note on this. The abort area in all devices is where we have scope for spec compliance issues. Also in the old days abort itself could trigger a firmware crash on some devices (including arrays). The problem is actually that the system is usually groaning because it's out of memory within the controller. That actually means that sending in another task (the abort) causes greater pressure. For this reason, some device drivers choose to skip the abort step and head straight to reset. For reset, you guarantee that all outstanding tasks for the unit are killed. Hmm, I like this idea, given the finickiness of the abort path in the uas driver, and that: 1) We really have no proper way to test this 2) We already have some known issues there (we don't kill sense urbs atm, and I've a note somewhere about a double free on some corner case where an urb submit fails) 3) 3) In all the cases where I've managed to trip op an uas device the only thing which actually worked to recover things was doing a usb reset 4) Aborts should not happen in the first place, so using a big hammer when they do is not really a big problem, instead we should focus on figuring out why they happen and fix the cause I think that just dropping abort handling altogether is a good idea actually, so if there are no objections I'm just going to do that. I can simply not set eh_abort_handler (aka set it to NULL), right ? Just so you know, if you do this, error handling becomes much more painful. The abort handler can now handle aborting and retrying without pausing the host. The reset definitely stops the host and causes a big and noticeable burp in processing. However, there are hosts which have to do it. I understand, but shouldn't aborts be something which rarely happens. I guess that with a faulty drive they happen more often, but at this point in time the uas driver's error handling problems are mostly with dealing with timeouts during probing, which are likely caused by the device not grokking some command we've send, and responding to this in a bad manner (hanging mostly). So I'm tempted to just remove the abort handling, and first focus on getting uas stable under all conditions. Once it is stable we can look into making it deal more graciously with errors. Sounds like a reasonable plan. [ .. ] Wait, could this also be your abort problem? In the running abort handler, we could get a scenario where we abort a bunch of commands at the same time. I don't think so, the uas code does not return from eh_abort_handler until the abort has either succeeded or timedout (for which a 3 second timeout is used). AFAIK the scsi core will always issue multiple aborts sequentially, right. And if the abort succeeds then the tag is free again for the next abort. Only if an abort fails (times out, which is what I've seen in all the error conditions which I've encountered so far), then will subsequent aborts, and the logical unit reset, fail due to there not being a free tag to use for them. If the logic for command/task abort and logical unit reset is similar then there is no point in implementing both. The logic is different in that when an abort gets issued other commands may (on paper) still complete normally, where as with a lun reset all commands on that lun (and usually there is only one lun) are toast. So by what I've seen I would first implement target reset (and host reset :-) With uas target == host, and that is already implemented and so far seems to be the only thing which actually
Re: Debugging scsi abort handling ?
Hi, On 08/28/2014 02:17 PM, Paolo Bonzini wrote: Il 28/08/2014 14:04, Hannes Reinecke ha scritto: Setting TASK ABORTED aside, the important part is that an abort can do one of two things: - complete the command, and then eh_abort should return after the driver has noticed the completion and called the -scsi_done callback for the Scsi_Cmnd*. - abort the command, and then the driver should never call the -scsi_done callback for the Scsi_Cmnd*. In practice we rely on the latter behaviour; when -scsi_done is called while the command is under eh_abort _really bad things_ will happen. As soon as eh_abort is called control is transferred back to the SCSI midlayer, so any LLDD should never send completions for these commands back to the midlayer. No, this is wrong. I think we have sorted it out a couple of months ago. virtio-scsi for example (due to QEMU quirks) will do the former more often than not. Ignoring scsi_eh_done which is just as harmless, scsi_eh_done is new to me. Should I ever use that, and if so when ? -scsi_done does nothing more than calling blk_complete_request. If the command is under abort, it has already been marked as complete by the block layer's timeout timer---see blk_rq_timed_out_timer and blk_rq_check_expired---or by blk_abort_request. Then, blk_complete_request will do nothing because its call to blk_mark_rq_complete returns true. All this, of course, as long as -scsi_done is called _before_ eh_abort returns. What about calling scsi_done after eh_abort if eh_abort returned FAILED? Regards, Hans -- 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: Debugging scsi abort handling ?
On Thu, 2014-08-28 at 14:04 +0200, Hannes Reinecke wrote: On 08/25/2014 01:15 PM, Paolo Bonzini wrote: - abort the command, and then the driver should never call the -scsi_done callback for the Scsi_Cmnd*. In practice we rely on the latter behaviour; when -scsi_done is called while the command is under eh_abort _really bad things_ will happen. As soon as eh_abort is called control is transferred back to the SCSI midlayer, so any LLDD should never send completions for these commands back to the midlayer. Mmh, then there is a small race window starting at the point in time when the abort function of an LLDD is called up to the point in time when that abort function has taken the necessary measures to make sure that scsi_done won't be called for a racing command completion , isn't it? Cheers Martin -- Linux on System z Development IBM Deutschland Research Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- 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: Debugging scsi abort handling ?
Il 28/08/2014 14:26, Hans de Goede ha scritto: Then, blk_complete_request will do nothing because its call to blk_mark_rq_complete returns true. All this, of course, as long as -scsi_done is called _before_ eh_abort returns. What about calling scsi_done after eh_abort if eh_abort returned FAILED? I invoke the fifth amendment. :) Paolo -- 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: Debugging scsi abort handling ?
Hi, On 08/28/2014 02:33 PM, Paolo Bonzini wrote: Il 28/08/2014 14:26, Hans de Goede ha scritto: Then, blk_complete_request will do nothing because its call to blk_mark_rq_complete returns true. All this, of course, as long as -scsi_done is called _before_ eh_abort returns. What about calling scsi_done after eh_abort if eh_abort returned FAILED? I invoke the fifth amendment. :) Although I appreciate the tongue in cheek answer, this was sort of a serious question, as at the moment this may happen with the uas driver. Regards, Hans -- 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 v3 16/17] arcmsr: support new adapter ARC12x4 series
On 08/28/2014 05:46 PM, Ching Huang wrote:
On Wed, 2014-08-27 at 16:00 +0200, Tomas Henzl wrote:
On 08/19/2014 09:25 AM, Ching Huang wrote:
From: Ching Huang ching2...@areca.com.tw
Add code for supporting Areca new Raid adapter ARC12x4 series.
Signed-off-by: Ching Huang ching2...@areca.com.tw
---
Hi Ching,
please look at the comments below -
}
@@ -1039,7 +1147,60 @@ static void arcmsr_done4abort_postqueue(
error = (flag_ccb ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ?
true : false;
arcmsr_drain_donequeue(acb, pCCB, error);
}
- }
+ }
+ break;
+ case ACB_ADAPTER_TYPE_D: {
+ struct MessageUnit_D *pmu = acb-pmuD;
+ uint32_t ccb_cdb_phy, outbound_write_pointer;
+ uint32_t doneq_index, index_stripped, addressLow, residual;
+ bool error;
+ struct CommandControlBlock *pCCB;
I have noticed this^ in this driver already before. Sometimes it makes sense
when a variable is declared in a 'case' block but often it is just
a waste of space. In this function this is the third 'bool error' declared.
Is there a reason for this style (this function is not the worst case) ?
Yea, there is many variable are duplicate declaration, I will clean it up.
+ outbound_write_pointer = pmu-done_qbuffer[0].addressLow + 1;
+ doneq_index = pmu-doneq_index;
+ residual = atomic_read(acb-ccboutstandingcount);
+ for (i = 0; i residual; i++) {
+ while ((doneq_index 0xFFF) !=
+ (outbound_write_pointer 0xFFF)) {
+ if (doneq_index 0x4000) {
+ index_stripped = doneq_index 0xFFF;
+ index_stripped += 1;
+ index_stripped %=
+ ARCMSR_MAX_ARC1214_DONEQUEUE;
+ pmu-doneq_index = index_stripped ?
+ (index_stripped | 0x4000) :
+ (index_stripped + 1);
+ } else {
+ index_stripped = doneq_index;
+ index_stripped += 1;
+ index_stripped %=
+ ARCMSR_MAX_ARC1214_DONEQUEUE;
+ pmu-doneq_index = index_stripped ?
+ index_stripped :
+ ((index_stripped | 0x4000) + 1);
+ }
+ doneq_index = pmu-doneq_index;
+ addressLow = pmu-done_qbuffer[doneq_index
+ 0xFFF].addressLow;
+ ccb_cdb_phy = (addressLow 0xFFF0);
+ pARCMSR_CDB = (struct ARCMSR_CDB *)
+ (acb-vir2phy_offset + ccb_cdb_phy);
+ pCCB = container_of(pARCMSR_CDB,
+ struct CommandControlBlock, arcmsr_cdb);
+ error = (addressLow
+ ARCMSR_CCBREPLY_FLAG_ERROR_MODE1) ?
+ true : false;
+ arcmsr_drain_donequeue(acb, pCCB, error);
+ writel(doneq_index,
pmu-outboundlist_read_pointer);
+ }
+ mdelay(10);
+ outbound_write_pointer =
+ pmu-done_qbuffer[0].addressLow + 1;
+ doneq_index = pmu-doneq_index;
+ }
+ pmu-postq_index = 0;
+ pmu-doneq_index = 0x40FF;
+ }
+ break;
}
}
...
@@ -1256,6 +1424,38 @@ static void arcmsr_post_ccb(struct Adapt
writel(ccb_post_stamp, phbcmu-inbound_queueport_low);
}
}
+ break;
+ case ACB_ADAPTER_TYPE_D: {
+ struct MessageUnit_D *pmu = acb-pmuD;
+ u16 index_stripped;
+ u16 postq_index;
u16 postq_index, toggle;
+ unsigned long flags;
+ struct InBound_SRB *pinbound_srb;
+
+ spin_lock_irqsave(acb-postq_lock, flags);
+ postq_index = pmu-postq_index;
+ pinbound_srb = (struct InBound_SRB
*)(pmu-post_qbuffer[postq_index 0xFF]);
+ pinbound_srb-addressHigh = dma_addr_hi32(cdb_phyaddr);
+ pinbound_srb-addressLow = dma_addr_lo32(cdb_phyaddr);
+ pinbound_srb-length = ccb-arc_cdb_size 2;
+ arcmsr_cdb-msgContext = dma_addr_lo32(cdb_phyaddr);
+ if (postq_index 0x4000) {
+ index_stripped = postq_index 0xFF;
+ index_stripped += 1;
+
[Bug 83391] Oops on sd_mod
https://bugzilla.kernel.org/show_bug.cgi?id=83391 Jeff Moyer jmo...@redhat.com changed: What|Removed |Added CC||jmo...@redhat.com --- Comment #3 from Jeff Moyer jmo...@redhat.com --- This is a vendor kernel. You should file a bug report with Red Hat here: https://bugzilla.redhat.com/enter_bug.cgi?product=Red%20Hat%20Enterprise%20Linux%206 First, though, you should update to a newer kernel... that one is several years old. -- You are receiving this mail because: You are watching the assignee of the bug. -- 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: Debugging scsi abort handling ?
On Thu, 2014-08-28 at 14:37 +0200, Hans de Goede wrote: Hi, On 08/28/2014 02:33 PM, Paolo Bonzini wrote: Il 28/08/2014 14:26, Hans de Goede ha scritto: Then, blk_complete_request will do nothing because its call to blk_mark_rq_complete returns true. All this, of course, as long as -scsi_done is called _before_ eh_abort returns. What about calling scsi_done after eh_abort if eh_abort returned FAILED? I invoke the fifth amendment. :) Although I appreciate the tongue in cheek answer, this was sort of a serious question, as at the moment this may happen with the uas driver. The answer is no. It's part of the internal mid layer functions you don't need to know about. You just call the command done functions and try not to care what actual function they point to. James -- 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: Debugging scsi abort handling ?
On Thu, 2014-08-28 at 14:21 +0200, Hans de Goede wrote: Hi, On 08/28/2014 02:04 PM, Hannes Reinecke wrote: On 08/25/2014 01:15 PM, Paolo Bonzini wrote: Il 25/08/2014 12:28, Bart Van Assche ha scritto: From SPC-4: 7.5.8 Control mode page [ ... ] A task aborted status (TAS) bit set to zero specifies that aborted commands shall be terminated by the device server without any response to the application client. A TAS bit set to one specifies that commands aborted by the actions of an I_T nexus other than the I_T nexus on which the command was received shall be completed with TASK ABORTED status (see SAM-5). Note the aborted by the actions of an I_T nexus other than the I_T nexus on which the command was received. In practice, this means that TASK ABORTED should only happen if you use the CLEAR TASK SET tmf and TST is not set to 001b (i.e. _not_ to per I_T nexus) in the Control mode page. It should never happen for a pen drive. Setting TASK ABORTED aside, the important part is that an abort can do one of two things: - complete the command, and then eh_abort should return after the driver has noticed the completion and called the -scsi_done callback for the Scsi_Cmnd*. - abort the command, and then the driver should never call the -scsi_done callback for the Scsi_Cmnd*. In practice we rely on the latter behaviour; when -scsi_done is called while the command is under eh_abort _really bad things_ will happen. Interesting, those very bad things may very well be exactly the things some uas users are seeing. But this sounds racy, I can stop a command from completing as the very first thing inside eh_abort, but I cannot stop it from completing when the scsi core is getting ready to call eh_abort, but eh_abort is not yet called. Is there some flag I should check before calling scsi_done to avoid this race? And if so which locks should I hold (and why does scsi_done not do this check itself) ? As soon as eh_abort is called control is transferred back to the SCSI midlayer, so any LLDD should never send completions for these commands back to the midlayer. I'm fine with not calling scsi_done from eh_abort, but I cannot guarantee that another thread will not complete the cmnd in the mean time before hand. This is expected. After error handling begins, the block layer ignores all done callbacks for commands under EH. You can get the situation where we try to abort (or do other EH) on a command you think you've already completed, but you just return success for that. James -- 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: Debugging scsi abort handling ?
On 08/28/2014 02:37 PM, Hans de Goede wrote: Hi, On 08/28/2014 02:33 PM, Paolo Bonzini wrote: Il 28/08/2014 14:26, Hans de Goede ha scritto: Then, blk_complete_request will do nothing because its call to blk_mark_rq_complete returns true. All this, of course, as long as -scsi_done is called _before_ eh_abort returns. What about calling scsi_done after eh_abort if eh_abort returned FAILED? I invoke the fifth amendment. :) Although I appreciate the tongue in cheek answer, this was sort of a serious question, as at the moment this may happen with the uas driver. As mentioned earlier, as soon as SCSI EH is invoked control is assumed to be transferred back to the SCSI midlayer. How the midlayer interprets any return value from the various eh_XX callbacks is immaterial to the LLDD. So even if the eh_abort returns FAILED control is still with the SCSI midlayer, so the earlier statements apply. IE the command will be short-circuited by the block layer anyway if -scsi_done() is called. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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: Debugging scsi abort handling ?
On 08/28/2014 02:31 PM, Martin Peschke wrote: On Thu, 2014-08-28 at 14:04 +0200, Hannes Reinecke wrote: On 08/25/2014 01:15 PM, Paolo Bonzini wrote: - abort the command, and then the driver should never call the -scsi_done callback for the Scsi_Cmnd*. In practice we rely on the latter behaviour; when -scsi_done is called while the command is under eh_abort _really bad things_ will happen. As soon as eh_abort is called control is transferred back to the SCSI midlayer, so any LLDD should never send completions for these commands back to the midlayer. Mmh, then there is a small race window starting at the point in time when the abort function of an LLDD is called up to the point in time when that abort function has taken the necessary measures to make sure that scsi_done won't be called for a racing command completion , isn't it? No, that's fine. The timeout function is under control of the block layer, which sets an atomic flag on the request before calling the timeout function. And -scsi_done() essentially just calls 'blk_complete_request();, which checks the very same flag before raising the block soft irq. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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: Debugging scsi abort handling ?
Il 28/08/2014 16:17, Hannes Reinecke ha scritto: As mentioned earlier, as soon as SCSI EH is invoked control is assumed to be transferred back to the SCSI midlayer. How the midlayer interprets any return value from the various eh_XX callbacks is immaterial to the LLDD. So even if the eh_abort returns FAILED control is still with the SCSI midlayer, so the earlier statements apply. IE the command will be short-circuited by the block layer anyway if -scsi_done() is called. As I parsed it, the question is not whether the short-circuiting will happen. It's whether you will have use-after-free bugs or not if you call -scsi_done() after eh_abort returns FAILED. Paolo -- 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: Debugging scsi abort handling ?
On 08/28/2014 04:56 PM, Paolo Bonzini wrote: Il 28/08/2014 16:17, Hannes Reinecke ha scritto: As mentioned earlier, as soon as SCSI EH is invoked control is assumed to be transferred back to the SCSI midlayer. How the midlayer interprets any return value from the various eh_XX callbacks is immaterial to the LLDD. So even if the eh_abort returns FAILED control is still with the SCSI midlayer, so the earlier statements apply. IE the command will be short-circuited by the block layer anyway if -scsi_done() is called. As I parsed it, the question is not whether the short-circuiting will happen. It's whether you will have use-after-free bugs or not if you call -scsi_done() after eh_abort returns FAILED. Paolo No. Once eh_abort is called control is back with the SCSI midlayer. (Read: REQ_ATOM_COMPLETE is set in req-atomic_flags). So you can call -scsi_done() at your hearts content and nothing will happen. What might happen, though, that the command is already dead and gone by the time you're calling -scsi_done() (if you call it after eh_abort). So there might not _be_ a command upon which you can call -scsi_done() to start with. Hence any LLDD need to clear up any internal references after a call to eh_XXX to ensure it doesn't call -scsi_done() an in invalid command. So even if the LLDD returns 'FAILED' upon a call to eh_XXX it _still_ needs to clear up the internal reference. Either that or return 'FAILED' for any later eh_XXX function until the internal references can be cleared up. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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: Debugging scsi abort handling ?
-Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Hannes Reinecke Sent: Thursday, 28 August, 2014 10:13 AM To: Paolo Bonzini; Hans de Goede; Bart Van Assche; SCSI development list Subject: Re: Debugging scsi abort handling ? On 08/28/2014 04:56 PM, Paolo Bonzini wrote: Il 28/08/2014 16:17, Hannes Reinecke ha scritto: As mentioned earlier, as soon as SCSI EH is invoked control is assumed to be transferred back to the SCSI midlayer. How the midlayer interprets any return value from the various eh_XX callbacks is immaterial to the LLDD. So even if the eh_abort returns FAILED control is still with the SCSI midlayer, so the earlier statements apply. IE the command will be short-circuited by the block layer anyway if -scsi_done() is called. As I parsed it, the question is not whether the short-circuiting will happen. It's whether you will have use-after-free bugs or not if you call -scsi_done() after eh_abort returns FAILED. Paolo No. Once eh_abort is called control is back with the SCSI midlayer. (Read: REQ_ATOM_COMPLETE is set in req-atomic_flags). So you can call -scsi_done() at your hearts content and nothing will happen. What might happen, though, that the command is already dead and gone by the time you're calling -scsi_done() (if you call it after eh_abort). So there might not _be_ a command upon which you can call - scsi_done() to start with. Hence any LLDD need to clear up any internal references after a call to eh_XXX to ensure it doesn't call -scsi_done() an in invalid command. So even if the LLDD returns 'FAILED' upon a call to eh_XXX it _still_ needs to clear up the internal reference. Either that or return 'FAILED' for any later eh_XXX function until the internal references can be cleared up. Is the block layer prevented from issuing a new command with the same tag before the error handling is finished? --- Rob ElliottHP Server Storage
Re: Debugging scsi abort handling ?
Il 28/08/2014 17:50, Elliott, Robert (Server Storage) ha scritto: Is the block layer prevented from issuing a new command with the same tag before the error handling is finished? Tags are chosen by the LLDs, so it's up to it to pick the right tags. Paolo -- 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: Debugging scsi abort handling ?
On Thu, Aug 28, 2014 at 05:54:50PM +0200, Paolo Bonzini wrote: Il 28/08/2014 17:50, Elliott, Robert (Server Storage) ha scritto: Is the block layer prevented from issuing a new command with the same tag before the error handling is finished? Tags are chosen by the LLDs, so it's up to it to pick the right tags. When using scsi-mq or the older block level tagging implementation they aren't. -- 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/22] scsi: dump sense buffer only for debugging
Dumping the entire sense buffer might overwhelm the logging functions,
so suppress it per default.
Signed-off-by: Hannes Reinecke h...@suse.de.
---
drivers/scsi/53c700.c| 1 -
drivers/scsi/constants.c | 32 ++--
drivers/scsi/scsi.c | 6 --
drivers/scsi/sg.c| 9 ++---
drivers/scsi/st.c| 11 ---
include/scsi/scsi_dbg.h | 10 ++
6 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 68bf423..8752481 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -603,7 +603,6 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata,
printk( ORIGINAL CMD %p RETURNED %d, new return is %d
sense is\n,
SCp, SCp-cmnd[7], result);
scsi_print_sense(SCp);
-
#endif
dma_unmap_single(hostdata-dev, slot-dma_handle,
SCSI_SENSE_BUFFERSIZE,
DMA_FROM_DEVICE);
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index f0a6595..ecce5b3 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1428,9 +1428,9 @@ scsi_print_sense_hdr(struct scsi_device *sdev, const char
*name,
}
EXPORT_SYMBOL(scsi_print_sense_hdr);
-static void
-scsi_dump_sense_buffer(struct scsi_device *sdev, const char *name,
- const unsigned char *sense_buffer, int sense_len)
+void
+__scsi_dump_sense(struct scsi_device *sdev, const char *name,
+ const unsigned char *sense_buffer, int sense_len)
{
char linebuf[128];
int i, linelen, remaining;
@@ -1446,9 +1446,21 @@ scsi_dump_sense_buffer(struct scsi_device *sdev, const
char *name,
Sense: %s\n, linebuf);
}
}
+EXPORT_SYMBOL(__scsi_dump_sense);
+
+void
+scsi_dump_sense(struct scsi_cmnd *cmd)
+{
+ struct gendisk *disk = cmd-request-rq_disk;
+ const char *disk_name = disk ? disk-disk_name : NULL;
+
+ __scsi_dump_sense(cmd-device, disk_name, cmd-sense_buffer,
+ SCSI_SENSE_BUFFERSIZE);
+}
+EXPORT_SYMBOL(scsi_dump_sense);
/* Normalize and print sense buffer with name prefix */
-void __scsi_print_sense(struct scsi_device *sdev, const char *name,
+int __scsi_print_sense(struct scsi_device *sdev, const char *name,
const unsigned char *sense_buffer, int sense_len)
{
struct scsi_sense_hdr sshdr;
@@ -1456,23 +1468,23 @@ void __scsi_print_sense(struct scsi_device *sdev, const
char *name,
res = scsi_normalize_sense(sense_buffer, sense_len, sshdr);
if (res == 0) {
- scsi_dump_sense_buffer(sdev, name, sense_buffer, sense_len);
- return;
+ __scsi_dump_sense(sdev, name, sense_buffer, sense_len);
+ return 0;
}
scsi_show_sense_hdr(sdev, name, sshdr);
scsi_show_extd_sense(sdev, name, sshdr.asc, sshdr.ascq);
- scsi_dump_sense_buffer(sdev, name, sense_buffer, sense_len);
+ return res;
}
EXPORT_SYMBOL(__scsi_print_sense);
/* Normalize and print sense buffer in SCSI command */
-void scsi_print_sense(struct scsi_cmnd *cmd)
+int scsi_print_sense(struct scsi_cmnd *cmd)
{
struct gendisk *disk = cmd-request-rq_disk;
const char *disk_name = disk ? disk-disk_name : NULL;
- __scsi_print_sense(cmd-device, disk_name, cmd-sense_buffer,
- SCSI_SENSE_BUFFERSIZE);
+ return __scsi_print_sense(cmd-device, disk_name, cmd-sense_buffer,
+ SCSI_SENSE_BUFFERSIZE);
}
EXPORT_SYMBOL(scsi_print_sense);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 8954036..283f053 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -597,8 +597,10 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int
disposition)
}
scsi_print_result(cmd);
scsi_print_command(cmd);
- if (status_byte(cmd-result) CHECK_CONDITION)
- scsi_print_sense(cmd);
+ if (status_byte(cmd-result) CHECK_CONDITION) {
+ if (scsi_print_sense(cmd))
+ scsi_dump_sense(cmd);
+ }
if (level 3)
scmd_printk(KERN_INFO, cmd,
scsi host busy %d failed %d\n,
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 16f826e..6bed135 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1359,9 +1359,12 @@ sg_rq_end_io(struct request *rq, int uptodate)
srp-header.driver_status = driver_byte(result);
if ((sdp-sgdebug 0)
((CHECK_CONDITION == srp-header.masked_status) ||
-(COMMAND_TERMINATED ==
[PATCH 06/22] scsi: stop decoding if scsi_normalize_sense() fails
If scsi_normalize_sense() fails we couldn't decode the sense
buffer, so we should not try to interpret the result.
We should rather dump the sense buffer instead and stop decoding.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 36e1ffd..6b05575 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1429,25 +1429,21 @@ scsi_print_sense_hdr(struct scsi_device *sdev, const
char *name,
EXPORT_SYMBOL(scsi_print_sense_hdr);
static void
-scsi_decode_sense_buffer(const unsigned char *sense_buffer, int sense_len,
- struct scsi_sense_hdr *sshdr)
+scsi_dump_sense_buffer(struct scsi_device *sdev, const char *name,
+ const unsigned char *sense_buffer, int sense_len)
{
- int k, num, res;
+ char linebuf[128];
+ int i, linelen, remaining;
- res = scsi_normalize_sense(sense_buffer, sense_len, sshdr);
- if (0 == res) {
- /* this may be SCSI-1 sense data */
- num = (sense_len 32) ? sense_len : 32;
- printk(Unrecognized sense data (in hex):);
- for (k = 0; k num; ++k) {
- if (0 == (k % 16)) {
- printk(\n);
- printk(KERN_INFO );
- }
- printk(%02x , sense_buffer[k]);
- }
- printk(\n);
- return;
+ remaining = sense_len;
+ for (i = 0; i sense_len; i += 16) {
+ linelen = min(remaining, 16);
+ remaining -= 16;
+
+ hex_dump_to_buffer(sense_buffer + i, linelen, 16, 1,
+ linebuf, sizeof(linebuf), false);
+ sdev_prefix_printk(KERN_INFO, sdev, name,
+ Sense: %s\n, linebuf);
}
}
@@ -1517,8 +1513,13 @@ void __scsi_print_sense(struct scsi_device *sdev, const
char *name,
const unsigned char *sense_buffer, int sense_len)
{
struct scsi_sense_hdr sshdr;
+ int res;
- scsi_decode_sense_buffer(sense_buffer, sense_len, sshdr);
+ res = scsi_normalize_sense(sense_buffer, sense_len, sshdr);
+ if (res == 0) {
+ scsi_dump_sense_buffer(sdev, name, sense_buffer, sense_len);
+ return;
+ }
scsi_show_sense_hdr(sdev, name, sshdr);
scsi_decode_sense_extras(sense_buffer, sense_len, sshdr);
scsi_show_extd_sense(sdev, name, sshdr.asc, sshdr.ascq);
--
1.8.5.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 01/22] Remove scsi_cmd_print_sense_hdr()
Unused.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 14 --
include/scsi/scsi_dbg.h | 2 --
2 files changed, 16 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index d35a5d6..2f44707 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1428,20 +1428,6 @@ scsi_print_sense_hdr(const char *name, struct
scsi_sense_hdr *sshdr)
}
EXPORT_SYMBOL(scsi_print_sense_hdr);
-/*
- * Print normalized SCSI sense header with device information and a prefix.
- */
-void
-scsi_cmd_print_sense_hdr(struct scsi_cmnd *scmd, const char *desc,
- struct scsi_sense_hdr *sshdr)
-{
- scmd_printk(KERN_INFO, scmd, %s: , desc);
- scsi_show_sense_hdr(sshdr);
- scmd_printk(KERN_INFO, scmd, %s: , desc);
- scsi_show_extd_sense(sshdr-asc, sshdr-ascq);
-}
-EXPORT_SYMBOL(scsi_cmd_print_sense_hdr);
-
static void
scsi_decode_sense_buffer(const unsigned char *sense_buffer, int sense_len,
struct scsi_sense_hdr *sshdr)
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index e89844c..5a43a4c 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -9,8 +9,6 @@ extern void __scsi_print_command(unsigned char *);
extern void scsi_show_extd_sense(unsigned char, unsigned char);
extern void scsi_show_sense_hdr(struct scsi_sense_hdr *);
extern void scsi_print_sense_hdr(const char *, struct scsi_sense_hdr *);
-extern void scsi_cmd_print_sense_hdr(struct scsi_cmnd *, const char *,
-struct scsi_sense_hdr *);
extern void scsi_print_sense(char *, struct scsi_cmnd *);
extern void __scsi_print_sense(const char *name,
const unsigned char *sense_buffer,
--
1.8.5.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 07/22] scsi: do not decode sense extras
Currently we're only decoding sense extras for tape devices.
And even there only for fixed format sense formats.
As this is of rather limited use in the general case we should
be stop trying to decode things here and rather dump the entire
sense code.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 63 +---
1 file changed, 1 insertion(+), 62 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 6b05575..f0a6595 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1447,67 +1447,6 @@ scsi_dump_sense_buffer(struct scsi_device *sdev, const
char *name,
}
}
-static void
-scsi_decode_sense_extras(const unsigned char *sense_buffer, int sense_len,
-struct scsi_sense_hdr *sshdr)
-{
- int k, num, res;
-
- if (sshdr-response_code 0x72)
- {
- /* only decode extras for fixed format now */
- char buff[80];
- int blen, fixed_valid;
- unsigned int info;
-
- fixed_valid = sense_buffer[0] 0x80;
- info = ((sense_buffer[3] 24) | (sense_buffer[4] 16) |
- (sense_buffer[5] 8) | sense_buffer[6]);
- res = 0;
- memset(buff, 0, sizeof(buff));
- blen = sizeof(buff) - 1;
- if (fixed_valid)
- res += snprintf(buff + res, blen - res,
- Info fld=0x%x, info);
- if (sense_buffer[2] 0x80) {
- /* current command has read a filemark */
- if (res 0)
- res += snprintf(buff + res, blen - res, , );
- res += snprintf(buff + res, blen - res, FMK);
- }
- if (sense_buffer[2] 0x40) {
- /* end-of-medium condition exists */
- if (res 0)
- res += snprintf(buff + res, blen - res, , );
- res += snprintf(buff + res, blen - res, EOM);
- }
- if (sense_buffer[2] 0x20) {
- /* incorrect block length requested */
- if (res 0)
- res += snprintf(buff + res, blen - res, , );
- res += snprintf(buff + res, blen - res, ILI);
- }
- if (res 0)
- printk(%s\n, buff);
- } else if (sshdr-additional_length 0) {
- /* descriptor format with sense descriptors */
- num = 8 + sshdr-additional_length;
- num = (sense_len num) ? sense_len : num;
- printk(Descriptor sense data with sense descriptors
- (in hex):);
- for (k = 0; k num; ++k) {
- if (0 == (k % 16)) {
- printk(\n);
- printk(KERN_INFO );
- }
- printk(%02x , sense_buffer[k]);
- }
-
- printk(\n);
- }
-
-}
-
/* Normalize and print sense buffer with name prefix */
void __scsi_print_sense(struct scsi_device *sdev, const char *name,
const unsigned char *sense_buffer, int sense_len)
@@ -1521,8 +1460,8 @@ void __scsi_print_sense(struct scsi_device *sdev, const
char *name,
return;
}
scsi_show_sense_hdr(sdev, name, sshdr);
- scsi_decode_sense_extras(sense_buffer, sense_len, sshdr);
scsi_show_extd_sense(sdev, name, sshdr.asc, sshdr.ascq);
+ scsi_dump_sense_buffer(sdev, name, sense_buffer, sense_len);
}
EXPORT_SYMBOL(__scsi_print_sense);
--
1.8.5.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 09/22] Use sdev as argument for scsi_print_result
Passing in the SCSI device will tag the logging message with
the originating device.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 28 ++--
drivers/scsi/sd.c| 3 +--
include/scsi/scsi_dbg.h | 2 +-
3 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index ecce5b3..c0d7b3d 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1503,31 +1503,39 @@ static const char * const driverbyte_table[]={
DRIVER_INVALID, DRIVER_TIMEOUT, DRIVER_HARD, DRIVER_SENSE};
#define NUM_DRIVERBYTE_STRS ARRAY_SIZE(driverbyte_table)
-void scsi_show_result(int result)
+void scsi_show_result(struct scsi_device *sdev, const char *name, int result)
{
int hb = host_byte(result);
int db = driver_byte(result);
+ const char *hb_string;
+ const char *db_string;
- printk(Result: hostbyte=%s driverbyte=%s\n,
- (hb NUM_HOSTBYTE_STRS ? hostbyte_table[hb] : invalid),
- (db NUM_DRIVERBYTE_STRS ? driverbyte_table[db] : invalid));
+ hb_string = (hb NUM_HOSTBYTE_STRS) ? hostbyte_table[hb] : invalid;
+ db_string = (db NUM_DRIVERBYTE_STRS) ?
+ driverbyte_table[db] : invalid;
+
+
+ sdev_prefix_printk(KERN_INFO, sdev, name,
+ Result: hostbyte=%s driverbyte=%s\n,
+ hb_string, db_string);
}
#else
-void scsi_show_result(int result)
+void scsi_show_result(struct scsi_device *sdev, const char *name, int result)
{
- printk(Result: hostbyte=0x%02x driverbyte=0x%02x\n,
- host_byte(result), driver_byte(result));
+ sdev_prefix_printk(KERN_INFO, sdev, name,
+ Result: hostbyte=0x%02x driverbyte=0x%02x\n,
+ host_byte(result), driver_byte(result));
}
#endif
EXPORT_SYMBOL(scsi_show_result);
-
void scsi_print_result(struct scsi_cmnd *cmd)
{
- scmd_printk(KERN_INFO, cmd, );
- scsi_show_result(cmd-result);
+ const char *devname = cmd-request-rq_disk ?
+ cmd-request-rq_disk-disk_name : NULL;
+ scsi_show_result(cmd-device, devname, cmd-result);
}
EXPORT_SYMBOL(scsi_print_result);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index aac267a..e780644 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3328,7 +3328,6 @@ static void sd_print_sense_hdr(struct scsi_disk *sdkp,
static void sd_print_result(struct scsi_disk *sdkp, int result)
{
- sd_printk(KERN_INFO, sdkp, );
- scsi_show_result(result);
+ scsi_show_result(sdkp-device, sdkp-disk-disk_name, result);
}
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 0bbf118..3d9ac9f 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -19,7 +19,7 @@ extern int __scsi_print_sense(struct scsi_device *, const
char *,
extern void scsi_dump_sense(struct scsi_cmnd *);
extern void __scsi_dump_sense(struct scsi_device *, const char *,
const unsigned char *, int);
-extern void scsi_show_result(int);
+extern void scsi_show_result(struct scsi_device *, const char *, int);
extern void scsi_print_result(struct scsi_cmnd *);
extern void scsi_print_status(unsigned char);
extern const char *scsi_sense_key_string(unsigned char);
--
1.8.5.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 02/22] aha152x: Remove #ifdef 0 section
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/aha152x.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index e77b72f..4da3a3b 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -1553,13 +1553,6 @@ static void busfree_run(struct Scsi_Host *shpnt)
struct scsi_cmnd *cmd = HOSTDATA(shpnt)-done_SC;
struct aha152x_scdata *sc = SCDATA(cmd);
-#if 0
- if(HOSTDATA(shpnt)-debug debug_eh) {
- printk(ERR_LEAD received sense: ,
CMDINFO(DONE_SC));
- scsi_print_sense(bh, DONE_SC);
- }
-#endif
-
scsi_eh_restore_cmnd(cmd, sc-ses);
cmd-SCp.Status = SAM_STAT_CHECK_CONDITION;
--
1.8.5.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 11/22] Implement scsi_opcode_sa_name
Implement a lookup array for SERVICE ACTION commands instead
of hardcoding it in a large switch statement.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 130 +++
1 file changed, 53 insertions(+), 77 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 323e944..813c482 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -244,102 +244,77 @@ static const struct value_name_pair
variable_length_arr[] = {
};
#define VARIABLE_LENGTH_SZ ARRAY_SIZE(variable_length_arr)
-static const char * get_sa_name(const struct value_name_pair * arr,
- int arr_sz, int service_action)
+struct sa_name_list {
+ int cmd;
+ const struct value_name_pair *arr;
+ int arr_sz;
+};
+
+static struct sa_name_list sa_names_arr[] = {
+ {VARIABLE_LENGTH_CMD, variable_length_arr, VARIABLE_LENGTH_SZ},
+ {MAINTENANCE_IN, maint_in_arr, MAINT_IN_SZ},
+ {MAINTENANCE_OUT, maint_out_arr, MAINT_OUT_SZ},
+ {PERSISTENT_RESERVE_IN, pr_in_arr, PR_IN_SZ},
+ {PERSISTENT_RESERVE_OUT, pr_out_arr, PR_OUT_SZ},
+ {SERVICE_ACTION_IN_12, serv_in12_arr, SERV_IN12_SZ},
+ {SERVICE_ACTION_OUT_12, serv_out12_arr, SERV_OUT12_SZ},
+ {SERVICE_ACTION_BIDIRECTIONAL, serv_bidi_arr, SERV_BIDI_SZ},
+ {SERVICE_ACTION_IN_16, serv_in16_arr, SERV_IN16_SZ},
+ {SERVICE_ACTION_OUT_16, serv_out16_arr, SERV_OUT16_SZ},
+ {THIRD_PARTY_COPY_IN, tpc_in_arr, TPC_IN_SZ},
+ {THIRD_PARTY_COPY_OUT, tpc_out_arr, TPC_OUT_SZ},
+ {0, NULL, 0},
+};
+#define SA_NAME_LIST_SZ ARRAY_SIZE(sa_names_arr)
+
+static int scsi_opcode_sa_name(int cmd, int service_action,
+ const char **sa_name)
{
- int k;
+ struct sa_name_list *sa_name_ptr = sa_names_arr;
+ const struct value_name_pair * arr = NULL;
+ int arr_sz, k;
+
+ for (k = 0; k SA_NAME_LIST_SZ; ++k, ++sa_name_ptr) {
+ if (sa_name_ptr-cmd == cmd) {
+ arr = sa_name_ptr-arr;
+ arr_sz = sa_name_ptr-arr_sz;
+ break;
+ }
+ }
+ if (!arr)
+ return 0;
for (k = 0; k arr_sz; ++k, ++arr) {
if (service_action == arr-value)
break;
}
- return (k arr_sz) ? arr-name : NULL;
+ if (k arr_sz)
+ *sa_name = arr-name;
+
+ return 1;
}
/* attempt to guess cdb length if cdb_len==0 . No trailing linefeed. */
static void print_opcode_name(unsigned char * cdbp, int cdb_len)
{
int sa, len, cdb0;
- int fin_name = 0;
const char * name;
cdb0 = cdbp[0];
- switch(cdb0) {
- case VARIABLE_LENGTH_CMD:
+ if (cdb0 == VARIABLE_LENGTH_CMD) {
len = scsi_varlen_cdb_length(cdbp);
if (len 10) {
printk(short variable length command,
len=%d ext_len=%d, len, cdb_len);
- break;
+ return;
}
sa = (cdbp[8] 8) + cdbp[9];
- name = get_sa_name(variable_length_arr, VARIABLE_LENGTH_SZ,
- sa);
- if (name)
- printk(%s, name);
- else
- printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa);
-
- if ((cdb_len 0) (len != cdb_len))
- printk(, in_cdb_len=%d, ext_len=%d, len, cdb_len);
-
- break;
- case MAINTENANCE_IN:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(maint_in_arr, MAINT_IN_SZ, sa);
- fin_name = 1;
- break;
- case MAINTENANCE_OUT:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(maint_out_arr, MAINT_OUT_SZ, sa);
- fin_name = 1;
- break;
- case PERSISTENT_RESERVE_IN:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(pr_in_arr, PR_IN_SZ, sa);
- fin_name = 1;
- break;
- case PERSISTENT_RESERVE_OUT:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(pr_out_arr, PR_OUT_SZ, sa);
- fin_name = 1;
- break;
- case SERVICE_ACTION_IN_12:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(serv_in12_arr, SERV_IN12_SZ, sa);
- fin_name = 1;
- break;
- case SERVICE_ACTION_OUT_12:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(serv_out12_arr, SERV_OUT12_SZ, sa);
- fin_name = 1;
- break;
- case SERVICE_ACTION_BIDIRECTIONAL:
- sa = cdbp[1] 0x1f;
- name = get_sa_name(serv_bidi_arr, SERV_BIDI_SZ, sa);
- fin_name = 1;
- break;
- case
[PATCH 04/22] scsi: introduce sdev_prefix_printk()
Signed-off-by: Hannes Reinecke h...@suse.de
---
include/scsi/scsi_device.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 1a0d184..91ac2e6 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -243,6 +243,11 @@ struct scsi_dh_data {
#define sdev_dbg(sdev, fmt, a...) \
dev_dbg((sdev)-sdev_gendev, fmt, ##a)
+#define sdev_prefix_printk(l, sdev, p, fmt, a...) \
+ (p) ? \
+ sdev_printk(l, sdev, [%s] fmt, p, ##a) : \
+ sdev_printk(l, sdev, fmt, ##a)
+
#define scmd_printk(prefix, scmd, fmt, a...) \
(scmd)-request-rq_disk ? \
sdev_printk(prefix, (scmd)-device, [%s] fmt,\
--
1.8.5.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 17/22] scsi: print disposition in scsi_print_result()
Print the result disposition in scsi_print_result() to simplify
code.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 43 +--
drivers/scsi/scsi.c | 28 +---
drivers/scsi/scsi_lib.c | 2 +-
drivers/scsi/sd.c| 6 --
include/scsi/scsi_dbg.h | 4 ++--
5 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 771cd8e..c59d128 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1501,6 +1501,33 @@ int scsi_print_sense(struct scsi_cmnd *cmd)
EXPORT_SYMBOL(scsi_print_sense);
#ifdef CONFIG_SCSI_CONSTANTS
+static const struct error_info disposition_table[] =
+{
+ { NEEDS_RETRY, NEEDS_RETRY },
+ { SUCCESS, SUCCESS },
+ { FAILED, FAILED },
+ { QUEUED, QUEUED },
+ { SOFT_ERROR, SOFT_ERROR },
+ { ADD_TO_MLQUEUE, ADD_TO_MLQUEUE },
+ { TIMEOUT_ERROR, TIMEOUT },
+ { SCSI_RETURN_NOT_HANDLED, NOT_HANDLED },
+ { FAST_IO_FAIL, FAST_IO_FAIL },
+};
+#endif
+
+const char *
+scsi_disposition_string(unsigned int disposition) {
+#ifdef CONFIG_SCSI_CONSTANTS
+ int i;
+
+ for (i = 0; disposition_table[i].text; i++)
+ if (disposition_table[i].code12 == disposition)
+ return disposition_table[i].text;
+#endif
+ return NULL;
+}
+
+#ifdef CONFIG_SCSI_CONSTANTS
static const char * const hostbyte_table[]={
DID_OK, DID_NO_CONNECT, DID_BUS_BUSY, DID_TIME_OUT, DID_BAD_TARGET,
@@ -1515,7 +1542,8 @@ static const char * const driverbyte_table[]={
DRIVER_INVALID, DRIVER_TIMEOUT, DRIVER_HARD, DRIVER_SENSE};
#define NUM_DRIVERBYTE_STRS ARRAY_SIZE(driverbyte_table)
-void scsi_show_result(struct scsi_device *sdev, const char *name, int result)
+void scsi_show_result(struct scsi_device *sdev, const char *name,
+ int result, int disposition)
{
int hb = host_byte(result);
int db = driver_byte(result);
@@ -1528,26 +1556,29 @@ void scsi_show_result(struct scsi_device *sdev, const
char *name, int result)
sdev_prefix_printk(KERN_INFO, sdev, name,
- Result: hostbyte=%s driverbyte=%s\n,
+ %sResult: hostbyte=%s driverbyte=%s\n,
+ scsi_disposition_string(disposition),
hb_string, db_string);
}
#else
-void scsi_show_result(struct scsi_device *sdev, const char *name, int result)
+void scsi_show_result(struct scsi_device *sdev, const char *name,
+ int result, int disposition)
{
sdev_prefix_printk(KERN_INFO, sdev, name,
- Result: hostbyte=0x%02x driverbyte=0x%02x\n,
+ %sResult: hostbyte=0x%02x driverbyte=0x%02x\n,
+ scsi_disposition_string(disposition),
host_byte(result), driver_byte(result));
}
#endif
EXPORT_SYMBOL(scsi_show_result);
-void scsi_print_result(struct scsi_cmnd *cmd)
+void scsi_print_result(struct scsi_cmnd *cmd, int disposition)
{
const char *devname = cmd-request-rq_disk ?
cmd-request-rq_disk-disk_name : NULL;
- scsi_show_result(cmd-device, devname, cmd-result);
+ scsi_show_result(cmd-device, devname, cmd-result, disposition);
}
EXPORT_SYMBOL(scsi_print_result);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 283f053..d267e61 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -569,33 +569,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int
disposition)
scmd_printk(KERN_INFO, cmd, Done: );
if (level 2)
printk(0x%p , cmd);
- /*
-* Dump truncated values, so we usually fit within
-* 80 chars.
-*/
- switch (disposition) {
- case SUCCESS:
- printk(SUCCESS\n);
- break;
- case NEEDS_RETRY:
- printk(RETRY\n);
- break;
- case ADD_TO_MLQUEUE:
- printk(MLQUEUE\n);
- break;
- case FAILED:
- printk(FAILED\n);
- break;
- case TIMEOUT_ERROR:
- /*
-* If called via scsi_times_out.
-*/
- printk(TIMEOUT\n);
- break;
- default:
- printk(UNKNOWN\n);
- }
- scsi_print_result(cmd);
+
[PATCH 12/22] scsi: remove obsolete __scsi_print_command() usages
Some drivers still use the __scsi_print_command() function although
they infact refer to the scsi command. So move them over to use
scsi_print_command() instead.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/aha152x.c | 18 --
drivers/scsi/arm/acornscsi.c | 9 +
drivers/scsi/arm/fas216.c| 30 +-
3 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 4287f86..68af777 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -988,10 +988,11 @@ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt,
struct completion *complete,
#if defined(AHA152X_DEBUG)
if (HOSTDATA(shpnt)-debug debug_queue) {
- printk(INFO_LEAD queue: %p; cmd_len=%d pieces=%d size=%u
cmnd=,
- CMDINFO(SCpnt), SCpnt, SCpnt-cmd_len,
- scsi_sg_count(SCpnt), scsi_bufflen(SCpnt));
- __scsi_print_command(SCpnt-cmnd);
+ scmd_printk(KERN_INFO, SCpnt,
+ queue: %p; cmd_len=%d pieces=%d size=%u\n,
+ SCpnt, SCpnt-cmd_len,
+ scsi_sg_count(SCpnt), scsi_bufflen(SCpnt));
+ scsi_print_command(SCpnt);
}
#endif
@@ -2077,8 +2078,7 @@ static void cmd_init(struct Scsi_Host *shpnt)
#if defined(AHA152X_DEBUG)
if (HOSTDATA(shpnt)-debug debug_cmd) {
- printk(DEBUG_LEAD cmd_init: , CMDINFO(CURRENT_SC));
- __scsi_print_command(CURRENT_SC-cmnd);
+ scsi_print_command(CURRENT_SC);
}
#endif
@@ -2906,11 +2906,9 @@ static void disp_enintr(struct Scsi_Host *shpnt)
*/
static void show_command(Scsi_Cmnd *ptr)
{
- scmd_printk(KERN_DEBUG, ptr, %p: cmnd=(, ptr);
+ scsi_print_command(ptr);
- __scsi_print_command(ptr-cmnd);
-
- printk(KERN_DEBUG ); request_bufflen=%d; resid=%d; phase |,
+ scmd_printk(KERN_DEBUG, ptr, request_bufflen=%d; resid=%d; phase |,
scsi_bufflen(ptr), scsi_get_resid(ptr));
if (ptr-SCp.phase not_issued)
diff --git a/drivers/scsi/arm/acornscsi.c b/drivers/scsi/arm/acornscsi.c
index d89b9b4..78dd881 100644
--- a/drivers/scsi/arm/acornscsi.c
+++ b/drivers/scsi/arm/acornscsi.c
@@ -850,11 +850,12 @@ static void acornscsi_done(AS_Host *host, struct
scsi_cmnd **SCpntp,
break;
default:
- printk(KERN_ERR scsi%d.H: incomplete data transfer
detected: result=%08X command=,
- host-host-host_no, SCpnt-result);
- __scsi_print_command(SCpnt-cmnd);
+ scmd_printk(KERN_ERR, SCPnt,
+ incomplete data transfer detected:
+result=%08X\n, SCpnt-result);
+ scsi_print_command(SCpnt);
acornscsi_dumpdma(host, done);
- acornscsi_dumplog(host, SCpnt-device-id);
+ acornscsi_dumplog(host, SCpnt-device-id);
SCpnt-result = 0x;
SCpnt-result |= DID_ERROR 16;
}
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index 71cfb1e..778fd36 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -308,8 +308,7 @@ static void fas216_log_command(FAS216_Info *info, int level,
fas216_do_log(info, '0' + SCpnt-device-id, fmt, args);
va_end(args);
- printk( CDB: );
- __scsi_print_command(SCpnt-cmnd);
+ scsi_print_command(SCpnt);
}
static void
@@ -2079,12 +2078,12 @@ fas216_std_done(FAS216_Info *info, struct scsi_cmnd
*SCpnt, unsigned int result)
break;
default:
- printk(KERN_ERR scsi%d.%c: incomplete data transfer
- detected: res=%08X ptr=%p len=%X CDB: ,
- info-host-host_no, '0' + SCpnt-device-id,
- SCpnt-result, info-scsi.SCp.ptr,
- info-scsi.SCp.this_residual);
- __scsi_print_command(SCpnt-cmnd);
+ scmd_printk(KERN_ERR, SCpnt,
+ incomplete data transfer
+ detected: res=%08X ptr=%p len=%X\n,
+ SCpnt-result, info-scsi.SCp.ptr,
+ info-scsi.SCp.this_residual);
+ scsi_print_command(SCpnt-cmnd);
SCpnt-result = ~(255 16);
SCpnt-result |= DID_BAD_TARGET 16;
goto request_sense;
@@ -2158,12 +2157,11 @@ static void fas216_done(FAS216_Info *info, unsigned int
result)
* to transfer, we should not have a valid pointer.
*/
if
[PATCH 20/22] scsi: align logging messages
Always use 'scmd 0x%p' when logging a command pointer.
Suggested-by: Robert Elliot ell...@hp.com
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/scsi.c | 7 +++---
drivers/scsi/scsi_error.c | 58 +++
drivers/scsi/scsi_lib.c | 2 +-
3 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 58e2d7f..7e677d0 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -533,14 +533,15 @@ void scsi_log_send(struct scsi_cmnd *cmd)
__scsi_print_command(cmd-cmnd, buf, 80);
if (level 2)
- scmd_printk(KERN_INFO, cmd, Send: 0x%p %s\n,
+ scmd_printk(KERN_INFO, cmd,
+ Send: scmd 0x%p %s\n,
cmd, buf);
else
scmd_printk(KERN_INFO, cmd, Send: %s\n, buf);
if (level 3) {
struct Scsi_Host *shost = cmd-device-host;
scmd_printk(KERN_INFO, cmd,
- Send: 0x%p buffer = 0x%p,
+ Send: scmd 0x%p buffer = 0x%p,
bufflen = %d,
queuecommand 0x%p\n,
cmd, scsi_sglist(cmd),
@@ -577,7 +578,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int
disposition)
__scsi_print_command(cmd-cmnd, buf, 80);
if (level 2)
scmd_printk(KERN_INFO, cmd,
- Done: 0x%p %s\n, cmd, buf);
+ Done: scmd 0x%p %s\n, cmd, buf);
else
scmd_printk(KERN_INFO, cmd, Done: %s\n, buf);
scsi_print_result(cmd, disposition);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 07887b1..5736570 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -123,33 +123,33 @@ scmd_eh_abort_handler(struct work_struct *work)
if (scsi_host_eh_past_deadline(sdev-host)) {
SCSI_LOG_ERROR_RECOVERY(3,
scmd_printk(KERN_INFO, scmd,
- scmd %p eh timeout, not aborting\n,
+ scmd 0x%p eh timeout, not aborting\n,
scmd));
} else {
- SCSI_LOG_ERROR_RECOVERY(3,
+ SCSI_LOG_ERROR_RECOVERY(2,
scmd_printk(KERN_INFO, scmd,
- aborting command %p\n, scmd));
+ aborting scmd 0x%p\n, scmd));
rtn = scsi_try_to_abort_cmd(sdev-host-hostt, scmd);
if (rtn == SUCCESS) {
set_host_byte(scmd, DID_TIME_OUT);
if (scsi_host_eh_past_deadline(sdev-host)) {
SCSI_LOG_ERROR_RECOVERY(3,
scmd_printk(KERN_INFO, scmd,
- scmd %p eh timeout,
+ scmd 0x%p eh timeout,
not retrying aborted
command\n, scmd));
} else if (!scsi_noretry_cmd(scmd)
(++scmd-retries = scmd-allowed)) {
SCSI_LOG_ERROR_RECOVERY(3,
scmd_printk(KERN_WARNING, scmd,
- scmd %p retry
+ scmd 0x%p retry
aborted command\n, scmd));
scsi_queue_insert(scmd, SCSI_MLQUEUE_EH_RETRY);
return;
} else {
SCSI_LOG_ERROR_RECOVERY(3,
scmd_printk(KERN_WARNING, scmd,
- scmd %p finish
+ scmd 0x%p finish
aborted command\n, scmd));
scsi_finish_command(scmd);
return;
@@ -157,23 +157,23 @@ scmd_eh_abort_handler(struct work_struct *work)
} else {
const char *rtn_str = scsi_disposition_string(rtn);
if (rtn_str) {
- SCSI_LOG_ERROR_RECOVERY(3,
+
[PATCH 03/22] sd: Remove scsi_print_sense() in sd_done()
sd_done() was calling scsi_print_sense() for a sense code of 'NO_SENSE'. Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/sd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index aa43496..f5ca203 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1730,7 +1730,6 @@ static int sd_done(struct scsi_cmnd *SCpnt) * unknown amount of data was transferred so treat it as an * error. */ - scsi_print_sense(sd, SCpnt); SCpnt-result = 0; memset(SCpnt-sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); break; -- 1.8.5.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 21/22] scsi: reduce messages for command failure
When devices fail they can generate quite a lot of error
messages, possibly overloading the logging system.
So we should be putting these messages under logging control
to be able to silence them if requested.
And we should shorten the overall message; more verbose
logging can be requested by the normal scsi logging.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/scsi_lib.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ed5e40f..64f8e33 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1038,10 +1038,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
case ACTION_FAIL:
/* Give up and fail the remainder of the request */
if (!(req-cmd_flags REQ_QUIET)) {
- scsi_print_result(cmd, SUCCESS);
- if (driver_byte(result) DRIVER_SENSE)
- scsi_print_sense(cmd);
- scsi_print_command(cmd);
+ SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, cmd,
+ Failed command, error %d\n, error));
}
if (!scsi_end_request(req, error, blk_rq_err_bytes(req), 0))
return;
--
1.8.5.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 05/22] scsi: Use sdev as argument for sense code printing
We should be using the standard dev_printk() variants for
sense code printing.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/53c700.c | 2 +-
drivers/scsi/ch.c | 2 +-
drivers/scsi/constants.c | 113 +
drivers/scsi/osst.c| 8 ++--
drivers/scsi/scsi.c| 2 +-
drivers/scsi/scsi_error.c | 2 +-
drivers/scsi/scsi_ioctl.c | 2 +-
drivers/scsi/scsi_lib.c| 4 +-
drivers/scsi/sd.c | 9 ++--
drivers/scsi/sg.c | 2 +-
drivers/scsi/sr_ioctl.c| 6 +--
drivers/scsi/st.c | 6 ++-
drivers/scsi/storvsc_drv.c | 3 +-
include/scsi/scsi_dbg.h| 17 ---
14 files changed, 91 insertions(+), 87 deletions(-)
diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index fabd4be..68bf423 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -602,7 +602,7 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata,
#ifdef NCR_700_DEBUG
printk( ORIGINAL CMD %p RETURNED %d, new return is %d
sense is\n,
SCp, SCp-cmnd[7], result);
- scsi_print_sense(53c700, SCp);
+ scsi_print_sense(SCp);
#endif
dma_unmap_single(hostdata-dev, slot-dma_handle,
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index ef5ae0d..eea94a9 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -207,7 +207,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
DPRINTK(result: 0x%x\n,result);
if (driver_byte(result) DRIVER_SENSE) {
if (debug)
- scsi_print_sense_hdr(ch-name, sshdr);
+ scsi_print_sense_hdr(ch-device, ch-name, sshdr);
errno = ch_find_errno(sshdr);
switch(sshdr.sense_key) {
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 2f44707..36e1ffd 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1292,18 +1292,19 @@ static const struct error_info additional[] =
struct error_info2 {
unsigned char code1, code2_min, code2_max;
+ const char * str;
const char * fmt;
};
static const struct error_info2 additional2[] =
{
- {0x40, 0x00, 0x7f, Ram failure (%x)},
- {0x40, 0x80, 0xff, Diagnostic failure on component (%x)},
- {0x41, 0x00, 0xff, Data path failure (%x)},
- {0x42, 0x00, 0xff, Power-on or self-test failure (%x)},
- {0x4D, 0x00, 0xff, Tagged overlapped commands (task tag %x)},
- {0x70, 0x00, 0xff, Decompression exception short algorithm id of %x},
- {0, 0, 0, NULL}
+ {0x40, 0x00, 0x7f, Ram failure, },
+ {0x40, 0x80, 0xff, Diagnostic failure on component, },
+ {0x41, 0x00, 0xff, Data path failure, },
+ {0x42, 0x00, 0xff, Power-on or self-test failure, },
+ {0x4D, 0x00, 0xff, Tagged overlapped commands, task tag },
+ {0x70, 0x00, 0xff, Decompression exception, short algorithm id of },
+ {0, 0, 0, NULL, NULL}
};
/* description of the sense key values */
@@ -1349,7 +1350,8 @@ EXPORT_SYMBOL(scsi_sense_key_string);
* This string may contain a %x and should be printed with ascq as arg.
*/
const char *
-scsi_extd_sense_format(unsigned char asc, unsigned char ascq) {
+scsi_extd_sense_format(unsigned char asc, unsigned char ascq,
+ const char **fmt) {
#ifdef CONFIG_SCSI_CONSTANTS
int i;
unsigned short code = ((asc 8) | ascq);
@@ -1361,7 +1363,8 @@ scsi_extd_sense_format(unsigned char asc, unsigned char
ascq) {
if (additional2[i].code1 == asc
ascq = additional2[i].code2_min
ascq = additional2[i].code2_max)
- return additional2[i].fmt;
+ *fmt = additional2[i].fmt;
+ return additional2[i].str;
}
#endif
return NULL;
@@ -1369,49 +1372,47 @@ scsi_extd_sense_format(unsigned char asc, unsigned char
ascq) {
EXPORT_SYMBOL(scsi_extd_sense_format);
void
-scsi_show_extd_sense(unsigned char asc, unsigned char ascq)
+scsi_show_extd_sense(struct scsi_device *sdev, const char *name,
+unsigned char asc, unsigned char ascq)
{
-const char *extd_sense_fmt = scsi_extd_sense_format(asc, ascq);
-
- if (extd_sense_fmt) {
- if (strstr(extd_sense_fmt, %x)) {
- printk(Add. Sense: );
- printk(extd_sense_fmt, ascq);
- } else
- printk(Add. Sense: %s, extd_sense_fmt);
+ const char *extd_sense_fmt = NULL;
+ const char *extd_sense_str = scsi_extd_sense_format(asc, ascq,
+ extd_sense_str);
+
+ if (extd_sense_str) {
+ sdev_prefix_printk(KERN_INFO, sdev, name,
+ Add. Sense: %s (%s%x),
+
[PATCH 16/22] libata: use __scsi_print_command()
libata already uses an internal buffer, so we should be using
__scsi_print_command() here.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/ata/libata-eh.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index dad83df..acf06ba 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2509,15 +2509,11 @@ static void ata_eh_link_report(struct ata_link *link)
if (ata_is_atapi(qc-tf.protocol)) {
if (qc-scsicmd)
- scsi_print_command(qc-scsicmd);
+ __scsi_print_command(qc-scsicmd-cmnd,
+cdb_buf, sizeof(cdb_buf));
else
- snprintf(cdb_buf, sizeof(cdb_buf),
-cdb %02x %02x %02x %02x %02x %02x %02x %02x
-%02x %02x %02x %02x %02x %02x %02x %02x\n
,
-cdb[0], cdb[1], cdb[2], cdb[3],
-cdb[4], cdb[5], cdb[6], cdb[7],
-cdb[8], cdb[9], cdb[10], cdb[11],
-cdb[12], cdb[13], cdb[14], cdb[15]);
+ __scsi_print_command((unsigned char *)cdb,
+cdb_buf, sizeof(cdb_buf));
} else {
const char *descr = ata_get_cmd_descript(cmd-command);
if (descr)
--
1.8.5.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 19/22] scsi: use local buffer for scsi_log_(send|completion)
scsi_logging is somewhat special, and so instead of tweaking
scsi_print_command() we should be using __scsi_print_command()
with a local buffer, which then can be formatted according
to the logging level.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/scsi.c | 40
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d267e61..58e2d7f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -529,17 +529,23 @@ void scsi_log_send(struct scsi_cmnd *cmd)
level = SCSI_LOG_LEVEL(SCSI_LOG_MLQUEUE_SHIFT,
SCSI_LOG_MLQUEUE_BITS);
if (level 1) {
- scmd_printk(KERN_INFO, cmd, Send: );
+ char buf[80];
+
+ __scsi_print_command(cmd-cmnd, buf, 80);
if (level 2)
- printk(0x%p , cmd);
- printk(\n);
- scsi_print_command(cmd);
+ scmd_printk(KERN_INFO, cmd, Send: 0x%p %s\n,
+ cmd, buf);
+ else
+ scmd_printk(KERN_INFO, cmd, Send: %s\n, buf);
if (level 3) {
- printk(KERN_INFO buffer = 0x%p, bufflen = %d,
- queuecommand 0x%p\n,
- scsi_sglist(cmd), scsi_bufflen(cmd),
- cmd-device-host-hostt-queuecommand);
-
+ struct Scsi_Host *shost = cmd-device-host;
+ scmd_printk(KERN_INFO, cmd,
+ Send: 0x%p buffer = 0x%p,
+bufflen = %d,
+queuecommand 0x%p\n,
+ cmd, scsi_sglist(cmd),
+ scsi_bufflen(cmd),
+ shost-hostt-queuecommand);
}
}
}
@@ -566,15 +572,17 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int
disposition)
SCSI_LOG_MLCOMPLETE_BITS);
if (((level 0) (cmd-result || disposition != SUCCESS)) ||
(level 1)) {
- scmd_printk(KERN_INFO, cmd, Done: );
+ char buf[80];
+
+ __scsi_print_command(cmd-cmnd, buf, 80);
if (level 2)
- printk(0x%p , cmd);
+ scmd_printk(KERN_INFO, cmd,
+ Done: 0x%p %s\n, cmd, buf);
+ else
+ scmd_printk(KERN_INFO, cmd, Done: %s\n, buf);
scsi_print_result(cmd, disposition);
- scsi_print_command(cmd);
- if (status_byte(cmd-result) CHECK_CONDITION) {
- if (scsi_print_sense(cmd))
- scsi_dump_sense(cmd);
- }
+ if (status_byte(cmd-result) CHECK_CONDITION)
+ scsi_print_sense(cmd);
if (level 3)
scmd_printk(KERN_INFO, cmd,
scsi host busy %d failed %d\n,
--
1.8.5.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 15/22] scsi: use dev_printk() variants in scsi_print_command()
We already have a local buffer, so we can use it to format
the cdb, too.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index c74cb85..771cd8e 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -412,18 +412,30 @@ EXPORT_SYMBOL(__scsi_print_command);
void scsi_print_command(struct scsi_cmnd *cmd)
{
char buf[80];
- int k, off = 0;
+ int k, off = 0, linelen, remaining = cmd-cmd_len;
if (cmd-cmnd == NULL)
return;
off = print_opcode_name(cmd-cmnd, cmd-cmd_len, buf, 80);
- scmd_printk(KERN_INFO, cmd, CDB: %s:, buf);
+ if (cmd-cmd_len = 16) {
+ strcat(buf, CDB:);
+ off += 5;
- /* print out all bytes in cdb */
- for (k = 0; k cmd-cmd_len; ++k)
- printk( %02x, cmd-cmnd[k]);
- printk(\n);
+ hex_dump_to_buffer(cmd-cmnd, cmd-cmd_len, 16, 1,
+ buf + off, sizeof(buf) - off, false);
+ scmd_printk(KERN_INFO, cmd, %s\n, buf);
+ } else {
+ scmd_printk(KERN_INFO, cmd, %s:\n, buf);
+ /* print out all bytes in cdb */
+ for (k = 0; k cmd-cmd_len; k += 16) {
+ linelen = min(remaining, 16);
+ remaining -= 16;
+ hex_dump_to_buffer(cmd-cmnd + k, linelen, 16, 1,
+ buf, sizeof(buf), false);
+ scmd_printk(KERN_INFO, cmd, CDB: %s\n, buf);
+ }
+ }
}
EXPORT_SYMBOL(scsi_print_command);
--
1.8.5.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 22/22] sd: Reduce logging output
There is no need to print out the command result verbatim;
that will be done by the scsi stack if required.
Here we just should log the result in short if requested.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/sd.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 624692c..3061acf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1701,14 +1701,15 @@ static int sd_done(struct scsi_cmnd *SCpnt)
sense_deferred = scsi_sense_is_deferred(sshdr);
}
#ifdef CONFIG_SCSI_LOGGING
- SCSI_LOG_HLCOMPLETE(1, scsi_print_result(SCpnt, SUCCESS));
+ SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
+ sd_done: result[status,msg,host,driver]=%x,%x,%x,%x\n,
+ status_byte(result), msg_byte(result),
+ host_byte(result), driver_byte(result)));
if (sense_valid) {
SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
- sd_done: sb[respc,sk,asc,
- ascq]=%x,%x,%x,%x\n,
- sshdr.response_code,
- sshdr.sense_key, sshdr.asc,
- sshdr.ascq));
+ sd_done: sb[respc,sk,asc,ascq]=%x,%x,%x,%x\n,
+ sshdr.response_code, sshdr.sense_key,
+ sshdr.asc, sshdr.ascq));
}
#endif
sdkp-medium_access_timed_out = 0;
--
1.8.5.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 13/22] scsi: use local buffer for printing the opcode
SCSI opcode printing is tricky and needs to take into account
several different corner cases. So instead of trying to come
up with an elaborate printk() statement we should be printing
it into a local buffer.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 72 ++--
1 file changed, 45 insertions(+), 27 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 813c482..e9c099d 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -295,18 +295,20 @@ static int scsi_opcode_sa_name(int cmd, int
service_action,
}
/* attempt to guess cdb length if cdb_len==0 . No trailing linefeed. */
-static void print_opcode_name(unsigned char * cdbp, int cdb_len)
+static int print_opcode_name(unsigned char * cdbp, int cdb_len,
+char *buf, int buf_len)
{
- int sa, len, cdb0;
- const char * name;
+ int sa, len, cdb0, off;
+ const char * name = NULL;
cdb0 = cdbp[0];
if (cdb0 == VARIABLE_LENGTH_CMD) {
len = scsi_varlen_cdb_length(cdbp);
if (len 10) {
- printk(short variable length command,
- len=%d ext_len=%d, len, cdb_len);
- return;
+ off = scnprintf(buf, buf_len,
+ short variable length command,
+ len=%d ext_len=%d, len, cdb_len);
+ return off;
}
sa = (cdbp[8] 8) + cdbp[9];
} else {
@@ -318,41 +320,51 @@ static void print_opcode_name(unsigned char * cdbp, int
cdb_len)
if (cdb0 0xc0) {
name = cdb_byte0_names[cdb0];
if (name)
- printk(%s, name);
+ off = scnprintf(buf, buf_len, %s, name);
else
- printk(cdb[0]=0x%x (reserved), cdb0);
+ off = scnprintf(buf, buf_len,
+ cdb[0]=0x%x (reserved), cdb0);
} else
- printk(cdb[0]=0x%x (vendor), cdb0);
+ off = scnprintf(buf, buf_len,
+ cdb[0]=0x%x (vendor), cdb0);
} else {
if (name)
- printk(%s, name);
+ off = scnprintf(buf, buf_len, %s, name);
else
- printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa);
+ off = scnprintf(buf, buf_len,
+ cdb[0]=0x%x, sa=0x%x, cdb0, sa);
if ((cdb_len 0) (len != cdb_len))
- printk(, in_cdb_len=%d, ext_len=%d, len, cdb_len);
+ off += scnprintf(buf + off, buf_len - off,
+, in_cdb_len=%d, ext_len=%d,
+len, cdb_len);
}
+ return off;
}
#else /* ifndef CONFIG_SCSI_CONSTANTS */
-static void print_opcode_name(unsigned char * cdbp, int cdb_len)
+static int print_opcode_name(unsigned char * cdbp, int cdb_len,
+char *buf, int buf_len)
{
- int sa, len, cdb0;
+ int sa, len, cdb0, off;
cdb0 = cdbp[0];
switch(cdb0) {
case VARIABLE_LENGTH_CMD:
len = scsi_varlen_cdb_length(cdbp);
if (len 10) {
- printk(short opcode=0x%x command, len=%d
- ext_len=%d, cdb0, len, cdb_len);
+ off = scnprintf(buf, buf_len,
+ short opcode=0x%x command, len=%d
+ ext_len=%d, cdb0, len, cdb_len);
break;
}
sa = (cdbp[8] 8) + cdbp[9];
- printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa);
+ off = scnprintf(buf, buf_len, cdb[0]=0x%x, sa=0x%x, cdb0, sa);
if (len != cdb_len)
- printk(, in_cdb_len=%d, ext_len=%d, len, cdb_len);
+ off += scnprintf(buf + off, buflen - off,
+, in_cdb_len=%d, ext_len=%d,
+len, cdb_len);
break;
case MAINTENANCE_IN:
case MAINTENANCE_OUT:
@@ -366,23 +378,29 @@ static void print_opcode_name(unsigned char * cdbp, int
cdb_len)
case THIRD_PARTY_COPY_IN:
case THIRD_PARTY_COPY_OUT:
sa = cdbp[1] 0x1f;
- printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa);
+ off = scnprintf(buf + off, buflen - off,
+ cdb[0]=0x%x, sa=0x%x, cdb0, sa);
break;
default:
if (cdb0 0xc0)
-
[PATCH 00/22] scsi logging update
Hi all, here's my next round of scsi logging updates. Main feature is the update to have all logging statements in one line so that they won't be broken up even under high load. This will dramatically improve debugging. Additionally all printk() statements are moved to dev_printk() variants to ensure proper device tagging and keep the systemd journal happy. To achieve this I had to use a on-stack buffer for formatting opcodes and sense codes; so the stack usage will increase somewhat. Reviews, comments etc are welcome. Hannes Reinecke (22): Remove scsi_cmd_print_sense_hdr() aha152x: Remove #ifdef 0 section sd: Remove scsi_print_sense() in sd_done() scsi: introduce sdev_prefix_printk() scsi: Use sdev as argument for sense code printing scsi: stop decoding if scsi_normalize_sense() fails scsi: do not decode sense extras scsi: dump sense buffer only for debugging Use sdev as argument for scsi_print_result scsi: consolidate scsi_print_status() Implement scsi_opcode_sa_name scsi: remove obsolete __scsi_print_command() usages scsi: use local buffer for printing the opcode scsi: pass in string buffer to __scsi_print_command() scsi: use dev_printk() variants in scsi_print_command() libata: use __scsi_print_command() scsi: print disposition in scsi_print_result() scsi_error: format abort error message scsi: use local buffer for scsi_log_(send|completion) scsi: align logging messages scsi: reduce messages for command failure sd: Reduce logging output drivers/ata/libata-eh.c | 12 +- drivers/scsi/53c700.c | 3 +- drivers/scsi/aha152x.c| 32 +-- drivers/scsi/arm/acornscsi.c | 9 +- drivers/scsi/arm/fas216.c | 30 +-- drivers/scsi/ch.c | 7 +- drivers/scsi/constants.c | 570 -- drivers/scsi/osst.c | 8 +- drivers/scsi/scsi.c | 65 ++--- drivers/scsi/scsi_error.c | 68 ++--- drivers/scsi/scsi_ioctl.c | 2 +- drivers/scsi/scsi_lib.c | 10 +- drivers/scsi/sd.c | 28 ++- drivers/scsi/sg.c | 9 +- drivers/scsi/sr_ioctl.c | 16 +- drivers/scsi/st.c | 13 +- drivers/scsi/storvsc_drv.c| 3 +- include/scsi/scsi_dbg.h | 34 +-- include/scsi/scsi_device.h| 5 + include/trace/events/scsi.h | 17 +- include/trace/events/target.h | 17 +- 21 files changed, 457 insertions(+), 501 deletions(-) -- 1.8.5.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 14/22] scsi: pass in string buffer to __scsi_print_command()
Instead of using an on-stack buffer for __scsi_print_command()
we should pass in the string buffer directly.
This allows us to simplify the caller.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/ch.c| 5 +++--
drivers/scsi/constants.c | 16
drivers/scsi/sr_ioctl.c | 10 +++---
include/scsi/scsi_dbg.h | 2 +-
4 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index eea94a9..ed023cc 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -189,6 +189,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
{
int errno, retries = 0, timeout, result;
struct scsi_sense_hdr sshdr;
+ char logbuf[80];
timeout = (cmd[0] == INITIALIZE_ELEMENT_STATUS)
? timeout_init : timeout_move;
@@ -196,8 +197,8 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
retry:
errno = 0;
if (debug) {
- DPRINTK(command: );
- __scsi_print_command(cmd);
+ __scsi_print_command(cmd, logbuf, 80);
+ DPRINTK(command: %s, logbuf);
}
result = scsi_execute_req(ch-device, cmd, direction, buffer,
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index e9c099d..c74cb85 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -394,18 +394,18 @@ static int print_opcode_name(unsigned char * cdbp, int
cdb_len,
}
#endif
-void __scsi_print_command(unsigned char *cdb)
+void __scsi_print_command(unsigned char *cdb, char *buf, int buf_len)
{
- char buf[80];
- int k, len, off = 0;
+ int len, off = 0;
- off = print_opcode_name(cdb, 0, buf, 80);
- printk(%s, buf);
+ off = print_opcode_name(cdb, 0, buf, buf_len);
len = scsi_command_size(cdb);
/* print out all bytes in cdb */
- for (k = 0; k len; ++k)
- printk( %02x, cdb[k]);
- printk(\n);
+ strcat(buf, : );
+ off += 2;
+
+ hex_dump_to_buffer(cdb, len, 32, 1,
+ buf + off, buf_len - off, false);
}
EXPORT_SYMBOL(__scsi_print_command);
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index 17e0c2b..3e82e66 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -188,6 +188,7 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
struct scsi_sense_hdr sshdr;
int result, err = 0, retries = 0;
struct request_sense *sense = cgc-sense;
+ char logbuf[80];
SDev = cd-device;
@@ -257,14 +258,17 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
/* sense: Invalid command operation code */
err = -EDRIVE_CANT_DO_THIS;
#ifdef DEBUG
- __scsi_print_command(cgc-cmd);
+ __scsi_print_command(cgc-cmd, logbuf, 80);
+ sr_printk(KERN_DEBUG, cd,
+ CDROM (ioctl) illegal request,
+ command: %s\n, logbuf);
scsi_print_sense_hdr(cd-device, cd-cdi.name, sshdr);
#endif
break;
default:
+ __scsi_print_command(cgc-cmd, logbuf, 80);
sr_printk(KERN_ERR, cd,
- CDROM (ioctl) error, command: );
- __scsi_print_command(cgc-cmd);
+ CDROM (ioctl) error, command: %s\n, logbuf);
scsi_print_sense_hdr(cd-device, cd-cdi.name, sshdr);
err = -EIO;
}
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index a46bc55..e682fe3 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -6,7 +6,7 @@ struct scsi_device;
struct scsi_sense_hdr;
extern void scsi_print_command(struct scsi_cmnd *);
-extern void __scsi_print_command(unsigned char *);
+extern void __scsi_print_command(unsigned char *, char *, int);
extern void scsi_show_extd_sense(struct scsi_device *, const char *,
unsigned char, unsigned char);
extern void scsi_show_sense_hdr(struct scsi_device *, const char *,
--
1.8.5.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 18/22] scsi_error: format abort error message
Suggested-by: Robert Elliot ell...@hp.com
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/scsi_error.c | 16
include/scsi/scsi_dbg.h | 1 +
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 6c99624..07887b1 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -155,10 +155,18 @@ scmd_eh_abort_handler(struct work_struct *work)
return;
}
} else {
- SCSI_LOG_ERROR_RECOVERY(3,
- scmd_printk(KERN_INFO, scmd,
- scmd %p abort failed, rtn %d\n,
- scmd, rtn));
+ const char *rtn_str = scsi_disposition_string(rtn);
+ if (rtn_str) {
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_INFO, scmd,
+ scmd %p abort failed,
+%s\n, scmd, rtn_str));
+ } else {
+ SCSI_LOG_ERROR_RECOVERY(3,
+ scmd_printk(KERN_INFO, scmd,
+ scmd %p abort failed,
+0x%x\n, scmd, rtn));
+ }
}
}
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index fa04587..d60e7d8 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -25,5 +25,6 @@ extern const char *scsi_print_status(unsigned char);
extern const char *scsi_sense_key_string(unsigned char);
extern const char *scsi_extd_sense_format(unsigned char, unsigned char,
const char **);
+extern const char *scsi_disposition_string(unsigned int);
#endif /* _SCSI_SCSI_DBG_H */
--
1.8.5.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 10/22] scsi: consolidate scsi_print_status()
Update scsi_print_status() to return a const string and remove
the open-coded versions.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/aha152x.c| 7 ++
drivers/scsi/constants.c | 50 ---
include/scsi/scsi_dbg.h | 2 +-
include/trace/events/scsi.h | 17 +--
include/trace/events/target.h | 17 ++-
5 files changed, 34 insertions(+), 59 deletions(-)
diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 4da3a3b..4287f86 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -2130,11 +2130,8 @@ static void status_run(struct Scsi_Host *shpnt)
CURRENT_SC-SCp.Status = GETPORT(SCSIDAT);
#if defined(AHA152X_DEBUG)
- if (HOSTDATA(shpnt)-debug debug_status) {
- printk(DEBUG_LEAD inbound status %02x , CMDINFO(CURRENT_SC),
CURRENT_SC-SCp.Status);
- scsi_print_status(CURRENT_SC-SCp.Status);
- printk(\n);
- }
+ if (HOSTDATA(shpnt)-debug debug_status)
+ printk(DEBUG_LEAD inbound status %02x\n, CMDINFO(CURRENT_SC),
CURRENT_SC-SCp.Status);
#endif
}
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index c0d7b3d..323e944 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -437,34 +437,40 @@ EXPORT_SYMBOL(scsi_print_command);
* scsi_print_status - print scsi status description
* @scsi_status: scsi status value
*
- * If the status is recognized, the description is printed.
- * Otherwise Unknown status is output. No trailing space.
- * If CONFIG_SCSI_CONSTANTS is not set, then print status in hex
- * (e.g. 0x2 for Check Condition).
+ * If the status is recognized, the description is returned.
+ * Otherwise Unknown status is returned.
**/
-void
+const char *
scsi_print_status(unsigned char scsi_status) {
-#ifdef CONFIG_SCSI_CONSTANTS
const char * ccp;
switch (scsi_status) {
- case 0:ccp = Good; break;
- case 0x2: ccp = Check Condition; break;
- case 0x4: ccp = Condition Met; break;
- case 0x8: ccp = Busy; break;
- case 0x10: ccp = Intermediate; break;
- case 0x14: ccp = Intermediate-Condition Met; break;
- case 0x18: ccp = Reservation Conflict; break;
- case 0x22: ccp = Command Terminated; break; /* obsolete */
- case 0x28: ccp = Task set Full; break;/* was: Queue Full */
- case 0x30: ccp = ACA Active; break;
- case 0x40: ccp = Task Aborted; break;
- default: ccp = Unknown status;
+ case SAM_STAT_GOOD:
+ ccp = Good; break;
+ case SAM_STAT_CHECK_CONDITION:
+ ccp = Check Condition; break;
+ case SAM_STAT_CONDITION_MET:
+ ccp = Condition Met; break;
+ case SAM_STAT_BUSY:
+ ccp = Busy; break;
+ case SAM_STAT_INTERMEDIATE:
+ ccp = Intermediate; break;
+ case SAM_STAT_INTERMEDIATE_CONDITION_MET:
+ ccp = Intermediate-Condition Met; break;
+ case SAM_STAT_RESERVATION_CONFLICT:
+ ccp = Reservation Conflict; break;
+ case SAM_STAT_COMMAND_TERMINATED:
+ ccp = Command Terminated; break; /* obsolete */
+ case SAM_STAT_TASK_SET_FULL:
+ ccp = Task set Full; break; /* was: Queue Full */
+ case SAM_STAT_ACA_ACTIVE:
+ ccp = ACA Active; break;
+ case SAM_STAT_TASK_ABORTED:
+ ccp = Task Aborted; break;
+ default:
+ ccp = Unknown status;
}
- printk(KERN_INFO %s, ccp);
-#else
- printk(KERN_INFO 0x%0x, scsi_status);
-#endif
+ return ccp;
}
EXPORT_SYMBOL(scsi_print_status);
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 3d9ac9f..a46bc55 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -21,7 +21,7 @@ extern void __scsi_dump_sense(struct scsi_device *, const
char *,
const unsigned char *, int);
extern void scsi_show_result(struct scsi_device *, const char *, int);
extern void scsi_print_result(struct scsi_cmnd *);
-extern void scsi_print_status(unsigned char);
+extern const char *scsi_print_status(unsigned char);
extern const char *scsi_sense_key_string(unsigned char);
extern const char *scsi_extd_sense_format(unsigned char, unsigned char,
const char **);
diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h
index db6c935..0a6835b 100644
--- a/include/trace/events/scsi.h
+++ b/include/trace/events/scsi.h
@@ -169,21 +169,6 @@
scsi_msgbyte_name(BUS_DEVICE_RESET),\
scsi_msgbyte_name(ABORT))
-#define scsi_statusbyte_name(result) { result, #result }
-#define show_statusbyte_name(val) \
- __print_symbolic(val, \
-
Re: [PATCH 00/22] scsi logging update
On 14-08-28 01:33 PM, Hannes Reinecke wrote: Hi all, here's my next round of scsi logging updates. Main feature is the update to have all logging statements in one line so that they won't be broken up even under high load. This will dramatically improve debugging. Additionally all printk() statements are moved to dev_printk() variants to ensure proper device tagging and keep the systemd journal happy. s/all/most/ ?? Surely there are situations where a dev cannot be associated with a printk(). For example in transport discovery before any devices are found (or after, if none are found). LLDs often helpfully log their HBA's firmware details prior to discovery (and may fail before discovery). And it is possible to write via sysfs to a driver that has no devices attached. How does one log that? Doug Gilbert To achieve this I had to use a on-stack buffer for formatting opcodes and sense codes; so the stack usage will increase somewhat. Reviews, comments etc are welcome. Hannes Reinecke (22): Remove scsi_cmd_print_sense_hdr() aha152x: Remove #ifdef 0 section sd: Remove scsi_print_sense() in sd_done() scsi: introduce sdev_prefix_printk() scsi: Use sdev as argument for sense code printing scsi: stop decoding if scsi_normalize_sense() fails scsi: do not decode sense extras scsi: dump sense buffer only for debugging Use sdev as argument for scsi_print_result scsi: consolidate scsi_print_status() Implement scsi_opcode_sa_name scsi: remove obsolete __scsi_print_command() usages scsi: use local buffer for printing the opcode scsi: pass in string buffer to __scsi_print_command() scsi: use dev_printk() variants in scsi_print_command() libata: use __scsi_print_command() scsi: print disposition in scsi_print_result() scsi_error: format abort error message scsi: use local buffer for scsi_log_(send|completion) scsi: align logging messages scsi: reduce messages for command failure sd: Reduce logging output drivers/ata/libata-eh.c | 12 +- drivers/scsi/53c700.c | 3 +- drivers/scsi/aha152x.c| 32 +-- drivers/scsi/arm/acornscsi.c | 9 +- drivers/scsi/arm/fas216.c | 30 +-- drivers/scsi/ch.c | 7 +- drivers/scsi/constants.c | 570 -- drivers/scsi/osst.c | 8 +- drivers/scsi/scsi.c | 65 ++--- drivers/scsi/scsi_error.c | 68 ++--- drivers/scsi/scsi_ioctl.c | 2 +- drivers/scsi/scsi_lib.c | 10 +- drivers/scsi/sd.c | 28 ++- drivers/scsi/sg.c | 9 +- drivers/scsi/sr_ioctl.c | 16 +- drivers/scsi/st.c | 13 +- drivers/scsi/storvsc_drv.c| 3 +- include/scsi/scsi_dbg.h | 34 +-- include/scsi/scsi_device.h| 5 + include/trace/events/scsi.h | 17 +- include/trace/events/target.h | 17 +- 21 files changed, 457 insertions(+), 501 deletions(-) -- 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/14] block: Make protection interval calculation generic
Now that the protection interval has been detached from the sector size
we need to be able to handle sizes that are different from 4K and
512. Make the interval calculation generic.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index a2b3e994b55b..396244b1e774 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -197,11 +197,7 @@ EXPORT_SYMBOL(bio_integrity_enabled);
static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
unsigned int sectors)
{
- /* At this point there are only 512b or 4096b DIF/EPP devices */
- if (bi-interval == 4096)
- return sectors = 3;
-
- return sectors;
+ return sectors (ilog2(bi-interval) - 9);
}
static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
--
1.9.3
--
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/14] block: Don't merge requests if integrity flags differ
We'd occasionally merge requests with conflicting integrity flags.
Introduce a merge helper which checks that the requests have compatible
integrity payloads.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/blk-integrity.c | 36 ++--
block/blk-merge.c | 6 +++---
include/linux/blkdev.h | 20 ++--
3 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 1c6ba442cd91..79ffb4855af0 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -186,37 +186,53 @@ int blk_integrity_compare(struct gendisk *gd1, struct
gendisk *gd2)
}
EXPORT_SYMBOL(blk_integrity_compare);
-int blk_integrity_merge_rq(struct request_queue *q, struct request *req,
- struct request *next)
+bool blk_integrity_merge_rq(struct request_queue *q, struct request *req,
+ struct request *next)
{
- if (blk_integrity_rq(req) != blk_integrity_rq(next))
- return -1;
+ if (blk_integrity_rq(req) == 0 blk_integrity_rq(next) == 0)
+ return true;
+
+ if (blk_integrity_rq(req) == 0 || blk_integrity_rq(next) == 0)
+ return false;
+
+ if (bio_integrity(req-bio)-bip_flags !=
+ bio_integrity(next-bio)-bip_flags)
+ return false;
if (req-nr_integrity_segments + next-nr_integrity_segments
q-limits.max_integrity_segments)
- return -1;
+ return false;
- return 0;
+ return true;
}
EXPORT_SYMBOL(blk_integrity_merge_rq);
-int blk_integrity_merge_bio(struct request_queue *q, struct request *req,
- struct bio *bio)
+bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
+struct bio *bio)
{
int nr_integrity_segs;
struct bio *next = bio-bi_next;
+ if (blk_integrity_rq(req) == 0 bio_integrity(bio) == NULL)
+ return true;
+
+ if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
+ return false;
+
+ if (bio_integrity(req-bio)-bip_flags != bio_integrity(bio)-bip_flags)
+ return false;
+
bio-bi_next = NULL;
nr_integrity_segs = blk_rq_count_integrity_sg(q, bio);
bio-bi_next = next;
if (req-nr_integrity_segments + nr_integrity_segs
q-limits.max_integrity_segments)
- return -1;
+ return false;
req-nr_integrity_segments += nr_integrity_segs;
- return 0;
+ return true;
}
EXPORT_SYMBOL(blk_integrity_merge_bio);
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 54535831f1e1..20dcc7d0aa8f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -308,7 +308,7 @@ static inline int ll_new_hw_segment(struct request_queue *q,
if (req-nr_phys_segments + nr_phys_segs queue_max_segments(q))
goto no_merge;
- if (bio_integrity(bio) blk_integrity_merge_bio(q, req, bio))
+ if (blk_integrity_merge_bio(q, req, bio) == false)
goto no_merge;
/*
@@ -405,7 +405,7 @@ static int ll_merge_requests_fn(struct request_queue *q,
struct request *req,
if (total_phys_segments queue_max_segments(q))
return 0;
- if (blk_integrity_rq(req) blk_integrity_merge_rq(q, req, next))
+ if (blk_integrity_merge_rq(q, req, next) == false)
return 0;
/* Merge is OK... */
@@ -585,7 +585,7 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
return false;
/* only merge integrity protected bio into ditto rq */
- if (bio_integrity(bio) != blk_integrity_rq(rq))
+ if (blk_integrity_merge_bio(rq-q, rq, bio) == false)
return false;
/* must be using the same buffer */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index cbc3608d08c1..e8b2b0062cb8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1503,10 +1503,10 @@ extern int blk_integrity_compare(struct gendisk *,
struct gendisk *);
extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
struct scatterlist *);
extern int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
-extern int blk_integrity_merge_rq(struct request_queue *, struct request *,
- struct request *);
-extern int blk_integrity_merge_bio(struct request_queue *, struct request *,
- struct bio *);
+extern bool blk_integrity_merge_rq(struct request_queue *, struct request *,
+ struct request *);
+extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
+
[PATCH 13/14] block: Add T10 Protection Information functions
The T10 Protection Information format is also used by some devices that
do not go through the SCSI layer (virtual block devices, NVMe). Relocate
the relevant functions to a block layer library that can be used without
involving SCSI.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
---
block/Kconfig | 1 +
block/Makefile | 4 +-
block/t10-pi.c | 164 ++
drivers/scsi/Kconfig | 1 -
drivers/scsi/sd_dif.c | 241 -
include/linux/crc-t10dif.h | 5 +-
include/linux/t10-pi.h | 60 +++
7 files changed, 250 insertions(+), 226 deletions(-)
create mode 100644 block/t10-pi.c
create mode 100644 include/linux/t10-pi.h
diff --git a/block/Kconfig b/block/Kconfig
index 2429515c05c2..1427106f30c9 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -77,6 +77,7 @@ config BLK_DEV_BSGLIB
config BLK_DEV_INTEGRITY
bool Block layer data integrity support
+ select CRC_T10_PI if BLK_DEV_INTEGRITY
---help---
Some storage devices allow extra information to be
stored/retrieved to help protect the data. The block layer
diff --git a/block/Makefile b/block/Makefile
index a2ce6ac935ec..00ecc97629db 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -20,6 +20,6 @@ obj-$(CONFIG_IOSCHED_DEADLINE)+= deadline-iosched.o
obj-$(CONFIG_IOSCHED_CFQ) += cfq-iosched.o
obj-$(CONFIG_BLOCK_COMPAT) += compat_ioctl.o
-obj-$(CONFIG_BLK_DEV_INTEGRITY)+= blk-integrity.o
obj-$(CONFIG_BLK_CMDLINE_PARSER) += cmdline-parser.o
-obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o
+obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o blk-integrity.o t10-pi.o
+
diff --git a/block/t10-pi.c b/block/t10-pi.c
new file mode 100644
index ..f45da5d55bd8
--- /dev/null
+++ b/block/t10-pi.c
@@ -0,0 +1,164 @@
+/*
+ * t10_pi.c - Functions for generating and verifying T10 Protection
+ * Information.
+ *
+ * Copyright (C) 2007, 2008, 2014 Oracle Corporation
+ * Written by: Martin K. Petersen martin.peter...@oracle.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; see the file COPYING. If not, write to
+ * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139,
+ * USA.
+ *
+ */
+
+#include linux/t10-pi.h
+#include linux/blkdev.h
+#include linux/crc-t10dif.h
+#include net/checksum.h
+
+typedef __u16 (csum_fn) (void *, unsigned int);
+
+static __u16 t10_pi_crc_fn(void *data, unsigned int len)
+{
+ return cpu_to_be16(crc_t10dif(data, len));
+}
+
+static __u16 t10_pi_ip_fn(void *data, unsigned int len)
+{
+ return ip_compute_csum(data, len);
+}
+
+/*
+ * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
+ * 16 bit app tag, 32 bit reference tag. Type 3 does not define the ref
+ * tag.
+ */
+static int t10_pi_generate(struct blk_integrity_iter *iter, csum_fn *fn,
+ unsigned int type)
+{
+ unsigned int i;
+
+ for (i = 0 ; i iter-data_size ; i += iter-interval) {
+ struct t10_pi_tuple *pi = iter-prot_buf;
+
+ pi-guard_tag = fn(iter-data_buf, iter-interval);
+ pi-app_tag = 0;
+
+ if (type == 1)
+ pi-ref_tag = cpu_to_be32(iter-seed 0x);
+ else
+ pi-ref_tag = 0;
+
+ iter-data_buf += iter-interval;
+ iter-prot_buf += sizeof(struct t10_pi_tuple);
+ iter-seed++;
+ }
+
+ return 0;
+}
+
+static int t10_pi_verify(struct blk_integrity_iter *iter, csum_fn *fn,
+ unsigned int type)
+{
+ unsigned int i;
+
+ for (i = 0 ; i iter-data_size ; i += iter-interval) {
+ struct t10_pi_tuple *pi = iter-prot_buf;
+ __u16 csum;
+
+ switch (type) {
+ case 1:
+ case 2:
+ if (pi-app_tag == 0x)
+ goto next;
+
+ if (be32_to_cpu(pi-ref_tag) !=
+ (iter-seed 0x)) {
+ pr_err(%s: ref tag error at location %lu \
+ (rcvd %u)\n, iter-disk_name,
+ iter-seed, be32_to_cpu(pi-ref_tag));
+ return -EILSEQ;
+ }
+
Block/SCSI data integrity update v3
This is the data integrity patch series originally submitted for 3.16 and 3.17. It has been rebased on top of block/for-3.18/core. Other than that there are no changes from v2. -- 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 01/14] block: Get rid of bdev_integrity_enabled()
bdev_integrity_enabled() is only used by bio_integrity_enabled().
Combine these two functions.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
Documentation/block/data-integrity.txt | 10 -
block/bio-integrity.c | 39 +++---
include/linux/bio.h| 6 +++---
3 files changed, 20 insertions(+), 35 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index 2d735b0ae383..b4eacf48053c 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -192,16 +192,6 @@ will require extra work due to the application tag.
supported by the block device.
-int bdev_integrity_enabled(block_device, int rw);
-
- bdev_integrity_enabled() will return 1 if the block device
- supports integrity metadata transfer for the data direction
- specified in 'rw'.
-
- bdev_integrity_enabled() honors the write_generate and
- read_verify flags in sysfs and will respond accordingly.
-
-
int bio_integrity_prep(bio);
To generate IMD for WRITE and to set up buffers for READ, the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index bc423f7b02da..d660d9ccc6b9 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -147,24 +147,6 @@ int bio_integrity_add_page(struct bio *bio, struct page
*page,
}
EXPORT_SYMBOL(bio_integrity_add_page);
-static int bdev_integrity_enabled(struct block_device *bdev, int rw)
-{
- struct blk_integrity *bi = bdev_get_integrity(bdev);
-
- if (bi == NULL)
- return 0;
-
- if (rw == READ bi-verify_fn != NULL
- (bi-flags INTEGRITY_FLAG_READ))
- return 1;
-
- if (rw == WRITE bi-generate_fn != NULL
- (bi-flags INTEGRITY_FLAG_WRITE))
- return 1;
-
- return 0;
-}
-
/**
* bio_integrity_enabled - Check whether integrity can be passed
* @bio: bio to check
@@ -174,16 +156,29 @@ static int bdev_integrity_enabled(struct block_device
*bdev, int rw)
* set prior to calling. The functions honors the write_generate and
* read_verify flags in sysfs.
*/
-int bio_integrity_enabled(struct bio *bio)
+bool bio_integrity_enabled(struct bio *bio)
{
+ struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
+
if (!bio_is_rw(bio))
- return 0;
+ return false;
/* Already protected? */
if (bio_integrity(bio))
- return 0;
+ return false;
+
+ if (bi == NULL)
+ return false;
+
+ if (bio_data_dir(bio) == READ bi-verify_fn != NULL
+ (bi-flags INTEGRITY_FLAG_READ))
+ return true;
+
+ if (bio_data_dir(bio) == WRITE bi-generate_fn != NULL
+ (bi-flags INTEGRITY_FLAG_WRITE))
+ return true;
- return bdev_integrity_enabled(bio-bi_bdev, bio_data_dir(bio));
+ return false;
}
EXPORT_SYMBOL(bio_integrity_enabled);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index b39e5000ff58..63e399b4fde5 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -666,7 +666,7 @@ struct biovec_slab {
extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t,
unsigned int);
extern void bio_integrity_free(struct bio *);
extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int,
unsigned int);
-extern int bio_integrity_enabled(struct bio *bio);
+extern bool bio_integrity_enabled(struct bio *bio);
extern int bio_integrity_set_tag(struct bio *, void *, unsigned int);
extern int bio_integrity_get_tag(struct bio *, void *, unsigned int);
extern int bio_integrity_prep(struct bio *);
@@ -685,9 +685,9 @@ static inline int bio_integrity(struct bio *bio)
return 0;
}
-static inline int bio_integrity_enabled(struct bio *bio)
+static inline bool bio_integrity_enabled(struct bio *bio)
{
- return 0;
+ return false;
}
static inline int bioset_integrity_create(struct bio_set *bs, int pool_size)
--
1.9.3
--
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/14] block: Integrity checksum flag
Make the choice of checksum a per-I/O property by introducing a flag
that can be inspected by the SCSI layer. There are several reasons for
this:
1. It allows us to switch choice of checksum without unloading and
reloading the HBA driver.
2. During error recovery we need to be able to tell the HBA that
checksums read from disk should not be verified and converted to IP
checksums.
3. For error injection purposes we need to be able to write a bad guard
tag to storage. Since the storage device only supports T10 CRC we
need to be able to disable IP checksum conversion on the HBA.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 3 +++
drivers/scsi/sd_dif.c | 6 --
include/linux/bio.h| 1 +
include/linux/blkdev.h | 1 +
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 7cac9c34615c..b5327751fecc 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -297,6 +297,9 @@ int bio_integrity_prep(struct bio *bio)
bip-bip_iter.bi_size = len;
bip_set_seed(bip, bio-bi_iter.bi_sector);
+ if (bi-flags BLK_INTEGRITY_IP_CHECKSUM)
+ bip-bip_flags |= BIP_IP_CHECKSUM;
+
/* Map it */
offset = offset_in_page(buf);
for (i = 0 ; i nr_pages ; i++) {
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 4ce636fdc15f..2198abee619e 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -255,12 +255,14 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
return;
/* Enable DMA of protection information */
- if (scsi_host_get_guard(sdkp-device-host) SHOST_DIX_GUARD_IP)
+ if (scsi_host_get_guard(sdkp-device-host) SHOST_DIX_GUARD_IP) {
if (type == SD_DIF_TYPE3_PROTECTION)
blk_integrity_register(disk, dif_type3_integrity_ip);
else
blk_integrity_register(disk, dif_type1_integrity_ip);
- else
+
+ disk-integrity-flags |= BLK_INTEGRITY_IP_CHECKSUM;
+ } else
if (type == SD_DIF_TYPE3_PROTECTION)
blk_integrity_register(disk, dif_type3_integrity_crc);
else
diff --git a/include/linux/bio.h b/include/linux/bio.h
index b508cf69206d..14bff3fe56d4 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -328,6 +328,7 @@ enum bip_flags {
BIP_MAPPED_INTEGRITY= 1 1, /* ref tag has been remapped */
BIP_CTRL_NOCHECK= 1 2, /* disable HBA integrity checking */
BIP_DISK_NOCHECK= 1 3, /* disable disk integrity checking */
+ BIP_IP_CHECKSUM = 1 4, /* IP checksum */
};
static inline sector_t bip_get_seed(struct bio_integrity_payload *bip)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index fc1ac3cab086..cbc3608d08c1 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1468,6 +1468,7 @@ enum blk_integrity_flags {
BLK_INTEGRITY_VERIFY= 1 0,
BLK_INTEGRITY_GENERATE = 1 1,
BLK_INTEGRITY_DEVICE_CAPABLE= 1 2,
+ BLK_INTEGRITY_IP_CHECKSUM = 1 3,
};
struct blk_integrity_iter {
--
1.9.3
--
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/14] sd: Honor block layer integrity handling flags
A set of flags introduced in the block layer enable better control over
how protection information is handled. These flags are useful for both
error injection and data recovery purposes. Checking can be enabled and
disabled for controller and disk, and the guard tag format is now a
per-I/O property.
Update sd_protect_op to communicate the relevant information to the
low-level device driver via a set of flags in scsi_cmnd.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
drivers/scsi/sd.c| 73
drivers/scsi/sd.h| 66 +--
drivers/scsi/sd_dif.c| 23 ++-
include/scsi/scsi_cmnd.h | 36 +---
4 files changed, 142 insertions(+), 56 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 2c2041ca4b70..53d049cc1a1e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -610,29 +610,44 @@ static void scsi_disk_put(struct scsi_disk *sdkp)
mutex_unlock(sd_ref_mutex);
}
-static void sd_prot_op(struct scsi_cmnd *scmd, unsigned int dif)
-{
- unsigned int prot_op = SCSI_PROT_NORMAL;
- unsigned int dix = scsi_prot_sg_count(scmd);
-
- if (scmd-sc_data_direction == DMA_FROM_DEVICE) {
- if (dif dix)
- prot_op = SCSI_PROT_READ_PASS;
- else if (dif !dix)
- prot_op = SCSI_PROT_READ_STRIP;
- else if (!dif dix)
- prot_op = SCSI_PROT_READ_INSERT;
- } else {
- if (dif dix)
- prot_op = SCSI_PROT_WRITE_PASS;
- else if (dif !dix)
- prot_op = SCSI_PROT_WRITE_INSERT;
- else if (!dif dix)
- prot_op = SCSI_PROT_WRITE_STRIP;
+
+
+static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
+ unsigned int dix, unsigned int dif)
+{
+ struct bio_integrity_payload *bip = bio_integrity(scmd-request-bio);
+ unsigned int prot_op = sd_prot_op(rq_data_dir(scmd-request), dix, dif);
+ unsigned int protect = 0;
+
+ if (dix) { /* DIX Type 0, 1, 2, 3 */
+ if (bip-bip_flags BIP_IP_CHECKSUM)
+ scmd-prot_flags |= SCSI_PROT_IP_CHECKSUM;
+
+ if ((bip-bip_flags BIP_CTRL_NOCHECK) == 0)
+ scmd-prot_flags |= SCSI_PROT_GUARD_CHECK;
+ }
+
+ if (dif != SD_DIF_TYPE3_PROTECTION) { /* DIX/DIF Type 0, 1, 2 */
+ scmd-prot_flags |= SCSI_PROT_REF_INCREMENT;
+
+ if ((bip bip-bip_flags BIP_CTRL_NOCHECK) == 0)
+ scmd-prot_flags |= SCSI_PROT_REF_CHECK;
+ }
+
+ if (dif) { /* DIX/DIF Type 1, 2, 3 */
+ scmd-prot_flags |= SCSI_PROT_TRANSFER_PI;
+
+ if (bip bip-bip_flags BIP_DISK_NOCHECK)
+ protect = 3 5; /* Disable target PI checking */
+ else
+ protect = 1 5; /* Enable target PI checking */
}
scsi_set_prot_op(scmd, prot_op);
scsi_set_prot_type(scmd, dif);
+ scmd-prot_flags = sd_prot_flag_mask(prot_op);
+
+ return protect;
}
static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)
@@ -893,7 +908,8 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
sector_t block = blk_rq_pos(rq);
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
- int ret, host_dif;
+ unsigned int dif, dix;
+ int ret;
unsigned char protect;
ret = scsi_init_io(SCpnt, GFP_ATOMIC);
@@ -995,7 +1011,7 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
SCpnt-cmnd[0] = WRITE_6;
if (blk_integrity_rq(rq))
- sd_dif_prepare(rq, block, sdp-sector_size);
+ sd_dif_prepare(SCpnt);
} else if (rq_data_dir(rq) == READ) {
SCpnt-cmnd[0] = READ_6;
@@ -1010,14 +1026,15 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd
*SCpnt)
writing : reading, this_count,
blk_rq_sectors(rq)));
- /* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
- host_dif = scsi_host_dif_capable(sdp-host, sdkp-protection_type);
- if (host_dif)
- protect = 1 5;
+ dix = scsi_prot_sg_count(SCpnt);
+ dif = scsi_host_dif_capable(SCpnt-device-host, sdkp-protection_type);
+
+ if (dif || dix)
+ protect = sd_setup_protect_cmnd(SCpnt, dix, dif);
else
protect = 0;
- if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ if (protect sdkp-protection_type == SD_DIF_TYPE2_PROTECTION) {
SCpnt-cmnd =
[PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms sector and sectors.
Going forward we will use the term seed to describe the initial
reference tag value for a given I/O. Interval will be used to describe
the portion of the data buffer that a given piece of protection
information is associated with.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 42 +-
block/blk-integrity.c | 10 +-
drivers/scsi/sd_dif.c | 46 +++---
include/linux/blkdev.h | 6 +++---
4 files changed, 52 insertions(+), 52 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e4a015f661fc..a2b3e994b55b 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -185,20 +185,20 @@ bool bio_integrity_enabled(struct bio *bio)
EXPORT_SYMBOL(bio_integrity_enabled);
/**
- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals for a bio
* @bi:blk_integrity profile for device
- * @sectors: Number of 512 sectors to convert
+ * @sectors: Size of the bio in 512-byte sectors
*
* Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device. Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data integrity
+ * interval size of the storage device. Convert the block layer sectors
+ * to the appropriate number of integrity intervals.
*/
-static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
- unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
+ unsigned int sectors)
{
/* At this point there are only 512b or 4096b DIF/EPP devices */
- if (bi-sector_size == 4096)
+ if (bi-interval == 4096)
return sectors = 3;
return sectors;
@@ -207,7 +207,7 @@ static inline unsigned int bio_integrity_hw_sectors(struct
blk_integrity *bi,
static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
unsigned int sectors)
{
- return bio_integrity_hw_sectors(bi, sectors) * bi-tuple_size;
+ return bio_integrity_intervals(bi, sectors) * bi-tuple_size;
}
/**
@@ -221,25 +221,25 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct blk_integrity_exchg bix;
struct bio_vec *bv;
struct bio_integrity_payload *bip = bio_integrity(bio);
- sector_t sector;
- unsigned int sectors, ret = 0, i;
+ sector_t seed;
+ unsigned int intervals, ret = 0, i;
void *prot_buf = page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset;
if (operate)
- sector = bio-bi_iter.bi_sector;
+ seed = bio-bi_iter.bi_sector;
else
- sector = bip-bip_iter.bi_sector;
+ seed = bip-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
- bix.sector_size = bi-sector_size;
+ bix.interval = bi-interval;
bio_for_each_segment_all(bv, bio, i) {
void *kaddr = kmap_atomic(bv-bv_page);
bix.data_buf = kaddr + bv-bv_offset;
bix.data_size = bv-bv_len;
bix.prot_buf = prot_buf;
- bix.sector = sector;
+ bix.seed = seed;
if (operate)
bi-generate_fn(bix);
@@ -251,9 +251,9 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
}
}
- sectors = bv-bv_len / bi-sector_size;
- sector += sectors;
- prot_buf += sectors * bi-tuple_size;
+ intervals = bv-bv_len / bi-interval;
+ seed += intervals;
+ prot_buf += intervals * bi-tuple_size;
kunmap_atomic(kaddr);
}
@@ -294,17 +294,17 @@ int bio_integrity_prep(struct bio *bio)
unsigned long start, end;
unsigned int len, nr_pages;
unsigned int bytes, offset, i;
- unsigned int sectors;
+ unsigned int intervals;
bi = bdev_get_integrity(bio-bi_bdev);
q = bdev_get_queue(bio-bi_bdev);
BUG_ON(bi == NULL);
BUG_ON(bio_integrity(bio));
- sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
+ intervals = bio_integrity_intervals(bi, bio_sectors(bio));
/* Allocate kernel buffer for protection data */
- len =
[PATCH 07/14] block: Clean up the code used to generate and verify integrity metadata
Instead of the operate parameter we pass in a seed value and a pointer
to a function that can be used to process the integrity metadata. The
generation function is changed to have a return value to fit into this
scheme.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 82 ++
drivers/scsi/sd_dif.c | 106 ++---
include/linux/bio.h| 12 ++
include/linux/blkdev.h | 9 ++---
4 files changed, 94 insertions(+), 115 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 396244b1e774..5ef996e8063f 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -207,49 +207,37 @@ static inline unsigned int bio_integrity_bytes(struct
blk_integrity *bi,
}
/**
- * bio_integrity_generate_verify - Generate/verify integrity metadata for a bio
+ * bio_integrity_process - Process integrity metadata for a bio
* @bio: bio to generate/verify integrity metadata for
- * @operate: operate number, 1 for generate, 0 for verify
+ * @proc_fn: Pointer to the relevant processing function
*/
-static int bio_integrity_generate_verify(struct bio *bio, int operate)
+static int bio_integrity_process(struct bio *bio,
+integrity_processing_fn *proc_fn)
{
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
- struct blk_integrity_exchg bix;
+ struct blk_integrity_iter iter;
struct bio_vec *bv;
struct bio_integrity_payload *bip = bio_integrity(bio);
- sector_t seed;
- unsigned int intervals, ret = 0, i;
+ unsigned int i, ret = 0;
void *prot_buf = page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset;
- if (operate)
- seed = bio-bi_iter.bi_sector;
- else
- seed = bip-bip_iter.bi_sector;
-
- bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
- bix.interval = bi-interval;
+ iter.disk_name = bio-bi_bdev-bd_disk-disk_name;
+ iter.interval = bi-interval;
+ iter.seed = bip_get_seed(bip);
+ iter.prot_buf = prot_buf;
bio_for_each_segment_all(bv, bio, i) {
void *kaddr = kmap_atomic(bv-bv_page);
- bix.data_buf = kaddr + bv-bv_offset;
- bix.data_size = bv-bv_len;
- bix.prot_buf = prot_buf;
- bix.seed = seed;
-
- if (operate)
- bi-generate_fn(bix);
- else {
- ret = bi-verify_fn(bix);
- if (ret) {
- kunmap_atomic(kaddr);
- return ret;
- }
- }
- intervals = bv-bv_len / bi-interval;
- seed += intervals;
- prot_buf += intervals * bi-tuple_size;
+ iter.data_buf = kaddr + bv-bv_offset;
+ iter.data_size = bv-bv_len;
+
+ ret = proc_fn(iter);
+ if (ret) {
+ kunmap_atomic(kaddr);
+ return ret;
+ }
kunmap_atomic(kaddr);
}
@@ -257,20 +245,6 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
}
/**
- * bio_integrity_generate - Generate integrity metadata for a bio
- * @bio: bio to generate integrity metadata for
- *
- * Description: Generates integrity metadata for a bio by calling the
- * block device's generation callback function. The bio must have a
- * bip attached with enough room to accommodate the generated
- * integrity metadata.
- */
-static void bio_integrity_generate(struct bio *bio)
-{
- bio_integrity_generate_verify(bio, 1);
-}
-
-/**
* bio_integrity_prep - Prepare bio for integrity I/O
* @bio: bio to prepare
*
@@ -321,7 +295,7 @@ int bio_integrity_prep(struct bio *bio)
bip-bip_owns_buf = 1;
bip-bip_iter.bi_size = len;
- bip-bip_iter.bi_sector = bio-bi_iter.bi_sector;
+ bip_set_seed(bip, bio-bi_iter.bi_sector);
/* Map it */
offset = offset_in_page(buf);
@@ -357,26 +331,13 @@ int bio_integrity_prep(struct bio *bio)
/* Auto-generate integrity metadata if this is a write */
if (bio_data_dir(bio) == WRITE)
- bio_integrity_generate(bio);
+ bio_integrity_process(bio, bi-generate_fn);
return 0;
}
EXPORT_SYMBOL(bio_integrity_prep);
/**
- * bio_integrity_verify - Verify integrity metadata for a bio
- * @bio: bio to verify
- *
- * Description: This function is called to verify the integrity of a
- * bio. The data in the bio io_vec is compared to the integrity
- * metadata returned by the HBA.
- */
-static int bio_integrity_verify(struct bio *bio)
-{
- return bio_integrity_generate_verify(bio, 0);
-}
-
-/**
*
[PATCH 08/14] block: Add prefix to block integrity profile flags
Add a BLK_ prefix to the integrity profile flags. Also rename the flags
to be more consistent with the generate/verify terminology in the rest
of the integrity code.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 4 ++--
block/blk-integrity.c | 43 ++-
include/linux/blkdev.h | 6 --
3 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 5ef996e8063f..c6bc2faf2af7 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -173,11 +173,11 @@ bool bio_integrity_enabled(struct bio *bio)
return false;
if (bio_data_dir(bio) == READ bi-verify_fn != NULL
- (bi-flags INTEGRITY_FLAG_READ))
+ (bi-flags BLK_INTEGRITY_VERIFY))
return true;
if (bio_data_dir(bio) == WRITE bi-generate_fn != NULL
- (bi-flags INTEGRITY_FLAG_WRITE))
+ (bi-flags BLK_INTEGRITY_GENERATE))
return true;
return false;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 3a83a7d08177..a7436ccc936b 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -269,42 +269,42 @@ static ssize_t integrity_tag_size_show(struct
blk_integrity *bi, char *page)
return sprintf(page, 0\n);
}
-static ssize_t integrity_read_store(struct blk_integrity *bi,
- const char *page, size_t count)
+static ssize_t integrity_verify_store(struct blk_integrity *bi,
+ const char *page, size_t count)
{
char *p = (char *) page;
unsigned long val = simple_strtoul(p, p, 10);
if (val)
- bi-flags |= INTEGRITY_FLAG_READ;
+ bi-flags |= BLK_INTEGRITY_VERIFY;
else
- bi-flags = ~INTEGRITY_FLAG_READ;
+ bi-flags = ~BLK_INTEGRITY_VERIFY;
return count;
}
-static ssize_t integrity_read_show(struct blk_integrity *bi, char *page)
+static ssize_t integrity_verify_show(struct blk_integrity *bi, char *page)
{
- return sprintf(page, %d\n, (bi-flags INTEGRITY_FLAG_READ) != 0);
+ return sprintf(page, %d\n, (bi-flags BLK_INTEGRITY_VERIFY) != 0);
}
-static ssize_t integrity_write_store(struct blk_integrity *bi,
-const char *page, size_t count)
+static ssize_t integrity_generate_store(struct blk_integrity *bi,
+ const char *page, size_t count)
{
char *p = (char *) page;
unsigned long val = simple_strtoul(p, p, 10);
if (val)
- bi-flags |= INTEGRITY_FLAG_WRITE;
+ bi-flags |= BLK_INTEGRITY_GENERATE;
else
- bi-flags = ~INTEGRITY_FLAG_WRITE;
+ bi-flags = ~BLK_INTEGRITY_GENERATE;
return count;
}
-static ssize_t integrity_write_show(struct blk_integrity *bi, char *page)
+static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page)
{
- return sprintf(page, %d\n, (bi-flags INTEGRITY_FLAG_WRITE) != 0);
+ return sprintf(page, %d\n, (bi-flags BLK_INTEGRITY_GENERATE) != 0);
}
static struct integrity_sysfs_entry integrity_format_entry = {
@@ -317,23 +317,23 @@ static struct integrity_sysfs_entry
integrity_tag_size_entry = {
.show = integrity_tag_size_show,
};
-static struct integrity_sysfs_entry integrity_read_entry = {
+static struct integrity_sysfs_entry integrity_verify_entry = {
.attr = { .name = read_verify, .mode = S_IRUGO | S_IWUSR },
- .show = integrity_read_show,
- .store = integrity_read_store,
+ .show = integrity_verify_show,
+ .store = integrity_verify_store,
};
-static struct integrity_sysfs_entry integrity_write_entry = {
+static struct integrity_sysfs_entry integrity_generate_entry = {
.attr = { .name = write_generate, .mode = S_IRUGO | S_IWUSR },
- .show = integrity_write_show,
- .store = integrity_write_store,
+ .show = integrity_generate_show,
+ .store = integrity_generate_store,
};
static struct attribute *integrity_attrs[] = {
integrity_format_entry.attr,
integrity_tag_size_entry.attr,
- integrity_read_entry.attr,
- integrity_write_entry.attr,
+ integrity_verify_entry.attr,
+ integrity_generate_entry.attr,
NULL,
};
@@ -406,7 +406,7 @@ int blk_integrity_register(struct gendisk *disk, struct
blk_integrity *template)
kobject_uevent(bi-kobj, KOBJ_ADD);
- bi-flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
+ bi-flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
bi-interval = queue_logical_block_size(disk-queue);
disk-integrity = bi;
} else
@@ -419,6 +419,7 @@ int
[PATCH 09/14] block: Add a disk flag to block integrity profile
So far we have relied on the app tag size to determine whether a disk
has been formatted with T10 protection information or not. However, not
all target devices provide application tag storage.
Add a flag to the block integrity profile that indicates whether the
disk has been formatted with protection information.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Sagi Grimberg sa...@dev.mellanox.co.il
---
Documentation/ABI/testing/sysfs-block | 8
block/blk-integrity.c | 12
drivers/scsi/sd_dif.c | 8 +++-
include/linux/blkdev.h| 1 +
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-block
b/Documentation/ABI/testing/sysfs-block
index 279da08f7541..8df003963d99 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -53,6 +53,14 @@ Description:
512 bytes of data.
+What: /sys/block/disk/integrity/device_is_integrity_capable
+Date: July 2014
+Contact: Martin K. Petersen martin.peter...@oracle.com
+Description:
+ Indicates whether a storage device is capable of storing
+ integrity metadata. Set if the device is T10 PI-capable.
+
+
What: /sys/block/disk/integrity/write_generate
Date: June 2008
Contact: Martin K. Petersen martin.peter...@oracle.com
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index a7436ccc936b..1c6ba442cd91 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -307,6 +307,12 @@ static ssize_t integrity_generate_show(struct
blk_integrity *bi, char *page)
return sprintf(page, %d\n, (bi-flags BLK_INTEGRITY_GENERATE) != 0);
}
+static ssize_t integrity_device_show(struct blk_integrity *bi, char *page)
+{
+ return sprintf(page, %u\n,
+ (bi-flags BLK_INTEGRITY_DEVICE_CAPABLE) != 0);
+}
+
static struct integrity_sysfs_entry integrity_format_entry = {
.attr = { .name = format, .mode = S_IRUGO },
.show = integrity_format_show,
@@ -329,11 +335,17 @@ static struct integrity_sysfs_entry
integrity_generate_entry = {
.store = integrity_generate_store,
};
+static struct integrity_sysfs_entry integrity_device_entry = {
+ .attr = { .name = device_is_integrity_capable, .mode = S_IRUGO },
+ .show = integrity_device_show,
+};
+
static struct attribute *integrity_attrs[] = {
integrity_format_entry.attr,
integrity_tag_size_entry.attr,
integrity_verify_entry.attr,
integrity_generate_entry.attr,
+ integrity_device_entry.attr,
NULL,
};
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 801c41851a01..1e971c6f8c2b 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -270,7 +270,13 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
Enabling DIX %s protection\n, disk-integrity-name);
/* Signal to block layer that we support sector tagging */
- if (dif type sdkp-ATO) {
+ if (dif type) {
+
+ disk-integrity-flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+
+ if (!sdkp)
+ return;
+
if (type == SD_DIF_TYPE3_PROTECTION)
disk-integrity-tag_size = sizeof(u16) + sizeof(u32);
else
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 54cbd96fe94a..fc1ac3cab086 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1467,6 +1467,7 @@ static inline uint64_t rq_io_start_time_ns(struct request
*req)
enum blk_integrity_flags {
BLK_INTEGRITY_VERIFY= 1 0,
BLK_INTEGRITY_GENERATE = 1 1,
+ BLK_INTEGRITY_DEVICE_CAPABLE= 1 2,
};
struct blk_integrity_iter {
--
1.9.3
--
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/14] block: Remove integrity tagging functions
None of the filesystems appear interested in using the integrity tagging
feature. Potentially because very few storage devices actually permit
using the application tag space.
Remove the tagging functions.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
Documentation/block/data-integrity.txt | 34
block/bio-integrity.c | 94 +-
block/blk-integrity.c | 2 -
drivers/scsi/sd_dif.c | 65 ---
include/linux/bio.h| 3 --
include/linux/blkdev.h | 4 --
6 files changed, 1 insertion(+), 201 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index 4d4de8b09530..f56ec97f0d14 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -206,36 +206,6 @@ will require extra work due to the application tag.
bio_integrity_enabled() returned 1.
-int bio_integrity_tag_size(bio);
-
- If the filesystem wants to use the application tag space it will
- first have to find out how much storage space is available.
- Because tag space is generally limited (usually 2 bytes per
- sector regardless of sector size), the integrity framework
- supports interleaving the information between the sectors in an
- I/O.
-
- Filesystems can call bio_integrity_tag_size(bio) to find out how
- many bytes of storage are available for that particular bio.
-
- Another option is bdev_get_tag_size(block_device) which will
- return the number of available bytes per hardware sector.
-
-
-int bio_integrity_set_tag(bio, void *tag_buf, len);
-
- After a successful return from bio_integrity_prep(),
- bio_integrity_set_tag() can be used to attach an opaque tag
- buffer to a bio. Obviously this only makes sense if the I/O is
- a WRITE.
-
-
-int bio_integrity_get_tag(bio, void *tag_buf, len);
-
- Similarly, at READ I/O completion time the filesystem can
- retrieve the tag buffer using bio_integrity_get_tag().
-
-
5.3 PASSING EXISTING INTEGRITY METADATA
Filesystems that either generate their own integrity metadata or
@@ -288,8 +258,6 @@ will require extra work due to the application tag.
.name = STANDARDSBODY-TYPE-VARIANT-CSUM,
.generate_fn= my_generate_fn,
.verify_fn = my_verify_fn,
- .get_tag_fn = my_get_tag_fn,
- .set_tag_fn = my_set_tag_fn,
.tuple_size = sizeof(struct my_tuple_size),
.tag_size = tag bytes per hw sector,
};
@@ -311,7 +279,5 @@ will require extra work due to the application tag.
are available per hardware sector. For DIF this is either 2 or
0 depending on the value of the Control Mode Page ATO bit.
- See 6.2 for a description of get_tag_fn and set_tag_fn.
-
--
2007-12-24 Martin K. Petersen martin.peter...@oracle.com
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 148a63e1e323..9be2431475e5 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -210,90 +210,6 @@ static inline unsigned int bio_integrity_bytes(struct
blk_integrity *bi,
}
/**
- * bio_integrity_tag_size - Retrieve integrity tag space
- * @bio: bio to inspect
- *
- * Description: Returns the maximum number of tag bytes that can be
- * attached to this bio. Filesystems can use this to determine how
- * much metadata to attach to an I/O.
- */
-unsigned int bio_integrity_tag_size(struct bio *bio)
-{
- struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
-
- BUG_ON(bio-bi_iter.bi_size == 0);
-
- return bi-tag_size * (bio-bi_iter.bi_size / bi-sector_size);
-}
-EXPORT_SYMBOL(bio_integrity_tag_size);
-
-static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
-int set)
-{
- struct bio_integrity_payload *bip = bio_integrity(bio);
- struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
- unsigned int nr_sectors;
-
- BUG_ON(bip-bip_buf == NULL);
-
- if (bi-tag_size == 0)
- return -1;
-
- nr_sectors = bio_integrity_hw_sectors(bi,
- DIV_ROUND_UP(len, bi-tag_size));
-
- if (nr_sectors * bi-tuple_size bip-bip_iter.bi_size) {
- printk(KERN_ERR %s: tag too big for bio: %u %u\n, __func__,
- nr_sectors * bi-tuple_size, bip-bip_iter.bi_size);
- return -1;
- }
-
- if (set)
- bi-set_tag_fn(bip-bip_buf, tag_buf, nr_sectors);
- else
-
[PATCH 02/14] block: Replace bi_integrity with bi_special
For commands like REQ_COPY we need a way to pass extra information along
with each bio. Like integrity metadata this information must be
available at the bottom of the stack so bi_private does not suffice.
Rename the existing bi_integrity field to bi_special and make it a union
so we can have different bio extensions for each class of command.
We previously used bi_integrity != NULL as a way to identify whether a
bio had integrity metadata or not. Introduce a REQ_INTEGRITY to be the
indicator now that bi_special can contain different things.
In addition, bio_integrity(bio) will now return a pointer to the
integrity payload (when applicable).
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
Documentation/block/data-integrity.txt | 10 +-
block/bio-integrity.c | 19 ++-
drivers/scsi/sd_dif.c | 8
include/linux/bio.h| 11 +--
include/linux/blk_types.h | 8 ++--
include/linux/blkdev.h | 7 ++-
6 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index b4eacf48053c..4d4de8b09530 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -129,11 +129,11 @@ interface for this is being worked on.
4.1 BIO
The data integrity patches add a new field to struct bio when
-CONFIG_BLK_DEV_INTEGRITY is enabled. bio-bi_integrity is a pointer
-to a struct bip which contains the bio integrity payload. Essentially
-a bip is a trimmed down struct bio which holds a bio_vec containing
-the integrity metadata and the required housekeeping information (bvec
-pool, vector count, etc.)
+CONFIG_BLK_DEV_INTEGRITY is enabled. bio_integrity(bio) returns a
+pointer to a struct bip which contains the bio integrity payload.
+Essentially a bip is a trimmed down struct bio which holds a bio_vec
+containing the integrity metadata and the required housekeeping
+information (bvec pool, vector count, etc.)
A kernel subsystem can enable data integrity protection on a bio by
calling bio_integrity_alloc(bio). This will allocate and attach the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index d660d9ccc6b9..148a63e1e323 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -79,6 +79,7 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio
*bio,
bip-bip_slab = idx;
bip-bip_bio = bio;
bio-bi_integrity = bip;
+ bio-bi_rw |= REQ_INTEGRITY;
return bip;
err:
@@ -96,7 +97,7 @@ EXPORT_SYMBOL(bio_integrity_alloc);
*/
void bio_integrity_free(struct bio *bio)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_set *bs = bio-bi_pool;
if (bip-bip_owns_buf)
@@ -128,7 +129,7 @@ EXPORT_SYMBOL(bio_integrity_free);
int bio_integrity_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_vec *iv;
if (bip-bip_vcnt = bip-bip_max_vcnt) {
@@ -229,7 +230,7 @@ EXPORT_SYMBOL(bio_integrity_tag_size);
static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
int set)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
unsigned int nr_sectors;
@@ -304,12 +305,12 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct bio_vec *bv;
sector_t sector;
unsigned int sectors, ret = 0, i;
- void *prot_buf = bio-bi_integrity-bip_buf;
+ void *prot_buf = bio_integrity(bio)-bip_buf;
if (operate)
sector = bio-bi_iter.bi_sector;
else
- sector = bio-bi_integrity-bip_iter.bi_sector;
+ sector = bio_integrity(bio)-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
bix.sector_size = bi-sector_size;
@@ -505,7 +506,7 @@ static void bio_integrity_verify_fn(struct work_struct
*work)
*/
void bio_integrity_endio(struct bio *bio, int error)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
BUG_ON(bip-bip_bio != bio);
@@ -536,7 +537,7 @@ EXPORT_SYMBOL(bio_integrity_endio);
*/
void bio_integrity_advance(struct bio *bio, unsigned int bytes_done)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
[PATCH 10/14] block: Relocate bio integrity flags
Move flags affecting the integrity code out of the bio bi_flags and into
the block integrity payload.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 4 ++--
drivers/scsi/sd_dif.c | 4 ++--
include/linux/bio.h | 9 -
include/linux/blk_types.h | 6 ++
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index c6bc2faf2af7..7cac9c34615c 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -100,7 +100,7 @@ void bio_integrity_free(struct bio *bio)
struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_set *bs = bio-bi_pool;
- if (bip-bip_owns_buf)
+ if (bip-bip_flags BIP_BLOCK_INTEGRITY)
kfree(page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset);
@@ -293,7 +293,7 @@ int bio_integrity_prep(struct bio *bio)
return -EIO;
}
- bip-bip_owns_buf = 1;
+ bip-bip_flags |= BIP_BLOCK_INTEGRITY;
bip-bip_iter.bi_size = len;
bip_set_seed(bip, bio-bi_iter.bi_sector);
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 1e971c6f8c2b..4ce636fdc15f 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -326,7 +326,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
unsigned int j;
/* Already remapped? */
- if (bio_flagged(bio, BIO_MAPPED_INTEGRITY))
+ if (bip-bip_flags BIP_MAPPED_INTEGRITY)
break;
virt = bip_get_seed(bip) 0x;
@@ -347,7 +347,7 @@ void sd_dif_prepare(struct request *rq, sector_t hw_sector,
kunmap_atomic(sdt);
}
- bio-bi_flags |= (1 BIO_MAPPED_INTEGRITY);
+ bip-bip_flags |= BIP_MAPPED_INTEGRITY;
}
}
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 3fd36660fd10..b508cf69206d 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -315,7 +315,7 @@ struct bio_integrity_payload {
unsigned short bip_slab; /* slab the bip came from */
unsigned short bip_vcnt; /* # of integrity bio_vecs */
unsigned short bip_max_vcnt; /* integrity bio_vec slots */
- unsignedbip_owns_buf:1; /* should free bip_buf */
+ unsigned short bip_flags; /* control flags */
struct work_struct bip_work; /* I/O completion */
@@ -323,6 +323,13 @@ struct bio_integrity_payload {
struct bio_vec bip_inline_vecs[0];/* embedded bvec array */
};
+enum bip_flags {
+ BIP_BLOCK_INTEGRITY = 1 0, /* block layer owns integrity data */
+ BIP_MAPPED_INTEGRITY= 1 1, /* ref tag has been remapped */
+ BIP_CTRL_NOCHECK= 1 2, /* disable HBA integrity checking */
+ BIP_DISK_NOCHECK= 1 3, /* disable disk integrity checking */
+};
+
static inline sector_t bip_get_seed(struct bio_integrity_payload *bip)
{
return bip-bip_iter.bi_sector;
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 0b7084f7c136..c96939d488ed 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -120,10 +120,8 @@ struct bio {
#define BIO_USER_MAPPED 6 /* contains user pages */
#define BIO_EOPNOTSUPP 7 /* not supported */
#define BIO_NULL_MAPPED 8 /* contains invalid user pages */
-#define BIO_FS_INTEGRITY 9 /* fs owns integrity data, not block layer */
-#define BIO_QUIET 10 /* Make BIO Quiet */
-#define BIO_MAPPED_INTEGRITY 11/* integrity metadata has been remapped */
-#define BIO_SNAP_STABLE12 /* bio data must be snapshotted during
write */
+#define BIO_QUIET 9 /* Make BIO Quiet */
+#define BIO_SNAP_STABLE10 /* bio data must be snapshotted during
write */
/*
* Flags starting here get preserved by bio_reset() - this includes
--
1.9.3
--
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/14] block: Remove bip_buf
bip_buf is not really needed so we can remove it.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
Reviewed-by: Christoph Hellwig h...@lst.de
Reviewed-by: Sagi Grimberg sa...@mellanox.com
---
block/bio-integrity.c | 10 ++
include/linux/bio.h | 3 ---
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 9be2431475e5..e4a015f661fc 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -101,7 +101,8 @@ void bio_integrity_free(struct bio *bio)
struct bio_set *bs = bio-bi_pool;
if (bip-bip_owns_buf)
- kfree(bip-bip_buf);
+ kfree(page_address(bip-bip_vec-bv_page) +
+ bip-bip_vec-bv_offset);
if (bs) {
if (bip-bip_slab != BIO_POOL_NONE)
@@ -219,14 +220,16 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
struct blk_integrity_exchg bix;
struct bio_vec *bv;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
sector_t sector;
unsigned int sectors, ret = 0, i;
- void *prot_buf = bio_integrity(bio)-bip_buf;
+ void *prot_buf = page_address(bip-bip_vec-bv_page) +
+ bip-bip_vec-bv_offset;
if (operate)
sector = bio-bi_iter.bi_sector;
else
- sector = bio_integrity(bio)-bip_iter.bi_sector;
+ sector = bip-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
bix.sector_size = bi-sector_size;
@@ -321,7 +324,6 @@ int bio_integrity_prep(struct bio *bio)
}
bip-bip_owns_buf = 1;
- bip-bip_buf = buf;
bip-bip_iter.bi_size = len;
bip-bip_iter.bi_sector = bio-bi_iter.bi_sector;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 63a0e53e238c..448d8c052cb7 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -310,9 +310,6 @@ struct bio_integrity_payload {
struct bvec_iterbip_iter;
- /* kill - should just use bip_vec */
- void*bip_buf; /* generated integrity data */
-
bio_end_io_t*bip_end_io;/* saved I/O completion fn */
unsigned short bip_slab; /* slab the bip came from */
--
1.9.3
--
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 0/15] SCSI XCOPY support for the kernel and device mapper
On Tue, Jul 15 2014 at 3:34pm -0400, Mikulas Patocka mpato...@redhat.com wrote: This patch series makes it possible to use SCSI XCOPY offload for the block layer and device mapper. It is based on Martin Petersen's work https://git.kernel.org/cgit/linux/kernel/git/mkp/linux.git/commit/?h=xcopyid=0bdeed274e16b3038a851552188512071974eea8, but it is changed significantly so that it is possible to propagate XCOPY bios through the device mapper stack. The basic architecture is this: in the function blkdev_issue_copy we create two bios, one for read and one for write (with bi_rw READ|REQ_COPY and WRITE|REQ_COPY). Both bios have a pointer to the same bio_copy structure. These two bios travel independently through the device mapper stack - each bio can go through different device mapper devices. When both the bios reach the physical block device (in the function blk_queue_bio) the bio pair is collected and a XCOPY request is allocated and sent to the scsi disk driver. Note that because device mapper mapping can dynamically change, there no guarantee that the XCOPY command succeeds. If it ends with an error, the caller is supposed to perform the copying manually. The dm-kcopyd subsystem is modified to use the XCOPY command, so device mapper targets that use it (mirror, snapshot, thin, cache) take advantage of copy offload automatically. There is a new ioctl BLKCOPY that makes it possible to use copy offload from userspace. Hi Martin (and others on linux-scsi), It would be ideal for XCOPY support to make its way upstream for 3.18.. but the window for staging this work in time is closing. Any chance you might have some time to review Mikulas' revised approach to your initial XCOPY support? -- 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 11/22] Implement scsi_opcode_sa_name
On 14-08-28 01:33 PM, Hannes Reinecke wrote:
Implement a lookup array for SERVICE ACTION commands instead
of hardcoding it in a large switch statement.
Signed-off-by: Hannes Reinecke h...@suse.de
---
drivers/scsi/constants.c | 130 +++
1 file changed, 53 insertions(+), 77 deletions(-)
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 323e944..813c482 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -244,102 +244,77 @@ static const struct value_name_pair
variable_length_arr[] = {
};
#define VARIABLE_LENGTH_SZ ARRAY_SIZE(variable_length_arr)
-static const char * get_sa_name(const struct value_name_pair * arr,
- int arr_sz, int service_action)
+struct sa_name_list {
+ int cmd;
+ const struct value_name_pair *arr;
+ int arr_sz;
+};
+
+static struct sa_name_list sa_names_arr[] = {
+ {VARIABLE_LENGTH_CMD, variable_length_arr, VARIABLE_LENGTH_SZ},
+ {MAINTENANCE_IN, maint_in_arr, MAINT_IN_SZ},
+ {MAINTENANCE_OUT, maint_out_arr, MAINT_OUT_SZ},
+ {PERSISTENT_RESERVE_IN, pr_in_arr, PR_IN_SZ},
+ {PERSISTENT_RESERVE_OUT, pr_out_arr, PR_OUT_SZ},
+ {SERVICE_ACTION_IN_12, serv_in12_arr, SERV_IN12_SZ},
+ {SERVICE_ACTION_OUT_12, serv_out12_arr, SERV_OUT12_SZ},
+ {SERVICE_ACTION_BIDIRECTIONAL, serv_bidi_arr, SERV_BIDI_SZ},
+ {SERVICE_ACTION_IN_16, serv_in16_arr, SERV_IN16_SZ},
+ {SERVICE_ACTION_OUT_16, serv_out16_arr, SERV_OUT16_SZ},
+ {THIRD_PARTY_COPY_IN, tpc_in_arr, TPC_IN_SZ},
+ {THIRD_PARTY_COPY_OUT, tpc_out_arr, TPC_OUT_SZ},
+ {0, NULL, 0},
+};
Plus I added these recently (after observing the output from
REPORT SUPPORTED OPERATION CODES):
READ BUFFER
WRITE BUFFER
SANITIZE
[And I'll take your lead and remove the big switch
statement from sg3_utils.]
Doug Gilbert
/* Read buffer [0x3c] service actions */
struct sg_lib_value_name_t sg_lib_read_buff_arr[] = {
{0x0, 0, combined header and data [or multiple modes]},
{0x2, 0, data},
{0x3, 0, descriptor},
{0xa, 0, read data from echo buffer},
{0xb, 0, echo buffer descriptor},
{0x1a, 0, enable expander comms protocol and echo buffer},
{0x1c, 0, error history},
{0x, 0, NULL},
};
/* Write buffer [0x3b] service actions */
struct sg_lib_value_name_t sg_lib_write_buff_arr[] = {
{0x0, 0, combined header and data [or multiple modes]},
{0x2, 0, data},
{0x4, 0, download microcode and activate},
{0x5, 0, download microcode, save, and activate},
{0x6, 0, download microcode with offsets and activate},
{0x7, 0, download microcode with offsets, save, and activate},
{0xa, 0, write data to echo buffer},
{0xd, 0, download microcode with offsets, select activation events,
save and defer activate},
{0xe, 0, download microcode with offsets, save and defer activate},
{0xf, 0, activate deferred microcode},
{0x1a, 0, enable expander comms protocol and echo buffer},
{0x1b, 0, disable expander comms protocol},
{0x1c, 0, download application client error history},
{0x, 0, NULL},
};
/* Sanitize [0x48] service actions */
struct sg_lib_value_name_t sg_lib_sanitize_sa_arr[] = {
{0x1, 0, Sanitize, overwrite},
{0x2, 0, Sanitize, block erase},
{0x3, 0, Sanitize, cryptographic erase},
{0x1f, 0, Sanitize, exit failure mode},
{0x, 0, NULL},
};
--
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: Block/SCSI data integrity update v3
On Thu, Aug 28, 2014 at 02:10:49PM -0600, Jens Axboe wrote: On 08/28/2014 01:31 PM, Martin K. Petersen wrote: This is the data integrity patch series originally submitted for 3.16 and 3.17. It has been rebased on top of block/for-3.18/core. Other than that there are no changes from v2. Thanks, picked up. I did not apply 14/14 since that should go through the SCSI tree. I can carry it if need be, but it'd be nice to have a SCSI signoff on it then. I'd rather avoid introducing a dependency of the SCSI tree on the block one, so I'd prefer if you merge the last one as well. I'll try to find some time to review it ASAP. -- 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: [RFC PATCH 07/10] scsi/trace: Use scsi_show_result trace point instead of printk
I'm not sure this is the correct way. Currently we have quite some code duplication in scsi_trace.c and constants.c, correct. So I definitely would like to see them both merged. But constants.c is influenced by CONFIG_SCSI_CONSTANTS, whereas scsi_trace isn't, and the functions in constants.c are used throughout the scsi stack. So I'd rather see to have scsi_trace to be updated to use the functions from constants.c, and remove the duplicate code in scsi_trace. The tracepoints need to use the magic print_flags co helpers so that output works properly if using the binary tracebuffer and user space tools that decoded it (e.g. trace-cmd or perf), so using a kernel function for decoding is not an option. But we can make these tracepoints dependent on CONFIG_SCSI_CONSTANTS to still allow building lighter kernels if we really care about it. -- 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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.
On Tue, Aug 26, 2014 at 10:54:51PM +, KY Srinivasan wrote: This looks pretty reasonable to me, but I wonder if we should move this up to common code so that it happens for any host rescan triggered by sysfs or other drivers as well. Ping. Any decision on if/when this patch may be checked in. I'll need a review or two for it still.. -- 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
[Bug 83391] Oops on sd_mod
https://bugzilla.kernel.org/show_bug.cgi?id=83391 --- Comment #4 from tomsun tomsunc...@gmail.com --- sorry, thank you very much! (In reply to Jeff Moyer from comment #3) This is a vendor kernel. You should file a bug report with Red Hat here: https://bugzilla.redhat.com/enter_bug. cgi?product=Red%20Hat%20Enterprise%20Linux%206 First, though, you should update to a newer kernel... that one is several years old. -- You are receiving this mail because: You are watching the assignee of the bug. -- 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
[Bug 83391] Oops on sd_mod
https://bugzilla.kernel.org/show_bug.cgi?id=83391 tomsun tomsunc...@gmail.com changed: What|Removed |Added Resolution|--- |INVALID Status|NEW |RESOLVED -- You are receiving this mail because: You are watching the assignee of the bug. -- 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 2/2] Drivers: scsi: storvsc: Force discovery of LUNs that may have been removed.
On 08/27/2014 09:31 AM, Hannes Reinecke wrote: On 08/19/2014 07:54 PM, Christoph Hellwig wrote: On Sat, Aug 16, 2014 at 08:09:48PM -0700, K. Y. Srinivasan wrote: The host asks the guest to scan when a LUN is removed or added. The only way a guest can identify the removed LUN is when an I/O is attempted on a removed LUN - the SRB status code indicates that the LUN is invalid. We currently handle this SRB status and remove the device. Rather than waiting for an I/O to remove the device, force the discovery of LUNs that may have been removed prior to discovering LUNs that may have been added. This looks pretty reasonable to me, but I wonder if we should move this up to common code so that it happens for any host rescan triggered by sysfs or other drivers as well. Not without proper testing. Currently we cannot rescan existing devices; the inquiry string is nailed to the sdev structure. The only way to really refresh the information is to delete it and rescan it again. How are distros handling 0x6/0x3f/0x0e (report luns changed) when it gets passed to userspace? Is everyone kicking off a new full (add and delete) scan to handle this or logging it? Is the driver returning this when the LUNs change? Also is the driver getting a 0x5/0x25/0 (invalid LUN) when the LUN does not exist, or is it just getting that SRB_STATUS_INVALID_LUN error code? -- 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: Debugging scsi abort handling ?
On Thu, 28 Aug 2014, James Bottomley wrote: I'm fine with not calling scsi_done from eh_abort, but I cannot guarantee that another thread will not complete the cmnd in the mean time before hand. This is expected. After error handling begins, the block layer ignores all done callbacks for commands under EH. This thread seems like FAQ's to me (I was looking for answers myself). It would be nice if the EH docs would clearly state something like, calling scsi_done on a command being aborted is a no-op. I found your post to this thread helpful, http://www.spinics.net/lists/linux-scsi/msg77305.html although it does raise more questions. Given that eh_abort_handler() may return SUCCESS or FAILURE, what are the implications for cmd-result? I took a look at esp_scsi to see if that would shed some light. That driver does the following in eh_abort_handler(). cmd-result = DID_ABORT 16; cmd-scsi_done(cmd); ... return SUCCESS; but only in certain circumstances. In others, it will wait for an active command to complete normally. eh_abort_handler() then returns SUCCESS if the command completes normally and quickly, and FAILURE if not (which seems semantically strange to me). In that driver, the command waited upon in eh_abort_handler() will never have result host byte == DID_ABORT. None of which makes much sense to me. Is it permissible to have normal command completion (host byte == DID_OK) and have eh_abort_handler() return SUCCESS? Conversely, is it okay to set_host_byte(cmd, DID_ABORT) and then return FAILURE from eh_abort_handler()? It would be nice if the docs could state unequivocally a aborted command is presumed to have no meaningful result (Is this true? If not, which bytes matter?) Another subtlety is that the abort handler is apparently expected to perform autosense for an aborted command (or wait for that to happen normally) and only return after this has taken place (rather than immediately killing the command). You can get the situation where we try to abort (or do other EH) on a command you think you've already completed, but you just return success for that. In general, the LLD cannot distinguish between a command previously completed and a command it has never encountered. But it seems that eh_abort_handler() should return SUCCESS either way. That seems like an unintuitive interpretation of successfully aborted so it perhaps this needs to be documented too. The drivers I've looked at (esp_scsi and ncr5380) don't do this. If asked to abort some command which they completed in the past, they would return failure. -- -- 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: Debugging scsi abort handling ?
On Thu, 28 Aug 2014, Hannes Reinecke wrote: What might happen, though, that the command is already dead and gone by the time you're calling -scsi_done() (if you call it after eh_abort). So there might not _be_ a command upon which you can call -scsi_done() to start with. Hence any LLDD need to clear up any internal references after a call to eh_XXX to ensure it doesn't call -scsi_done() an in invalid command. So even if the LLDD returns 'FAILED' upon a call to eh_XXX it _still_ needs to clear up the internal reference. This is a question that has been bothering me too. If the host's eh_abort_cmd() method returns FAILED, it seems the mid-layer is liable to re-issue the same command to the LLD (?) Either that or return 'FAILED' for any later eh_XXX function until the internal references can be cleared up. So if a command may or may not exist after eh_abort_handler() returns control to the mid-layer (regardless of SUCCESS or FAILURE), then the LLD has to be careful about keeping track of which commands were aborted, if those commands are still in the process of cleanup when eh_abort_handler() returns. It's hard to see how that can work when command pointers are only unique while a command exists. In effect, this would mean that EH functions cannot return at all, until the relevant command(s) are completely forgotten by the LLD; and that means the LLD itself may have to escalate abort - device reset - bus reset - etc instead of simply returning FAILED. -- -- 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: Debugging scsi abort handling ?
-Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Finn Thain Sent: Thursday, August 28, 2014 11:37 PM To: James Bottomley Cc: Hans de Goede; Hannes Reinecke; Paolo Bonzini; Bart Van Assche; SCSI development list Subject: Re: Debugging scsi abort handling ? On Thu, 28 Aug 2014, James Bottomley wrote: ... Another subtlety is that the abort handler is apparently expected to perform autosense for an aborted command (or wait for that to happen normally) and only return after this has taken place (rather than immediately killing the command). Sending a REQUEST SENSE and expecting to get sense data for another command only works in protocols like parallel SCSI that supported contingent allegiance - a feature that is obsolete in the SCSI architecture model. In modern SCSI protocols, that command just returns polled sense data like the status of background tasks, not sense data related to any particular command. James posted a patch a while back that bypasses sending that command and misinterpreting the result. You can get the situation where we try to abort (or do other EH) on a command you think you've already completed, but you just return success for that. In general, the LLD cannot distinguish between a command previously completed and a command it has never encountered. But it seems that eh_abort_handler() should return SUCCESS either way. That seems like an unintuitive interpretation of successfully aborted so it perhaps this needs to be documented too. The drivers I've looked at (esp_scsi and ncr5380) don't do this. If asked to abort some command which they completed in the past, they would return failure. That is how the SCSI architecture model defines ABORT TASK, so shouldn't be too surprising here. I wouldn't focus too much on those old parallel SCSI drivers; SAS, Fibre Channel, iSCSI, and SRP drivers are better examples now. --- Rob Elliott HP Server Storage -- 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
