Re: [PATCH 04/24] scsi: introduce sdev_prefix_printk()
On 10/03/2014 06:31 PM, Ewan Milne wrote: On Thu, 2014-10-02 at 18:37 +, Elliott, Robert (Server Storage) wrote: -Original Message- From: Hannes Reinecke [mailto:h...@suse.de] ... diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 4c3ab83..c01dc89 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -103,9 +103,10 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk) #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)-disk ? \ - sdev_printk(prefix, (sdsk)-device, [%s] fmt,\ - (sdsk)-disk-disk_name, ##a) : \ - sdev_printk(prefix, (sdsk)-device, fmt, ##a) + sdev_prefix_printk(prefix, (sdsk)-device,\ +(sdsk)-disk-disk_name, fmt, ##a) : \ + sdev_prefix_printk(prefix, (sdsk)-device,\ +NULL, fmt, ##a) #define sd_first_printk(prefix, sdsk, fmt, a...) \ do {\ ... diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 27ecee7..0b18a09 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -244,6 +244,15 @@ struct scsi_dh_data { #define sdev_dbg(sdev, fmt, a...) \ dev_dbg((sdev)-sdev_gendev, fmt, ##a) +/* + * like scmd_printk, but the device name is passed in + * as a string pointer + */ +#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 This triggers lots of compiler warnings with gcc 4.4.7 like: drivers/scsi/sd.c: In function 'sd_open': drivers/scsi/sd.c:1179: warning: reading through null pointer (argument 4) drivers/scsi/sd.c:1179: warning: format '%s' expects type 'char *', but argument 4 has type 'void *' That is from: SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, sd_open\n)); Since: #define NULL ((void *)0) gcc probably doesn't realize the (p)? prevents the NULL (a void *) from being passed to sdev_printk. Passing rather than NULL eliminates the compiler warnings. It eliminates the warnings, but unfortunately we then get log messages that look like: Oct 3 11:30:08 rhel-storage-01 kernel: sd 10:0:0:0: [sde] (null)FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE ^^ Changing it to (char *)NULL, like this: #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)-disk ? \ sdev_prefix_printk(prefix, (sdsk)-device,\ (sdsk)-disk-disk_name, fmt, ##a) : \ sdev_prefix_printk(prefix, (sdsk)-device,\ (char *)NULL, fmt, ##a) doesn't work either. The compiler gives an error: drivers/scsi/sd.c: In function 'sd_open': drivers/scsi/sd.c:1158:2: error: reading through null pointer (argument 4) [-Werror=format=] SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, sd_open\n)); ^ I've fixed it up in my next iteration of the patchset. (By simply using 'sdev_printk' if the prefix argument is NULL ...) 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: [PATCH 21/24] scsi: simplify scsi_log_(send|completion)
On 10/03/2014 04:32 AM, Elliott, Robert (Server Storage) wrote: -Original Message- From: Hannes Reinecke [mailto:h...@suse.de] Sent: Wednesday, 01 October, 2014 1:23 AM To: James Bottomley Cc: Christoph Hellwig; linux-scsi@vger.kernel.org; Elliott, Robert (Server Storage); Hannes Reinecke Subject: [PATCH 21/24] scsi: simplify scsi_log_(send|completion) Simplify scsi_log_(send|completion) by externalizing scsi_mlreturn_string() and always print the command address. Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 39 --- drivers/scsi/scsi.c | 43 ++- drivers/scsi/scsi_lib.c | 13 ++--- include/scsi/scsi_dbg.h | 3 ++- 4 files changed, 54 insertions(+), 44 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index b131900..207ebef 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1437,19 +1437,52 @@ const char *scsi_driverbyte_string(int result) } EXPORT_SYMBOL(scsi_driverbyte_string); -void scsi_print_result(struct scsi_cmnd *cmd) +#ifdef CONFIG_SCSI_CONSTANTS +#define scsi_mlreturn_name(result) { result, #result } +static const struct value_name_pair scsi_mlreturn_arr[] = { +scsi_mlreturn_name(NEEDS_RETRY), +scsi_mlreturn_name(SUCCESS), +scsi_mlreturn_name(FAILED), +scsi_mlreturn_name(QUEUED), +scsi_mlreturn_name(SOFT_ERROR), +scsi_mlreturn_name(ADD_TO_MLQUEUE), +scsi_mlreturn_name(TIMEOUT_ERROR), +scsi_mlreturn_name(SCSI_RETURN_NOT_HANDLED), +scsi_mlreturn_name(FAST_IO_FAIL) +}; SUCCESS is a misleading name to print on commands that were really not successful. Example: [ 5978.573297] sd 2:0:0:8: [sdz] tag#209 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK [ 5978.576335] sd 2:0:0:8: [sdz] tag#209 CDB: Test Unit Ready 00 00 00 00 00 00 [ 5978.578721] sd 2:0:0:8: [sdz] tag#209 Sense Key : Unit Attention [current] [ 5978.581177] sd 2:0:0:8: [sdz] tag#209 Add. Sense: Bus device reset function occurred Maybe that mlreturn value should be printed as COMPLETE since it is similar to the SAM service responses called COMMAND COMPLETE and FUNCTION COMPLETE. That is a more neutral term. I do agree that SUCCESS is rather misleading here. However, the midlayer always used 'SUCCESS' here, and changing that would be a different patch as it's bound to induce quite some discussion. I'm happy to send such a patch, but preferably _after_ this patchset is in. 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
[no subject]
commit 72f79f9e35bd3f78ee8853f2fcacaa197d23ebac upstream. Subject: [PATCH 3.16 350/357] ahci_xgene: Removing NCQ support from the APM X-Gene SoC AHCI SATA Host Controller driver. This patch removes the NCQ support from the APM X-Gene SoC AHCI Host Controller driver as it doesn't support it. Signed-off-by: Loc Ho l...@apm.com Signed-off-by: Suman Tripathi stripa...@apm.com Signed-off-by: Tejun Heo t...@kernel.org [bwh: Backported to 3.16: host flags are passed to ahci_platform_init_host()] Signed-off-by: Ben Hutchings b...@decadent.org.uk Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org Acked-by: Suman Tripathi stripa...@apm.com --- drivers/ata/ahci_xgene.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/drivers/ata/ahci_xgene.c +++ b/drivers/ata/ahci_xgene.c @@ -337,7 +337,7 @@ static struct ata_port_operations xgene_ }; static const struct ata_port_info xgene_ahci_port_info = { - .flags = AHCI_FLAG_COMMON | ATA_FLAG_NCQ, + .flags = AHCI_FLAG_COMMON, .pio_mask = ATA_PIO4, .udma_mask = ATA_UDMA6, .port_ops = xgene_ahci_ops, @@ -484,7 +484,7 @@ static int xgene_ahci_probe(struct platf goto disable_resources; } - hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ; + hflags = AHCI_HFLAG_NO_PMP | AHCI_HFLAG_NO_NCQ; rc = ahci_platform_init_host(pdev, hpriv, xgene_ahci_port_info, hflags, 0, 0); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 23/29] atari_scsi: Convert to platform device
On Sun, 5 Oct 2014, Michael Schmitz wrote: On Fri, 3 Oct 2014, Geert Uytterhoeven wrote: + if (ATARIHW_PRESENT(TT_SCSI)) { + atari_scsi_reg_read = atari_scsi_tt_reg_read; + atari_scsi_reg_write = atari_scsi_tt_reg_write; + } else if (ATARIHW_PRESENT(ST_SCSI)) { + atari_scsi_reg_read = atari_scsi_falcon_reg_read; + atari_scsi_reg_write = atari_scsi_falcon_reg_write; Can these be handled through the platform_device's resources? I don't know. Should be possible - I've seen that used in the ISP116x HCD driver (arch-dependent post-register access delay function passed via platform data). Yes, possible, but is it desirable? +#ifdef REAL_DMA + /* If running on a Falcon and if there's TT-Ram (i.e., more than one +* memory block, since there's always ST-Ram in a Falcon), then +* allocate a STRAM_BUFFER_SIZE byte dribble buffer for transfers +* from/to alternative Ram. +*/ + if (ATARIHW_PRESENT(ST_SCSI) !ATARIHW_PRESENT(EXTD_DMA) + m68k_num_memory 1) { + atari_dma_buffer = atari_stram_alloc(STRAM_BUFFER_SIZE, SCSI); + if (!atari_dma_buffer) { + pr_err(PFX can't allocate ST-RAM double buffer\n); + return -ENOMEM; + } + atari_dma_phys_buffer = atari_stram_to_phys(atari_dma_buffer); + atari_dma_orig_addr = 0; + } +#endif More platform data? Perhaps. + if (IS_A_TT()) + instance-irq = IRQ_TT_MFP_SCSI; + else + instance-irq = IRQ_NONE; platform_device resource? If I thought it possible to parameterize the driver such that it never had to test IS_A_TT(), I'd probably agree that this would be more elegant. Otherwise I'd prefer not to have parts of the logic separated off into the platform resources while the remaining logic remains in the driver itself. While I don't think platform resources are desirable in this driver, I would like to hear Michael's views. The IRQ is a good candidate to be passed via platform data. Geert didn't say so, but after thinking about his review comments I imagine that he wants all the Atari IRQ numbers kept in one place and all the Mac IRQ numbers in a different place, and so on. Makes sense. Register access primitives can be done via platform data as well. Likewise, the ST-DMA locking primitives. That still leaves the DMA setup and completion code - Falcon and TT differ here in that they require a different order of DMA setup and NCR setup (the Falcon SCSI chip is hooked up via the ST-DMA, the TT one memory mapped so DMA setup must come last on Falcon, and DMA completion check first. TT has that reversed. That's a bit more hassle and might require lib_NCR5380 approach similar to the ESP SCSI driver. Aside from TT and ST, is there a third configuration that might benefit from a more data driven configuration? TT and Falcon, not sure any of the ST/STE series ever had a SCSI adapter. Medusa is TT compatible If there's no third configuration then I see little to be gained from completely parameterizing the driver using a bunch of resources. What am I missing? Why would it be desirable to have bits of driver code in arch/m68k/ and other bits of the same driver kept elsewhere in the tree (in drivers/)? (and IRQ_NONE is wrong, you should use 0) + if (IS_A_TT()) { Check for instance-irq instead? Yes, you'd think so, but a later patch (not in this set) would have to change it back to IS_A_TT(). Further patches align atari_NCR5380.c with NCR5380.c, such that the core driver then checks host-irq to find out whether an IRQ is in use. For Atari ST, request_irq() is not called even though the ST DMA irq is in use. That's why the IRQ is set to 0, I guess? Works for me. My patch tests for IS_A_TT not 0 because 0 has a different meaning depending on which core driver you use. I don't want to support three forks of NCR5380.c. One of the objectives of this patch series is to try to move toward convergence on a common core driver. The old code states that setting instance-irq = 0 keeps the midlevel from tampering with the SCSI IRQ during queuecmd which would interfere with IDE and floppy. I don't know what you mean. AFAIK, the SCSI mid layer doesn't care about instance-irq. I guess this is still relevant - I would not have seen the ST-DMA locked by IDE during SCSI queuecmd otherwise. Lock-ups were due to disabling local IRQs. Please see, [PATCH 22/29] atari_scsi: Fix atari_scsi deadlocks on Falcon in this series and (from our
Re: [PATCH -next] scsi: ufs: fix configuring power mode after UIC link down
Hi Subhash, 2014-10-06 7:38 GMT+09:00 Subhash Jadavani subha...@codeaurora.org: Hi Akinobu, Thanks for the patch. After you reported the issue, I was looking through our driver to make sure that why this issue was not catched and in fact it's already have a fix internally and it was yet to be send upstream. I am fine with your patch but our approach to fix the mentioned issue looks more complete. Here is what how we fixed it: - After UFS link startup in ufshcd_probe(), link would anyway be in PWM-G1, 1-lane, SLOW-AUTO mode so we would call a function namated ufshcd_init_pwr_info() which would set the hba-pwr_info to reflect the correct power mode after link startup. Here is the code reference: https://www.codeaurora.org/cgit/quic/femto/kernel/msm-3.10/tree/drivers/scsi /ufs/ufshcd.c?h=caf/msm-3.10#n4818. It would be good if you can adapt this fix in your current patch. This looks more appropriate way to fix this problem. So I withdraw this patch. As this problem is not a regression, could you fix this in that way in your appropriate timing? Thanks for taking care of this. -- 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 23/29] atari_scsi: Convert to platform device
Hi Finn, Can these be handled through the platform_device's resources? I don't know. Should be possible - I've seen that used in the ISP116x HCD driver (arch-dependent post-register access delay function passed via platform data). Yes, possible, but is it desirable? Only long term, i.e. if we can find a way to fold all the separate *_NCR5380 drivers into a core one. If there's no third configuration then I see little to be gained from completely parameterizing the driver using a bunch of resources. What am I missing? Why would it be desirable to have bits of driver code in arch/m68k/ and other bits of the same driver kept elsewhere in the tree (in drivers/)? Again, only makes sense if the goal is to reduce the driver to a core driver with implementation bits separate. (and IRQ_NONE is wrong, you should use 0) + if (IS_A_TT()) { Check for instance-irq instead? Yes, you'd think so, but a later patch (not in this set) would have to change it back to IS_A_TT(). Further patches align atari_NCR5380.c with NCR5380.c, such that the core driver then checks host-irq to find out whether an IRQ is in use. For Atari ST, request_irq() is not called even though the ST DMA irq is in use. That's why the IRQ is set to 0, I guess? Works for me. My patch tests for IS_A_TT not 0 because 0 has a different meaning depending on which core driver you use. I don't want to support three forks of NCR5380.c. One of the objectives of this patch series is to try to move toward convergence on a common core driver. I wasn't talking about the use of IS_A_TT() here, rather about what host-irq = 0 achieves. Anyway, in the context of atari_scsi.c, host-irq == 0 is synonymous with !IS_A_TT(). If host-irq == 0 is problematic with the long term goal of a singe 5380 core driver, we'll just have to pick a value which means 'driver uses IRQ but you're not to fiddle with it'. The old code states that setting instance-irq = 0 keeps the midlevel from tampering with the SCSI IRQ during queuecmd which would interfere with IDE and floppy. I don't know what you mean. AFAIK, the SCSI mid layer doesn't care about instance-irq. You're right - that's another obsolete comment then. The midlevel uses spinlocks now for ages, this means we can set instance-irq to the actual IRQ used. I guess this is still relevant - I would not have seen the ST-DMA locked by IDE during SCSI queuecmd otherwise. Lock-ups were due to disabling local IRQs. Please see, Absolutely right, but that's not what I meant. I assumed the midlevel disabled interrupts during queuecommand - it does not, so the point is moot. If using the actual IRQ for instance-irq helps at all, no objection to that. SCSI commands can still be queued while IDE IO is in flight - that is what the 'ST-DMA locked by IDE' above refers to. Cheers, Michael -- 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 23/29] atari_scsi: Convert to platform device
On Mon, Oct 6, 2014 at 9:05 AM, Finn Thain fth...@telegraphics.com.au wrote: + if (IS_A_TT()) + instance-irq = IRQ_TT_MFP_SCSI; + else + instance-irq = IRQ_NONE; platform_device resource? If I thought it possible to parameterize the driver such that it never had to test IS_A_TT(), I'd probably agree that this would be more elegant. Otherwise I'd prefer not to have parts of the logic separated off into the platform resources while the remaining logic remains in the driver itself. While I don't think platform resources are desirable in this driver, I would like to hear Michael's views. The IRQ is a good candidate to be passed via platform data. Geert didn't say so, but after thinking about his review comments I imagine that he wants all the Atari IRQ numbers kept in one place and all the Mac IRQ numbers in a different place, and so on. Makes sense. In the end, it's the same SCSI chip. The only differences are resources, interrupts, DMA, and glue logic. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say programmer or something like that. -- Linus Torvalds -- 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/26] fas216: Return DID_ERROR for incomplete data transfer
fas216 returns DID_BAD_TARGET for an incomplete data transfer. The midlayer uses DID_BAD_TARGET to signal a non-existing or not reachable target. So we should rather be using DID_ERROR here. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/arm/fas216.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c index 71cfb1e..7fc6fd3 100644 --- a/drivers/scsi/arm/fas216.c +++ b/drivers/scsi/arm/fas216.c @@ -2085,8 +2085,7 @@ fas216_std_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, unsigned int result) 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; + set_host_byte(SCpnt, DID_ERROR); goto request_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
[PATCHv6 00/26] scsi logging update (the boring part)
Hi all, here is now the sixth iteration of my scsi logging update. For this I've fixed all issues noticed by Robert Elliott (thanks, Robert!) and Ewan Milne. And added the 'Reviewed-by:' tags where appropriate. Hannes Reinecke (26): Remove scsi_cmd_print_sense_hdr() sd: Remove scsi_print_sense() in sd_done() aha152x: Debug output update and whitespace cleanup scsi: introduce sdev_prefix_printk() scsi: Use sdev as argument for sense code printing acornscsi: use scsi_print_command() fas216: Return DID_ERROR for incomplete data transfer fas216: Update logging messages 53c700: remove scsi_print_sense() usage scsi: stop decoding if scsi_normalize_sense() fails scsi: do not decode sense extras scsi: use 'bool' as return value for scsi_normalize_sense() scsi: remove scsi_print_status() Implement scsi_opcode_sa_name scsi: merge print_opcode_name() scsi: consolidate opcode lookup in scsi_opcode_sa_name() scsi: repurpose the last argument from print_opcode_name() scsi: Remove scsi_print_command when calling abort scsi: separate out scsi_(host|driver)byte_string() sd: Cleanup logging scsi: simplify scsi_log_(send|completion) scsi: fixup logging messages in scsi_error.c scsi: use shost argument in scsi_eh_prt_fail_stats scsi_error: document scsi_try_to_abort_cmd scsi: check for corrent return code in scsi_eh_abort_cmds() scsi: correct return values for .eh_abort_handler implementations drivers/scsi/53c700.c | 13 +- drivers/scsi/NCR5380.c| 7 +- drivers/scsi/aha152x.c| 994 +- drivers/scsi/aha1740.c| 2 +- drivers/scsi/arm/acornscsi.c | 12 +- drivers/scsi/arm/fas216.c | 39 +- drivers/scsi/atari_NCR5380.c | 3 +- drivers/scsi/ch.c | 29 +- drivers/scsi/constants.c | 576 +- drivers/scsi/esas2r/esas2r_main.c | 2 +- drivers/scsi/hosts.c | 4 +- drivers/scsi/megaraid.c | 8 +- drivers/scsi/osst.c | 8 +- drivers/scsi/ps3rom.c | 4 - drivers/scsi/scsi.c | 45 +- drivers/scsi/scsi_error.c | 123 ++--- drivers/scsi/scsi_ioctl.c | 2 +- drivers/scsi/scsi_lib.c | 19 +- drivers/scsi/sd.c | 56 +-- drivers/scsi/sd.h | 6 +- drivers/scsi/sg.c | 6 +- drivers/scsi/sr.h | 3 +- drivers/scsi/sr_ioctl.c | 10 +- drivers/scsi/st.c | 9 +- drivers/scsi/stex.c | 9 +- drivers/scsi/storvsc_drv.c| 3 +- drivers/scsi/sun3_NCR5380.c | 3 +- include/scsi/scsi_dbg.h | 28 +- include/scsi/scsi_device.h| 9 + include/scsi/scsi_eh.h| 14 +- 30 files changed, 686 insertions(+), 1360 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 10/26] scsi: stop decoding if scsi_normalize_sense() fails
If scsi_normalize_sense() fails we couldn't decode the sense buffer, and the scsi_sense_hdr fields are invalid. For those cases we should rather dump the sense buffer and not try to decode invalid fields. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 34 -- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index f9e2317..0b74e94 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1435,26 +1435,21 @@ scsi_print_sense_hdr(const 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(const unsigned char *sense_buffer, int sense_len) { - int k, num, res; - - 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]); + int k, num; + + 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(\n); - return; + printk(%02x , sense_buffer[k]); } + printk(\n); + return; } static void @@ -1524,7 +1519,10 @@ void __scsi_print_sense(const struct scsi_device *sdev, const char *name, { struct scsi_sense_hdr sshdr; - scsi_decode_sense_buffer(sense_buffer, sense_len, sshdr); + if (!scsi_normalize_sense(sense_buffer, sense_len, sshdr)) { + scsi_dump_sense_buffer(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 08/26] fas216: Update logging messages
Update logging messages to use dev_printk() variants for correct device annotations. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/arm/fas216.c | 26 +++--- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c index 7fc6fd3..cea3463 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,11 @@ 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); set_host_byte(SCpnt, DID_ERROR); goto request_sense; } @@ -2157,12 +2155,11 @@ static void fas216_done(FAS216_Info *info, unsigned int result) * to transfer, we should not have a valid pointer. */ if (info-scsi.SCp.ptr info-scsi.SCp.this_residual == 0) { - printk(scsi%d.%c: zero bytes left to transfer, but - buffer pointer still valid: ptr=%p len=%08x CDB: , - info-host-host_no, '0' + SCpnt-device-id, - info-scsi.SCp.ptr, info-scsi.SCp.this_residual); + scmd_printk(KERN_INFO, SCpnt, + zero bytes left to transfer, but buffer pointer still valid: ptr=%p len=%08x\n, + info-scsi.SCp.ptr, info-scsi.SCp.this_residual); info-scsi.SCp.ptr = NULL; - __scsi_print_command(SCpnt-cmnd); + scsi_print_command(SCpnt); } /* @@ -2663,8 +2660,7 @@ int fas216_eh_host_reset(struct scsi_cmnd *SCpnt) fas216_checkmagic(info); - printk(scsi%d.%c: %s: resetting host\n, - info-host-host_no, '0' + SCpnt-device-id, __func__); + fas216_log(info, LOG_ERROR, resetting host); /* * Reset the SCSI chip. -- 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 04/26] scsi: introduce sdev_prefix_printk()
Like scmd_printk(), but the device name is passed in as a string. Can be used by eg ULDs which do not have access to the scsi_cmnd structure. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/ch.c | 3 +-- drivers/scsi/sd.h | 6 +++--- drivers/scsi/sg.c | 4 ++-- drivers/scsi/sr.h | 3 +-- drivers/scsi/st.c | 3 +-- include/scsi/scsi_device.h | 9 + 6 files changed, 17 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c index ef5ae0d..52060e7 100644 --- a/drivers/scsi/ch.c +++ b/drivers/scsi/ch.c @@ -85,8 +85,7 @@ static const char * vendor_labels[CH_TYPES-4] = { // module_param_string_array(vendor_labels, NULL, 0444); #define ch_printk(prefix, ch, fmt, a...) \ - sdev_printk(prefix, (ch)-device, [%s] fmt, \ - (ch)-name, ##a) + sdev_prefix_printk(prefix, (ch)-device, (ch)-name, fmt, ##a) #define DPRINTK(fmt, arg...) \ do { \ diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h index 4c3ab83..19d20af 100644 --- a/drivers/scsi/sd.h +++ b/drivers/scsi/sd.h @@ -103,9 +103,9 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk) #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)-disk ? \ - sdev_printk(prefix, (sdsk)-device, [%s] fmt,\ - (sdsk)-disk-disk_name, ##a) : \ - sdev_printk(prefix, (sdsk)-device, fmt, ##a) + sdev_prefix_printk(prefix, (sdsk)-device,\ +(sdsk)-disk-disk_name, fmt, ##a) : \ + sdev_printk(prefix, (sdsk)-device, fmt, ##a) #define sd_first_printk(prefix, sdsk, fmt, a...) \ do {\ diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 01cf888..87304c8 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -219,8 +219,8 @@ static void sg_device_destroy(struct kref *kref); #define SZ_SG_REQ_INFO sizeof(sg_req_info_t) #define sg_printk(prefix, sdp, fmt, a...) \ - sdev_printk(prefix, (sdp)-device, [%s] fmt, \ - (sdp)-disk-disk_name, ##a) + sdev_prefix_printk(prefix, (sdp)-device, \ + (sdp)-disk-disk_name, fmt, ##a) static int sg_allow_access(struct file *filp, unsigned char *cmd) { diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h index 1d1f6f4..1de3371 100644 --- a/drivers/scsi/sr.h +++ b/drivers/scsi/sr.h @@ -57,8 +57,7 @@ typedef struct scsi_cd { } Scsi_CD; #define sr_printk(prefix, cd, fmt, a...) \ - sdev_printk(prefix, (cd)-device, [%s] fmt, \ - (cd)-cdi.name, ##a) + sdev_prefix_printk(prefix, (cd)-device, (cd)-cdi.name, fmt, ##a) int sr_do_ioctl(Scsi_CD *, struct packet_command *); diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c index d3fd6e8..e479343 100644 --- a/drivers/scsi/st.c +++ b/drivers/scsi/st.c @@ -306,8 +306,7 @@ static inline char *tape_name(struct scsi_tape *tape) } #define st_printk(prefix, t, fmt, a...) \ - sdev_printk(prefix, (t)-device, %s: fmt, \ - tape_name(t), ##a) + sdev_prefix_printk(prefix, (t)-device, tape_name(t), fmt, ##a) #ifdef DEBUG #define DEBC_printk(t, fmt, a...) \ if (debugging) { st_printk(ST_DEB_MSG, t, fmt, ##a ); } diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 27ecee7..0b18a09 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -244,6 +244,15 @@ struct scsi_dh_data { #define sdev_dbg(sdev, fmt, a...) \ dev_dbg((sdev)-sdev_gendev, fmt, ##a) +/* + * like scmd_printk, but the device name is passed in + * as a string pointer + */ +#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 11/26] 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 sense extras; the tape driver does its own decoding anyway. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 62 1 file changed, 62 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 0b74e94..ec69ec8 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1452,67 +1452,6 @@ scsi_dump_sense_buffer(const unsigned char *sense_buffer, int sense_len) return; } -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(const struct scsi_device *sdev, const char *name, const unsigned char *sense_buffer, int sense_len) @@ -1524,7 +1463,6 @@ void __scsi_print_sense(const 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); } 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 01/26] Remove scsi_cmd_print_sense_hdr()
Unused. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@infradead.org 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 05/26] scsi: Use sdev as argument for sense code printing
We should be using the standard dev_printk() variants for sense code printing. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@infradead.org Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/53c700.c | 2 +- drivers/scsi/ch.c | 2 +- drivers/scsi/constants.c | 117 ++--- 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 --- include/scsi/scsi_eh.h | 2 +- 15 files changed, 97 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 52060e7..53621a3 100644 --- a/drivers/scsi/ch.c +++ b/drivers/scsi/ch.c @@ -206,7 +206,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..f9e2317 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,53 @@ 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(const struct scsi_device *sdev, const char *name, +unsigned char asc, unsigned char ascq) { -const char *extd_sense_fmt = scsi_extd_sense_format(asc, ascq); + const char *extd_sense_fmt = NULL; + const char *extd_sense_str = scsi_extd_sense_format(asc, ascq, + extd_sense_fmt); + + if (extd_sense_str) { + if (extd_sense_fmt) + sdev_prefix_printk(KERN_INFO, sdev, name, + Add. Sense: %s (%s%x), + extd_sense_str, extd_sense_fmt, +
[PATCH 09/26] 53c700: remove scsi_print_sense() usage
The 53c700 driver would be using scsi_print_sense() in a debug statement, which was never compiled in. Plus the same information can get retrieved with logging. So remove it. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/53c700.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c index 68bf423..179a24e 100644 --- a/drivers/scsi/53c700.c +++ b/drivers/scsi/53c700.c @@ -592,19 +592,14 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata, hostdata-cmd = NULL; if(SCp != NULL) { - struct NCR_700_command_slot *slot = + struct NCR_700_command_slot *slot = (struct NCR_700_command_slot *)SCp-host_scribble; - + dma_unmap_single(hostdata-dev, slot-pCmd, MAX_COMMAND_SIZE, DMA_TO_DEVICE); if (slot-flags == NCR_700_FLAG_AUTOSENSE) { char *cmnd = NCR_700_get_sense_cmnd(SCp-device); -#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(SCp); -#endif dma_unmap_single(hostdata-dev, slot-dma_handle, SCSI_SENSE_BUFFERSIZE, DMA_FROM_DEVICE); /* restore the old result if the request sense was -- 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 06/26] acornscsi: use scsi_print_command()
Update acornscsi to use scsi_print_command() instead of the underscore version and use scmd_printk() in acornscsi_done(). This will add correct device annotations in the resulting message. And we should be using set_host_byte() for setting the final result. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/arm/acornscsi.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/arm/acornscsi.c b/drivers/scsi/arm/acornscsi.c index d89b9b4..deaaf84 100644 --- a/drivers/scsi/arm/acornscsi.c +++ b/drivers/scsi/arm/acornscsi.c @@ -850,13 +850,13 @@ 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, SCpnt-result); + scsi_print_command(SCpnt); acornscsi_dumpdma(host, done); - acornscsi_dumplog(host, SCpnt-device-id); - SCpnt-result = 0x; - SCpnt-result |= DID_ERROR 16; + acornscsi_dumplog(host, SCpnt-device-id); + set_host_byte(SCpnt, DID_ERROR); } } } -- 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 16/26] scsi: consolidate opcode lookup in scsi_opcode_sa_name()
Consolidate the CDB opcode lookup in scsi_opcode_sa_name(), so that we don't have to call several functions to figure out the CDB opcode string. Reviewed_by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellweg h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 37 ++--- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 3b22740..6d34d1f 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -29,6 +29,7 @@ #define THIRD_PARTY_COPY_OUT 0x83 #define THIRD_PARTY_COPY_IN 0x84 +#define VENDOR_SPECIFIC_CDB 0xc0 struct sa_name_list { int opcode; @@ -286,12 +287,19 @@ static struct sa_name_list sa_names_arr[] = { #endif /* CONFIG_SCSI_CONSTANTS */ static bool scsi_opcode_sa_name(int opcode, int service_action, - const char **sa_name) + const char **cdb_name, const char **sa_name) { struct sa_name_list *sa_name_ptr = sa_names_arr; const struct value_name_pair *arr = NULL; int arr_sz, k; + *cdb_name = NULL; + if (opcode = VENDOR_SPECIFIC_CDB) + return false; + + if (opcode ARRAY_SIZE(cdb_byte0_names)) + *cdb_name = cdb_byte0_names[opcode]; + for (k = 0; sa_name_ptr-opcode; ++k, ++sa_name_ptr) { if (sa_name_ptr-opcode == opcode) { arr = sa_name_ptr-arr; @@ -316,7 +324,7 @@ static bool scsi_opcode_sa_name(int opcode, int service_action, static void print_opcode_name(unsigned char * cdbp, int cdb_len) { int sa, len, cdb0; - const char *name = NULL; + const char *cdb_name = NULL, *sa_name = NULL; cdb0 = cdbp[0]; if (cdb0 == VARIABLE_LENGTH_CMD) { @@ -332,21 +340,20 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) len = cdb_len; } - if (!scsi_opcode_sa_name(cdb0, sa, name)) { - if (cdb0 0xc0) { - if (ARRAY_SIZE(cdb_byte0_names) 1) { - name = cdb_byte0_names[cdb0]; - if (name) - printk(%s, name); - else - printk(cdb[0]=0x%x (reserved), cdb0); - } else - printk(cdb[0]=0x%x, cdb0); - } else + if (!scsi_opcode_sa_name(cdb0, sa, cdb_name, sa_name)) { + if (cdb_name) + printk(%s, cdb_name); + else if (cdb0 = VENDOR_SPECIFIC_CDB) printk(cdb[0]=0x%x (vendor), cdb0); + else if (cdb0 = 0x60 cdb0 0x7e) + printk(cdb[0]=0x%x (reserved), cdb0); + else + printk(cdb[0]=0x%x, cdb0); } else { - if (name) - printk(%s, name); + if (sa_name) + printk(%s, sa_name); + else if (cdb_name) + printk(%s, sa=0x%x, cdb_name, sa); else printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa); -- 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/26] scsi: simplify scsi_log_(send|completion)
Simplify scsi_log_(send|completion) by externalizing scsi_mlreturn_string() and always print the command address. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 39 --- drivers/scsi/scsi.c | 43 ++- drivers/scsi/scsi_lib.c | 13 ++--- include/scsi/scsi_dbg.h | 3 ++- 4 files changed, 54 insertions(+), 44 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 1ebdc5d..59b7f9e 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1439,19 +1439,52 @@ const char *scsi_driverbyte_string(int result) } EXPORT_SYMBOL(scsi_driverbyte_string); -void scsi_print_result(struct scsi_cmnd *cmd) +#ifdef CONFIG_SCSI_CONSTANTS +#define scsi_mlreturn_name(result) { result, #result } +static const struct value_name_pair scsi_mlreturn_arr[] = { + scsi_mlreturn_name(NEEDS_RETRY), + scsi_mlreturn_name(SUCCESS), + scsi_mlreturn_name(FAILED), + scsi_mlreturn_name(QUEUED), + scsi_mlreturn_name(SOFT_ERROR), + scsi_mlreturn_name(ADD_TO_MLQUEUE), + scsi_mlreturn_name(TIMEOUT_ERROR), + scsi_mlreturn_name(SCSI_RETURN_NOT_HANDLED), + scsi_mlreturn_name(FAST_IO_FAIL) +}; +#endif + +const char *scsi_mlreturn_string(int result) +{ +#ifdef CONFIG_SCSI_CONSTANTS + const struct value_name_pair *arr = scsi_mlreturn_arr; + int k; + + for (k = 0; k ARRAY_SIZE(scsi_mlreturn_arr); ++k, ++arr) { + if (result == arr-value) + return arr-name; + } +#endif + return NULL; +} +EXPORT_SYMBOL(scsi_mlreturn_string); + +void scsi_print_result(struct scsi_cmnd *cmd, const char *msg, int disposition) { + const char *mlret_string = scsi_mlreturn_string(disposition); const char *hb_string = scsi_hostbyte_string(cmd-result); const char *db_string = scsi_driverbyte_string(cmd-result); if (hb_string || db_string) scmd_printk(KERN_INFO, cmd, - Result: hostbyte=%s driverbyte=%s, + %s%s Result: hostbyte=%s driverbyte=%s, + msg, mlret_string ? mlret_string : UNKNOWN, hb_string ? hb_string : invalid, db_string ? db_string : invalid); else scmd_printk(KERN_INFO, cmd, - Result: hostbyte=0x%02x driverbyte=0x%02x, + %s%s Result: hostbyte=0x%02x driverbyte=0x%02x, + msg, mlret_string ? mlret_string : UNKNOWN, host_byte(cmd-result), driver_byte(cmd-result)); } EXPORT_SYMBOL(scsi_print_result); diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 61aeaf1..3d81a07 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -527,9 +527,9 @@ void scsi_log_send(struct scsi_cmnd *cmd) * * 1: nothing (match completion) * -* 2: log opcode + command of all commands +* 2: log opcode + command of all commands + cmd address * -* 3: same as 2 plus dump cmd address +* 3: same as 2 * * 4: same as 3 plus dump extra junk */ @@ -537,10 +537,8 @@ 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: ); - if (level 2) - printk(0x%p , cmd); - printk(\n); + scmd_printk(KERN_INFO, cmd, + Send: scmd 0x%p\n, cmd); scsi_print_command(cmd); if (level 3) { printk(KERN_INFO buffer = 0x%p, bufflen = %d, @@ -565,7 +563,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) * * 2: same as 1 but for all command completions. * -* 3: same as 2 plus dump cmd address +* 3: same as 2 * * 4: same as 3 plus dump extra junk */ @@ -574,36 +572,7 @@ 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: ); - if (level 2) - printk(0x%p , cmd); - /* -* Dump truncated values, so we usually fit within -* 80 chars. -*/ - switch
[PATCH 26/26] scsi: correct return values for .eh_abort_handler implementations
The .eh_abort_handler needs to return SUCCESS, FAILED, or FAST_IO_FAIL. So fixup all callers to adhere to this requirement. Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/NCR5380.c| 2 +- drivers/scsi/aha1740.c| 2 +- drivers/scsi/esas2r/esas2r_main.c | 2 +- drivers/scsi/megaraid.c | 8 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c index 50873bb..e2c9e73 100644 --- a/drivers/scsi/NCR5380.c +++ b/drivers/scsi/NCR5380.c @@ -2703,7 +2703,7 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) { * aborted flag and get back into our main loop. */ - return 0; + return SUCCESS; } #endif diff --git a/drivers/scsi/aha1740.c b/drivers/scsi/aha1740.c index 5f31017..31ace4b 100644 --- a/drivers/scsi/aha1740.c +++ b/drivers/scsi/aha1740.c @@ -531,7 +531,7 @@ static int aha1740_eh_abort_handler (Scsi_Cmnd *dummy) * quiet as possible... */ - return 0; + return SUCCESS; } static struct scsi_host_template aha1740_template = { diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c index 6504a19..45aa684 100644 --- a/drivers/scsi/esas2r/esas2r_main.c +++ b/drivers/scsi/esas2r/esas2r_main.c @@ -1057,7 +1057,7 @@ int esas2r_eh_abort(struct scsi_cmnd *cmd) cmd-scsi_done(cmd); - return 0; + return SUCCESS; } spin_lock_irqsave(a-queue_lock, flags); diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index ac5d94c..2485255 100644 --- a/drivers/scsi/megaraid.c +++ b/drivers/scsi/megaraid.c @@ -1945,7 +1945,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) cmd-device-id, (u32)cmd-device-lun); if(list_empty(adapter-pending_list)) - return FALSE; + return FAILED; list_for_each_safe(pos, next, adapter-pending_list) { @@ -1968,7 +1968,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) (aor==SCB_ABORT) ? ABORTING:RESET, scb-idx); - return FALSE; + return FAILED; } else { @@ -1993,12 +1993,12 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) list_add_tail(SCSI_LIST(cmd), adapter-completed_list); - return TRUE; + return SUCCESS; } } } - return FALSE; + return FAILED; } static inline int -- 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 20/26] sd: Cleanup logging
Open-code scsi_print_result in sd.c, and cleanup logging to not print duplicate informations. With that we can remove scsi_show_result in constants.c Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 16 drivers/scsi/sd.c| 46 -- include/scsi/scsi_dbg.h | 1 - 3 files changed, 24 insertions(+), 39 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 3244ec2..1ebdc5d 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1439,22 +1439,6 @@ const char *scsi_driverbyte_string(int result) } EXPORT_SYMBOL(scsi_driverbyte_string); -void scsi_show_result(int result) -{ - const char *hb_string = scsi_hostbyte_string(result); - const char *db_string = scsi_driverbyte_string(result); - - if (hb_string || db_string) - printk(Result: hostbyte=%s driverbyte=%s\n, - hb_string ? hb_string : invalid, - db_string ? db_string : invalid); - else - printk(Result: hostbyte=0x%02x driverbyte=0x%02x\n, - host_byte(result), driver_byte(result)); -} -EXPORT_SYMBOL(scsi_show_result); - - void scsi_print_result(struct scsi_cmnd *cmd) { const char *hb_string = scsi_hostbyte_string(cmd-result); diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 848b17d..20b0539 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -116,7 +116,7 @@ static int sd_eh_action(struct scsi_cmnd *, int); static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer); static void scsi_disk_release(struct device *cdev); static void sd_print_sense_hdr(struct scsi_disk *, struct scsi_sense_hdr *); -static void sd_print_result(struct scsi_disk *, int); +static void sd_print_result(struct scsi_disk *, const char *, int); static DEFINE_SPINLOCK(sd_index_lock); static DEFINE_IDA(sd_index_ida); @@ -1479,7 +1479,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp) } if (res) { - sd_print_result(sdkp, res); + sd_print_result(sdkp, SYNCHRONIZE_CACHE failed, res); if (driver_byte(res) DRIVER_SENSE) sd_print_sense_hdr(sdkp, sshdr); @@ -1700,17 +1700,9 @@ static int sd_done(struct scsi_cmnd *SCpnt) if (sense_valid) sense_deferred = scsi_sense_is_deferred(sshdr); } -#ifdef CONFIG_SCSI_LOGGING - SCSI_LOG_HLCOMPLETE(1, scsi_print_result(SCpnt)); - 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)); - } -#endif + SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt, + sd_done: completed %d bytes\n, + good_bytes)); sdkp-medium_access_timed_out = 0; if (driver_byte(result) != DRIVER_SENSE @@ -1820,12 +1812,12 @@ sd_spinup_disk(struct scsi_disk *sdkp) /* no sense, TUR either succeeded or failed * with a status error */ if(!spintime !scsi_status_is_good(the_result)) { - sd_printk(KERN_NOTICE, sdkp, Unit Not Ready\n); - sd_print_result(sdkp, the_result); + sd_print_result(sdkp, Unit Not Ready, + the_result); } break; } - + /* * The device does not want the automatic start to be issued. */ @@ -1941,7 +1933,7 @@ static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp, struct scsi_sense_hdr *sshdr, int sense_valid, int the_result) { - sd_print_result(sdkp, the_result); + sd_print_result(sdkp, READ CAPACITY failed, the_result); if (driver_byte(the_result) DRIVER_SENSE) sd_print_sense_hdr(sdkp, sshdr); else @@ -3126,8 +3118,7 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) res = scsi_execute_req_flags(sdp, cmd, DMA_NONE, NULL, 0, sshdr, SD_TIMEOUT, SD_MAX_RETRIES, NULL, REQ_PM); if (res) { - sd_printk(KERN_WARNING, sdkp, START_STOP FAILED\n); - sd_print_result(sdkp, res); +
[PATCH 03/26] aha152x: Debug output update and whitespace cleanup
Remove all uncommented debugging code and move all printk() statements over to dev_printk(). And while we're at it we should be doing a whitespace cleanup, too. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/aha152x.c | 994 +++-- 1 file changed, 224 insertions(+), 770 deletions(-) diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c index e77b72f..2b960b3 100644 --- a/drivers/scsi/aha152x.c +++ b/drivers/scsi/aha152x.c @@ -230,7 +230,7 @@ * * ** - + see Documentation/scsi/aha152x.txt for configuration details **/ @@ -279,45 +279,11 @@ static LIST_HEAD(aha152x_host_list); #error define AUTOCONF or SETUP0 #endif -#if defined(AHA152X_DEBUG) -#define DEBUG_DEFAULT debug_eh - -#define DPRINTK(when,msgs...) \ - do { if(HOSTDATA(shpnt)-debug (when)) printk(msgs); } while(0) - -#define DO_LOCK(flags) \ - do { \ - if(spin_is_locked(QLOCK)) { \ - DPRINTK(debug_intr, DEBUG_LEAD (%s:%d) already locked at %s:%d\n, CMDINFO(CURRENT_SC), __func__, __LINE__, QLOCKER, QLOCKERL); \ - } \ - DPRINTK(debug_locking, DEBUG_LEAD (%s:%d) locking\n, CMDINFO(CURRENT_SC), __func__, __LINE__); \ - spin_lock_irqsave(QLOCK,flags); \ - DPRINTK(debug_locking, DEBUG_LEAD (%s:%d) locked\n, CMDINFO(CURRENT_SC), __func__, __LINE__); \ - QLOCKER=__func__; \ - QLOCKERL=__LINE__; \ - } while(0) - -#define DO_UNLOCK(flags) \ - do { \ - DPRINTK(debug_locking, DEBUG_LEAD (%s:%d) unlocking (locked at %s:%d)\n, CMDINFO(CURRENT_SC), __func__, __LINE__, QLOCKER, QLOCKERL); \ - spin_unlock_irqrestore(QLOCK,flags); \ - DPRINTK(debug_locking, DEBUG_LEAD (%s:%d) unlocked\n, CMDINFO(CURRENT_SC), __func__, __LINE__); \ - QLOCKER=(not locked); \ - QLOCKERL=0; \ - } while(0) - -#else -#define DPRINTK(when,msgs...) #defineDO_LOCK(flags) spin_lock_irqsave(QLOCK,flags) #defineDO_UNLOCK(flags)spin_unlock_irqrestore(QLOCK,flags) -#endif #define LEAD (scsi%d:%d:%d) -#define WARN_LEAD KERN_WARNINGLEAD #define INFO_LEAD KERN_INFO LEAD -#define NOTE_LEAD KERN_NOTICE LEAD -#define ERR_LEAD KERN_ERRLEAD -#define DEBUG_LEAD KERN_DEBUG LEAD #define CMDINFO(cmd) \ (cmd) ? ((cmd)-device-host-host_no) : -1, \ (cmd) ? ((cmd)-device-id 0x0f) : -1, \ @@ -345,10 +311,10 @@ CMD_INC_RESID(struct scsi_cmnd *cmd, int inc) enum { not_issued = 0x0001, /* command not yet issued */ - selecting = 0x0002, /* target is beeing selected */ + selecting = 0x0002, /* target is being selected */ identified = 0x0004, /* IDENTIFY was sent */ disconnected= 0x0008, /* target disconnected */ - completed = 0x0010, /* target sent COMMAND COMPLETE */ + completed = 0x0010, /* target sent COMMAND COMPLETE */ aborted = 0x0020, /* ABORT was sent */ resetted= 0x0040, /* BUS DEVICE RESET was sent */ spiordy = 0x0080, /* waiting for SPIORDY to raise */ @@ -396,7 +362,6 @@ static int exttrans[] = {0, 0}; module_param_array(exttrans, int, NULL, 0); MODULE_PARM_DESC(exttrans,use extended translation); -#if !defined(AHA152X_DEBUG) static int aha152x[] = {0, 11, 7, 1, 1, 0, DELAY_DEFAULT, 0}; module_param_array(aha152x, int, NULL, 0); MODULE_PARM_DESC(aha152x, parameters for first controller); @@ -404,19 +369,6 @@ MODULE_PARM_DESC(aha152x, parameters for first controller); static int aha152x1[] = {0, 11, 7, 1, 1, 0, DELAY_DEFAULT, 0}; module_param_array(aha152x1, int, NULL, 0); MODULE_PARM_DESC(aha152x1, parameters for second controller); -#else -static int debug[] = {DEBUG_DEFAULT, DEBUG_DEFAULT}; -module_param_array(debug, int, NULL, 0); -MODULE_PARM_DESC(debug, flags for driver debugging); - -static int aha152x[] = {0, 11, 7, 1, 1, 1, DELAY_DEFAULT, 0, DEBUG_DEFAULT}; -module_param_array(aha152x, int, NULL, 0); -MODULE_PARM_DESC(aha152x, parameters for first controller); - -static int aha152x1[] = {0, 11, 7, 1, 1, 1, DELAY_DEFAULT, 0, DEBUG_DEFAULT}; -module_param_array(aha152x1, int, NULL, 0); -MODULE_PARM_DESC(aha152x1, parameters for second controller); -#endif /* !defined(AHA152X_DEBUG) */ #endif /* MODULE */ #ifdef __ISAPNP__ @@ -446,7 +398,7 @@ static struct scsi_host_template aha152x_driver_template; /* * internal states of the host * - */ + */ enum aha152x_state { idle=0, unknown,
[PATCH 18/26] scsi: Remove scsi_print_command when calling abort
Calling scsi_print_command should not be necessary during abort; if the information is required one should enable scsi logging. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/53c700.c| 4 +--- drivers/scsi/NCR5380.c | 5 ++--- drivers/scsi/arm/fas216.c| 10 +++--- drivers/scsi/atari_NCR5380.c | 3 +-- drivers/scsi/ps3rom.c| 4 drivers/scsi/stex.c | 9 +++-- drivers/scsi/sun3_NCR5380.c | 3 +-- 7 files changed, 11 insertions(+), 27 deletions(-) diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c index 179a24e..474cc6d 100644 --- a/drivers/scsi/53c700.c +++ b/drivers/scsi/53c700.c @@ -1906,9 +1906,7 @@ NCR_700_abort(struct scsi_cmnd * SCp) { struct NCR_700_command_slot *slot; - scmd_printk(KERN_INFO, SCp, - New error handler wants to abort command\n\t); - scsi_print_command(SCp); + scmd_printk(KERN_INFO, SCp, abort command\n); slot = (struct NCR_700_command_slot *)SCp-host_scribble; diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c index 45da3c8..50873bb 100644 --- a/drivers/scsi/NCR5380.c +++ b/drivers/scsi/NCR5380.c @@ -2666,9 +2666,8 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) { struct Scsi_Host *instance = cmd-device-host; struct NCR5380_hostdata *hostdata = (struct NCR5380_hostdata *) instance-hostdata; Scsi_Cmnd *tmp, **prev; - - printk(KERN_WARNING scsi%d : aborting command\n, instance-host_no); - scsi_print_command(cmd); + + scmd_printk(KERN_WARNING, cmd, aborting command\n); NCR5380_print_status(instance); diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c index d2581cb..e64c3af 100644 --- a/drivers/scsi/arm/fas216.c +++ b/drivers/scsi/arm/fas216.c @@ -2423,14 +2423,11 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt) info-stats.aborts += 1; - printk(KERN_WARNING scsi%d: abort command , info-host-host_no); - __scsi_print_command(SCpnt-cmnd, SCpnt-cmd_len); + scmd_printk(KERN_WARNING, SCpnt, abort command\n); print_debug_list(); fas216_dumpstate(info); - printk(KERN_WARNING scsi%d: abort %p , info-host-host_no, SCpnt); - switch (fas216_find_command(info, SCpnt)) { /* * We found the command, and cleared it out. Either @@ -2438,7 +2435,7 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt) * target, or the busylun bit is not set. */ case res_success: - printk(success\n); + scmd_printk(KERN_WARNING, SCpnt, abort %p success\n, SCpnt); result = SUCCESS; break; @@ -2448,14 +2445,13 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt) * if the bus is free. */ case res_hw_abort: - /* * We are unable to abort the command for some reason. */ default: case res_failed: - printk(failed\n); + scmd_printk(KERN_WARNING, SCpnt, abort %p failed\n, SCpnt); break; } diff --git a/drivers/scsi/atari_NCR5380.c b/drivers/scsi/atari_NCR5380.c index 79e6f04..229c61b 100644 --- a/drivers/scsi/atari_NCR5380.c +++ b/drivers/scsi/atari_NCR5380.c @@ -2623,8 +2623,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd) Scsi_Cmnd *tmp, **prev; unsigned long flags; - printk(KERN_NOTICE scsi%d: aborting command\n, HOSTNO); - scsi_print_command(cmd); + scmd_printk(KERN_NOTICE, cmd, aborting command\n); NCR5380_print_status(instance); diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c index ef23fab..b3b48b5 100644 --- a/drivers/scsi/ps3rom.c +++ b/drivers/scsi/ps3rom.c @@ -220,10 +220,6 @@ static int ps3rom_queuecommand_lck(struct scsi_cmnd *cmd, unsigned char opcode; int res; -#ifdef DEBUG - scsi_print_command(cmd); -#endif - priv-curr_cmd = cmd; cmd-scsi_done = done; diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c index 1aa4bef..713af13 100644 --- a/drivers/scsi/stex.c +++ b/drivers/scsi/stex.c @@ -1162,9 +1162,7 @@ static int stex_abort(struct scsi_cmnd *cmd) int result = SUCCESS; unsigned long flags; - printk(KERN_INFO DRV_NAME - (%s): aborting command\n, pci_name(hba-pdev)); - scsi_print_command(cmd); + scmd_printk(KERN_INFO, cmd, aborting command\n); base = hba-mmio_base; spin_lock_irqsave(host-host_lock, flags); @@ -1352,9 +1350,8 @@ static int stex_reset(struct scsi_cmnd *cmd) hba = (struct st_hba *) cmd-device-host-hostdata[0]; - printk(KERN_INFO DRV_NAME - (%s): resetting host\n, pci_name(hba-pdev)); - scsi_print_command(cmd); + shost_printk(KERN_INFO, cmd-device-host, +resetting host\n); return
[PATCH 19/26] scsi: separate out scsi_(host|driver)byte_string()
Export functions for later use. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 58 include/scsi/scsi_dbg.h | 2 ++ 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 84b2614..3244ec2 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1406,38 +1406,68 @@ static const char * const hostbyte_table[]={ DID_PASSTHROUGH, DID_SOFT_ERROR, DID_IMM_RETRY, DID_REQUEUE, DID_TRANSPORT_DISRUPTED, DID_TRANSPORT_FAILFAST, DID_TARGET_FAILURE, DID_NEXUS_FAILURE }; -#define NUM_HOSTBYTE_STRS ARRAY_SIZE(hostbyte_table) static const char * const driverbyte_table[]={ DRIVER_OK, DRIVER_BUSY, DRIVER_SOFT, DRIVER_MEDIA, DRIVER_ERROR, DRIVER_INVALID, DRIVER_TIMEOUT, DRIVER_HARD, DRIVER_SENSE}; -#define NUM_DRIVERBYTE_STRS ARRAY_SIZE(driverbyte_table) -void scsi_show_result(int result) +#endif + +const char *scsi_hostbyte_string(int result) { + const char *hb_string = NULL; +#ifdef CONFIG_SCSI_CONSTANTS int hb = host_byte(result); - int db = driver_byte(result); - printk(Result: hostbyte=%s driverbyte=%s\n, - (hb NUM_HOSTBYTE_STRS ? hostbyte_table[hb] : invalid), - (db NUM_DRIVERBYTE_STRS ? driverbyte_table[db] : invalid)); + if (hb ARRAY_SIZE(hostbyte_table)) + hb_string = hostbyte_table[hb]; +#endif + return hb_string; } +EXPORT_SYMBOL(scsi_hostbyte_string); -#else +const char *scsi_driverbyte_string(int result) +{ + const char *db_string = NULL; +#ifdef CONFIG_SCSI_CONSTANTS + int db = driver_byte(result); + + if (db ARRAY_SIZE(driverbyte_table)) + db_string = driverbyte_table[db]; +#endif + return db_string; +} +EXPORT_SYMBOL(scsi_driverbyte_string); void scsi_show_result(int result) { - printk(Result: hostbyte=0x%02x driverbyte=0x%02x\n, - host_byte(result), driver_byte(result)); -} + const char *hb_string = scsi_hostbyte_string(result); + const char *db_string = scsi_driverbyte_string(result); -#endif + if (hb_string || db_string) + printk(Result: hostbyte=%s driverbyte=%s\n, + hb_string ? hb_string : invalid, + db_string ? db_string : invalid); + else + printk(Result: hostbyte=0x%02x driverbyte=0x%02x\n, + host_byte(result), driver_byte(result)); +} 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 *hb_string = scsi_hostbyte_string(cmd-result); + const char *db_string = scsi_driverbyte_string(cmd-result); + + if (hb_string || db_string) + scmd_printk(KERN_INFO, cmd, + Result: hostbyte=%s driverbyte=%s, + hb_string ? hb_string : invalid, + db_string ? db_string : invalid); + else + scmd_printk(KERN_INFO, cmd, + Result: hostbyte=0x%02x driverbyte=0x%02x, + host_byte(cmd-result), driver_byte(cmd-result)); } EXPORT_SYMBOL(scsi_print_result); diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h index 81d0418..50f4d85 100644 --- a/include/scsi/scsi_dbg.h +++ b/include/scsi/scsi_dbg.h @@ -19,6 +19,8 @@ extern void __scsi_print_sense(const struct scsi_device *, const char *name, int sense_len); extern void scsi_show_result(int); extern void scsi_print_result(struct scsi_cmnd *); +extern const char *scsi_hostbyte_string(int); +extern const char *scsi_driverbyte_string(int); extern const char *scsi_sense_key_string(unsigned char); extern const char *scsi_extd_sense_format(unsigned char, unsigned char, 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 23/26] scsi: use shost argument in scsi_eh_prt_fail_stats
The EH statistics are per host, so we should be using shost_printk() here. Suggested-by: Robert Elliott elli...@hp.com Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/scsi_error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 087ca02..c485f28 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -355,7 +355,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost, if (cmd_cancel || cmd_failed) { SCSI_LOG_ERROR_RECOVERY(3, - sdev_printk(KERN_INFO, sdev, + shost_printk(KERN_INFO, shost, %s: cmds failed: %d, cancel: %d\n, __func__, cmd_failed, cmd_cancel)); -- 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/26] scsi: merge print_opcode_name()
Instead of having two versions of print_opcode_name() we should be consolidating them into one version. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 90 ++-- 1 file changed, 34 insertions(+), 56 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 1a1ddfb..3b22740 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -30,6 +30,11 @@ #define THIRD_PARTY_COPY_IN 0x84 +struct sa_name_list { + int opcode; + const struct value_name_pair *arr; + int arr_sz; +}; #ifdef CONFIG_SCSI_CONSTANTS static const char * cdb_byte0_names[] = { @@ -244,12 +249,6 @@ static const struct value_name_pair variable_length_arr[] = { }; #define VARIABLE_LENGTH_SZ ARRAY_SIZE(variable_length_arr) -struct sa_name_list { - int opcode; - 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}, @@ -266,6 +265,26 @@ static struct sa_name_list sa_names_arr[] = { {0, NULL, 0}, }; +#else /* ifndef CONFIG_SCSI_CONSTANTS */ +static const char *cdb_byte0_names[]; + +static struct sa_name_list sa_names_arr[] = { + {VARIABLE_LENGTH_CMD, NULL, 0}, + {MAINTENANCE_IN, NULL, 0}, + {MAINTENANCE_OUT, NULL, 0}, + {PERSISTENT_RESERVE_IN, NULL, 0}, + {PERSISTENT_RESERVE_OUT, NULL, 0}, + {SERVICE_ACTION_IN_12, NULL, 0}, + {SERVICE_ACTION_OUT_12, NULL, 0}, + {SERVICE_ACTION_BIDIRECTIONAL, NULL, 0}, + {SERVICE_ACTION_IN_16, NULL, 0}, + {SERVICE_ACTION_OUT_16, NULL, 0}, + {THIRD_PARTY_COPY_IN, NULL, 0}, + {THIRD_PARTY_COPY_OUT, NULL, 0}, + {0, NULL, 0}, +}; +#endif /* CONFIG_SCSI_CONSTANTS */ + static bool scsi_opcode_sa_name(int opcode, int service_action, const char **sa_name) { @@ -273,7 +292,7 @@ static bool scsi_opcode_sa_name(int opcode, int service_action, const struct value_name_pair *arr = NULL; int arr_sz, k; - for (k = 0; sa_name_ptr-arr; ++k, ++sa_name_ptr) { + for (k = 0; sa_name_ptr-opcode; ++k, ++sa_name_ptr) { if (sa_name_ptr-opcode == opcode) { arr = sa_name_ptr-arr; arr_sz = sa_name_ptr-arr_sz; @@ -315,11 +334,14 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) if (!scsi_opcode_sa_name(cdb0, sa, name)) { if (cdb0 0xc0) { - name = cdb_byte0_names[cdb0]; - if (name) - printk(%s, name); - else - printk(cdb[0]=0x%x (reserved), cdb0); + if (ARRAY_SIZE(cdb_byte0_names) 1) { + name = cdb_byte0_names[cdb0]; + if (name) + printk(%s, name); + else + printk(cdb[0]=0x%x (reserved), cdb0); + } else + printk(cdb[0]=0x%x, cdb0); } else printk(cdb[0]=0x%x (vendor), cdb0); } else { @@ -333,50 +355,6 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) } } -#else /* ifndef CONFIG_SCSI_CONSTANTS */ - -static void print_opcode_name(unsigned char * cdbp, int cdb_len) -{ - int sa, len, cdb0; - - 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); - break; - } - sa = (cdbp[8] 8) + cdbp[9]; - printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa); - if (len != cdb_len) - printk(, in_cdb_len=%d, ext_len=%d, len, cdb_len); - break; - case MAINTENANCE_IN: - case MAINTENANCE_OUT: - case PERSISTENT_RESERVE_IN: - case PERSISTENT_RESERVE_OUT: - case SERVICE_ACTION_IN_12: - case SERVICE_ACTION_OUT_12: - case SERVICE_ACTION_BIDIRECTIONAL: - case SERVICE_ACTION_IN_16: - case SERVICE_ACTION_OUT_16: - case THIRD_PARTY_COPY_IN: - case THIRD_PARTY_COPY_OUT: - sa = cdbp[1] 0x1f; - printk(cdb[0]=0x%x, sa=0x%x, cdb0, sa); - break; - default: - if (cdb0 0xc0) - printk(cdb[0]=0x%x, cdb0); - else - printk(cdb[0]=0x%x (vendor), cdb0); -
[PATCH 14/26] Implement scsi_opcode_sa_name
Implement a lookup array for SERVICE ACTION commands instead of hardcoding it in a large switch statement. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@infradead.org Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 131 +++ 1 file changed, 53 insertions(+), 78 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 8253f7b..1a1ddfb 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -244,102 +244,76 @@ 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 opcode; + 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}, +}; + +static bool scsi_opcode_sa_name(int opcode, 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; sa_name_ptr-arr; ++k, ++sa_name_ptr) { + if (sa_name_ptr-opcode == opcode) { + arr = sa_name_ptr-arr; + arr_sz = sa_name_ptr-arr_sz; + break; + } + } + if (!arr) + return false; 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 true; } /* 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; + const char *name = NULL; 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,
[PATCH 12/26] scsi: use 'bool' as return value for scsi_normalize_sense()
Convert scsi_normalize_sense() and frieds to return 'bool' instead of an integer. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Yoshihiro Yunomae yoshihiro.yunomae...@hitachi.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/scsi_error.c | 16 drivers/scsi/scsi_lib.c | 2 +- include/scsi/scsi_eh.h| 14 +++--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 6c99624..a113e99 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -2408,20 +2408,20 @@ EXPORT_SYMBOL(scsi_reset_provider); * responded to a SCSI command with the CHECK_CONDITION status. * * Return value: - * 1 if valid sense data information found, else 0; + * true if valid sense data information found, else false; */ -int scsi_normalize_sense(const u8 *sense_buffer, int sb_len, - struct scsi_sense_hdr *sshdr) +bool scsi_normalize_sense(const u8 *sense_buffer, int sb_len, + struct scsi_sense_hdr *sshdr) { if (!sense_buffer || !sb_len) - return 0; + return false; memset(sshdr, 0, sizeof(struct scsi_sense_hdr)); sshdr-response_code = (sense_buffer[0] 0x7f); if (!scsi_sense_valid(sshdr)) - return 0; + return false; if (sshdr-response_code = 0x72) { /* @@ -2451,12 +2451,12 @@ int scsi_normalize_sense(const u8 *sense_buffer, int sb_len, } } - return 1; + return true; } EXPORT_SYMBOL(scsi_normalize_sense); -int scsi_command_normalize_sense(struct scsi_cmnd *cmd, -struct scsi_sense_hdr *sshdr) +bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd, + struct scsi_sense_hdr *sshdr) { return scsi_normalize_sense(cmd-sense_buffer, SCSI_SENSE_BUFFERSIZE, sshdr); diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index c3220b7..b853659 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -830,7 +830,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) struct request *req = cmd-request; int error = 0; struct scsi_sense_hdr sshdr; - int sense_valid = 0; + bool sense_valid = false; int sense_deferred = 0; enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY, ACTION_DELAYED_RETRY} action; diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h index 3d720ca..1d3c254 100644 --- a/include/scsi/scsi_eh.h +++ b/include/scsi/scsi_eh.h @@ -27,10 +27,10 @@ struct scsi_sense_hdr { /* See SPC-3 section 4.5 */ u8 additional_length; /* always 0 for fixed sense format */ }; -static inline int scsi_sense_valid(struct scsi_sense_hdr *sshdr) +static inline bool scsi_sense_valid(const struct scsi_sense_hdr *sshdr) { if (!sshdr) - return 0; + return false; return (sshdr-response_code 0x70) == 0x70; } @@ -42,12 +42,12 @@ extern void scsi_eh_flush_done_q(struct list_head *done_q); extern void scsi_report_bus_reset(struct Scsi_Host *, int); extern void scsi_report_device_reset(struct Scsi_Host *, int, int); extern int scsi_block_when_processing_errors(struct scsi_device *); -extern int scsi_normalize_sense(const u8 *sense_buffer, int sb_len, - struct scsi_sense_hdr *sshdr); -extern int scsi_command_normalize_sense(struct scsi_cmnd *cmd, - struct scsi_sense_hdr *sshdr); +extern bool scsi_normalize_sense(const u8 *sense_buffer, int sb_len, +struct scsi_sense_hdr *sshdr); +extern bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd, +struct scsi_sense_hdr *sshdr); -static inline int scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr) +static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr) { return ((sshdr-response_code = 0x70) (sshdr-response_code 1)); } -- 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/26] scsi: remove scsi_print_status()
Last caller is gone, so we can remove it. Reviewed-bt: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 35 --- include/scsi/scsi_dbg.h | 1 - 2 files changed, 36 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index ec69ec8..8253f7b 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -433,41 +433,6 @@ void scsi_print_command(struct scsi_cmnd *cmd) } 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). - **/ -void -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; - } - printk(KERN_INFO %s, ccp); -#else - printk(KERN_INFO 0x%0x, scsi_status); -#endif -} -EXPORT_SYMBOL(scsi_print_status); - #ifdef CONFIG_SCSI_CONSTANTS struct error_info { diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h index 6cbd179..386474e 100644 --- a/include/scsi/scsi_dbg.h +++ b/include/scsi/scsi_dbg.h @@ -19,7 +19,6 @@ extern void __scsi_print_sense(const struct scsi_device *, const char *name, int sense_len); extern void scsi_show_result(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); extern const char *scsi_extd_sense_format(unsigned char, unsigned char, 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 17/26] scsi: repurpose the last argument from print_opcode_name()
print_opcode_name() was only ever called with a '0' argument from LLDDs and ULDs which were _not_ supporting variable length CDBs, so the 'if' clause was never triggered. Instead we should be using the last argument to specify the cdb length to avoid accidental overflow when reading the cdb buffer. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/arm/fas216.c | 2 +- drivers/scsi/ch.c | 24 +--- drivers/scsi/constants.c | 25 ++--- drivers/scsi/sr_ioctl.c | 4 ++-- include/scsi/scsi_dbg.h | 2 +- 5 files changed, 27 insertions(+), 30 deletions(-) diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c index cea3463..d2581cb 100644 --- a/drivers/scsi/arm/fas216.c +++ b/drivers/scsi/arm/fas216.c @@ -2424,7 +2424,7 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt) info-stats.aborts += 1; printk(KERN_WARNING scsi%d: abort command , info-host-host_no); - __scsi_print_command(SCpnt-cmnd); + __scsi_print_command(SCpnt-cmnd, SCpnt-cmd_len); print_debug_list(); fas216_dumpstate(info); diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c index 53621a3..226ef77 100644 --- a/drivers/scsi/ch.c +++ b/drivers/scsi/ch.c @@ -182,7 +182,7 @@ static int ch_find_errno(struct scsi_sense_hdr *sshdr) } static int -ch_do_scsi(scsi_changer *ch, unsigned char *cmd, +ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len, void *buffer, unsigned buflength, enum dma_data_direction direction) { @@ -196,7 +196,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd, errno = 0; if (debug) { DPRINTK(command: ); - __scsi_print_command(cmd); + __scsi_print_command(cmd, cmd_len); } result = scsi_execute_req(ch-device, cmd, direction, buffer, @@ -257,7 +257,8 @@ ch_read_element_status(scsi_changer *ch, u_int elem, char *data) cmd[3] = elem 0xff; cmd[5] = 1; cmd[9] = 255; - if (0 == (result = ch_do_scsi(ch, cmd, buffer, 256, DMA_FROM_DEVICE))) { + if (0 == (result = ch_do_scsi(ch, cmd, 12, + buffer, 256, DMA_FROM_DEVICE))) { if (((buffer[16] 8) | buffer[17]) != elem) { DPRINTK(asked for element 0x%02x, got 0x%02x\n, elem,(buffer[16] 8) | buffer[17]); @@ -287,7 +288,7 @@ ch_init_elem(scsi_changer *ch) memset(cmd,0,sizeof(cmd)); cmd[0] = INITIALIZE_ELEMENT_STATUS; cmd[1] = (ch-device-lun 0x7) 5; - err = ch_do_scsi(ch, cmd, NULL, 0, DMA_NONE); + err = ch_do_scsi(ch, cmd, 6, NULL, 0, DMA_NONE); VPRINTK(KERN_INFO, ... finished\n); return err; } @@ -309,10 +310,10 @@ ch_readconfig(scsi_changer *ch) cmd[1] = (ch-device-lun 0x7) 5; cmd[2] = 0x1d; cmd[4] = 255; - result = ch_do_scsi(ch, cmd, buffer, 255, DMA_FROM_DEVICE); + result = ch_do_scsi(ch, cmd, 10, buffer, 255, DMA_FROM_DEVICE); if (0 != result) { cmd[1] |= (13); - result = ch_do_scsi(ch, cmd, buffer, 255, DMA_FROM_DEVICE); + result = ch_do_scsi(ch, cmd, 10, buffer, 255, DMA_FROM_DEVICE); } if (0 == result) { ch-firsts[CHET_MT] = @@ -437,7 +438,7 @@ ch_position(scsi_changer *ch, u_int trans, u_int elem, int rotate) cmd[4] = (elem 8) 0xff; cmd[5] = elem 0xff; cmd[8] = rotate ? 1 : 0; - return ch_do_scsi(ch, cmd, NULL, 0, DMA_NONE); + return ch_do_scsi(ch, cmd, 10, NULL, 0, DMA_NONE); } static int @@ -458,7 +459,7 @@ ch_move(scsi_changer *ch, u_int trans, u_int src, u_int dest, int rotate) cmd[6] = (dest 8) 0xff; cmd[7] = dest 0xff; cmd[10] = rotate ? 1 : 0; - return ch_do_scsi(ch, cmd, NULL,0, DMA_NONE); + return ch_do_scsi(ch, cmd, 12, NULL,0, DMA_NONE); } static int @@ -484,7 +485,7 @@ ch_exchange(scsi_changer *ch, u_int trans, u_int src, cmd[9] = dest20xff; cmd[10] = (rotate1 ? 1 : 0) | (rotate2 ? 2 : 0); - return ch_do_scsi(ch, cmd, NULL,0, DMA_NONE); + return ch_do_scsi(ch, cmd, 12, NULL, 0, DMA_NONE); } static void @@ -534,7 +535,7 @@ ch_set_voltag(scsi_changer *ch, u_int elem, memcpy(buffer,tag,32); ch_check_voltag(buffer); - result = ch_do_scsi(ch, cmd, buffer, 256, DMA_TO_DEVICE); + result = ch_do_scsi(ch, cmd, 12, buffer, 256, DMA_TO_DEVICE); kfree(buffer); return result; } @@ -765,7 +766,8 @@ static long ch_ioctl(struct file *file, ch_cmd[5] = 1; ch_cmd[9] = 255; - result = ch_do_scsi(ch, ch_cmd, buffer, 256, DMA_FROM_DEVICE); + result = ch_do_scsi(ch, ch_cmd, 12, +
[PATCH 22/26] scsi: fixup logging messages in scsi_error.c
Use the matching scope for logging messages to allow for better command tracing. Suggested-by: Robert Elliott elli...@hp.com Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/hosts.c | 4 +-- drivers/scsi/scsi_error.c | 77 +++ 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 6de80e3..06030e1 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -485,8 +485,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) WQ_UNBOUND | WQ_MEM_RECLAIM, 1, shost-host_no); if (!shost-tmf_work_q) { - printk(KERN_WARNING scsi%d: failed to create tmf workq\n, - shost-host_no); + shost_printk(KERN_WARNING, shost, +failed to create tmf workq\n); goto fail_kthread; } scsi_proc_hostdir_add(shost-hostt); diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index a113e99..087ca02 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1156,9 +1156,9 @@ int scsi_eh_get_sense(struct list_head *work_q, shost = scmd-device-host; if (scsi_host_eh_past_deadline(shost)) { SCSI_LOG_ERROR_RECOVERY(3, - shost_printk(KERN_INFO, shost, - skip %s, past eh deadline\n, -__func__)); + scmd_printk(KERN_INFO, scmd, + %s: skip request sense, past eh deadline\n, +current-comm)); break; } if (status_byte(scmd-result) != CHECK_CONDITION) @@ -1265,9 +1265,9 @@ static int scsi_eh_test_devices(struct list_head *cmd_list, /* Push items back onto work_q */ list_splice_init(cmd_list, work_q); SCSI_LOG_ERROR_RECOVERY(3, - shost_printk(KERN_INFO, sdev-host, -skip %s, past eh deadline, -__func__)); + sdev_printk(KERN_INFO, sdev, + %s: skip test device, past eh deadline, + current-comm)); break; } } @@ -1318,21 +1318,20 @@ static int scsi_eh_abort_cmds(struct list_head *work_q, if (scsi_host_eh_past_deadline(shost)) { list_splice_init(check_list, work_q); SCSI_LOG_ERROR_RECOVERY(3, - shost_printk(KERN_INFO, shost, - skip %s, past eh deadline\n, -__func__)); + scmd_printk(KERN_INFO, scmd, + %s: skip aborting cmd, past eh deadline\n, + current-comm)); return list_empty(work_q); } SCSI_LOG_ERROR_RECOVERY(3, - shost_printk(KERN_INFO, shost, -%s: aborting cmd: 0x%p\n, -current-comm, scmd)); + scmd_printk(KERN_INFO, scmd, +%s: aborting cmd\n, current-comm)); rtn = scsi_try_to_abort_cmd(shost-hostt, scmd); if (rtn == FAILED) { SCSI_LOG_ERROR_RECOVERY(3, - shost_printk(KERN_INFO, shost, -%s: aborting cmd failed: 0x%p\n, -current-comm, scmd)); + scmd_printk(KERN_INFO, scmd, + %s: aborting cmd failed\n, +current-comm)); list_splice_init(check_list, work_q); return list_empty(work_q); } @@ -1390,9 +1389,9 @@ static int scsi_eh_stu(struct Scsi_Host *shost, shost_for_each_device(sdev, shost) { if (scsi_host_eh_past_deadline(shost)) { SCSI_LOG_ERROR_RECOVERY(3, - shost_printk(KERN_INFO, shost, - skip %s, past eh deadline\n, -
[PATCH 25/26] scsi: check for corrent return code in scsi_eh_abort_cmds()
scsi_try_to_abort_cmd() might return SUCCESS, FAILED, or FAST_IO_FAIL. So just checking for FAILED will treat FAST_IO_FAIL as SUCCESS, which is wrong. Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/scsi_error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 9209407..06cd624 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -1345,7 +1345,7 @@ static int scsi_eh_abort_cmds(struct list_head *work_q, scmd_printk(KERN_INFO, scmd, %s: aborting cmd\n, current-comm)); rtn = scsi_try_to_abort_cmd(shost-hostt, scmd); - if (rtn == FAILED) { + if (rtn != SUCCESS) { SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd, %s: aborting cmd failed\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
Re: [PATCH 01/29] ncr5380: Use printk() not pr_debug()
On 10/02/2014 08:56 AM, Finn Thain wrote: Having defined NDEBUG, and having set the console log level, I'd like to see some output. Don't use pr_debug(), it's annoying to have to define DEBUG as well. Signed-off-by: Finn Thain fth...@telegraphics.com.au --- Use of pr_debug() here was a bad idea of mine. Joe was right when he questioned it. Reviewed-by: Hannes Reinecke h...@suse.de 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: [PATCH 02/29] ncr5380: Remove unused hostdata fields
On 10/02/2014 08:56 AM, Finn Thain wrote: Remove unused fields from hostdata structs declared with the NCR5380_implementation_fields macro. Signed-off-by: Finn Thain fth...@telegraphics.com.au Reviewed-by: Hannes Reinecke h...@suse.de 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: [PATCH 07/29] ncr5380: Cleanup TAG_NEXT and TAG_NONE macros
On 10/02/2014 08:56 AM, Finn Thain wrote: Both atari_NCR5380.c and sun3_NCR5380.c core drivers #undef TAG_NONE and then redefine it. But the original definition is unused because NCR5380.c lacks support for tagged queueing. So just define it once. The TAG_NEXT macro only appears in the arguments to NCR5380_select() calls. But that routine doesn't use its tag argument as the tag was already assigned in NCR5380_main(). So remove the unused argument and the macro. Signed-off-by: Finn Thain fth...@telegraphics.com.au Why not use 'SCSI_NO_TAG' from include/scsi/scsi_tcq.h ? 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: [PATCH 03/29] ncr5380: Fix compiler warnings and __setup options
On 10/02/2014 08:56 AM, Finn Thain wrote: Some __setup() options mentioned in Documentation/scsi don't work because a few lines of code went missing sometime since Linux 2.4. Fix the options and thus fix some compiler warnings for both the non-modular case, CC drivers/scsi/dtc.o drivers/scsi/dtc.c:176:20: warning: 'dtc_setup' defined but not used [-Wunused-function] and the modular case, CC [M] drivers/scsi/pas16.o drivers/scsi/pas16.c:335:20: warning: 'pas16_setup' defined but not used [-Wunused-function] CC [M] drivers/scsi/t128.o drivers/scsi/t128.c:147:20: warning: 't128_setup' defined but not used [-Wunused-function] Signed-off-by: Finn Thain fth...@telegraphics.com.au Reviewed-by: Hannes Reinecke h...@suse.de 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: [PATCH 04/29] ncr5380: Remove unused macros
On 10/02/2014 08:56 AM, Finn Thain wrote: Some macros are never evaluated (i.e. FOO, USLEEP, SCSI2 and USE_WRAPPER; and in some drivers, NCR5380_intr and NCR5380_proc_info). DRIVER_SETUP serves no purpose anymore. Remove these macro definitions. Signed-off-by: Finn Thain fth...@telegraphics.com.au Reviewed-by: Hannes Reinecke h...@suse.de 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: [PATCH 28/29] atari_NCR5380: Refactor Falcon special cases
On 10/02/2014 08:56 AM, Finn Thain wrote: Make the atari_NCR5380.c core driver usable by sun3_scsi, mac_scsi and others by moving some of the Falcon-specific code out of the core driver: !IS_A_TT, atari_read_overruns and falcon_dont_release. Replace these with hostdata variables and flags. FLAG_CHECK_LAST_BYTE_SENT is unused in atari_NCR5380.c so don't set it. Hmm. Shouldn't there be some more patches for sun3_scsi and mac_scsi to actually make use of this? I'd _love_ to get rid of all of this NCR5380 code duplication ... 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: [PATCH 8/8] IB/srp: Add multichannel support
On 10/02/14 19:30, Christoph Hellwig wrote: Also if we want to merge scsi LLDDs that can take advantage of multiqueue support it would probably be best if I take this via the SCSI tree. Sending these patches to you is fine with me, at least if Roland agrees. Bart. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 28/29] atari_NCR5380: Refactor Falcon special cases
On Mon, 6 Oct 2014, Hannes Reinecke wrote: On 10/02/2014 08:56 AM, Finn Thain wrote: Make the atari_NCR5380.c core driver usable by sun3_scsi, mac_scsi and others by moving some of the Falcon-specific code out of the core driver: !IS_A_TT, atari_read_overruns and falcon_dont_release. Replace these with hostdata variables and flags. FLAG_CHECK_LAST_BYTE_SENT is unused in atari_NCR5380.c so don't set it. Hmm. Shouldn't there be some more patches for sun3_scsi and mac_scsi to actually make use of this? Yes, I think you are right, some of those patches should be included. I've done some testing of mac_scsi and dmx3191d using the atari_NCR5380.c core driver but this isn't working adequately yet. It's not yet a proof of concept. To convert sun3_scsi to use the atari_NCR5380 core driver is easier. If I had atari and sun3 hardware to test on, everything would be done in one patch set and I could test the end result rather than asking others to test it in stages on the appropriate hardware. Anyway, I will include more patches when I re-submit: not just the sun3_NCR5380.c removal (which is ugly but hopefully Sam can simplify it) but also some more work on atari_NCR5380.c (to minimise testing effort). I'd _love_ to get rid of all of this NCR5380 code duplication ... My reading is that if mac_scsi can work with atari_NCR5380 then the same is very likely true for the ARM drivers (oak, cumana_1) and also dmx3191d. To merge atari_NCR5380.c with NCR5380.c would be much more difficult. If someone who has the hardware were to take an interest in the remaining drivers (just the ISA card drivers, dtc, pas16, g_NCR5380, t128), perhaps it would be feasible. -- Cheers, Hannes -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/29] ncr5380: Cleanup TAG_NEXT and TAG_NONE macros
On Mon, 6 Oct 2014, Hannes Reinecke wrote: On 10/02/2014 08:56 AM, Finn Thain wrote: Both atari_NCR5380.c and sun3_NCR5380.c core drivers #undef TAG_NONE and then redefine it. But the original definition is unused because NCR5380.c lacks support for tagged queueing. So just define it once. The TAG_NEXT macro only appears in the arguments to NCR5380_select() calls. But that routine doesn't use its tag argument as the tag was already assigned in NCR5380_main(). So remove the unused argument and the macro. Signed-off-by: Finn Thain fth...@telegraphics.com.au Why not use 'SCSI_NO_TAG' from include/scsi/scsi_tcq.h ? I didn't use SCSI_NO_TAG because it seemed to relate to the int tag member in struct request and not the unsigned char tag member in struct scsi_cmnd. In light of the plan to remove the tag member from struct scsi_cmnd, I don't know what to do with this patch. It doesn't really help with that plan so I figured it would be NAK'd. I haven't given any thought to the problem of converting drivers to block layer tags (which might involve SCSI_NO_TAG, I guess). I have too much other work in progress. That's why I suggested commenting out #define SUPPORT_TAGS until they could be converted. -- -- 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 28/29] atari_NCR5380: Refactor Falcon special cases
On Monday 06 October 2014, Finn Thain wrote: On Mon, 6 Oct 2014, Hannes Reinecke wrote: On 10/02/2014 08:56 AM, Finn Thain wrote: Make the atari_NCR5380.c core driver usable by sun3_scsi, mac_scsi and others by moving some of the Falcon-specific code out of the core driver: !IS_A_TT, atari_read_overruns and falcon_dont_release. Replace these with hostdata variables and flags. FLAG_CHECK_LAST_BYTE_SENT is unused in atari_NCR5380.c so don't set it. Hmm. Shouldn't there be some more patches for sun3_scsi and mac_scsi to actually make use of this? Yes, I think you are right, some of those patches should be included. I've done some testing of mac_scsi and dmx3191d using the atari_NCR5380.c core driver but this isn't working adequately yet. It's not yet a proof of concept. To convert sun3_scsi to use the atari_NCR5380 core driver is easier. If I had atari and sun3 hardware to test on, everything would be done in one patch set and I could test the end result rather than asking others to test it in stages on the appropriate hardware. Anyway, I will include more patches when I re-submit: not just the sun3_NCR5380.c removal (which is ugly but hopefully Sam can simplify it) but also some more work on atari_NCR5380.c (to minimise testing effort). I'd _love_ to get rid of all of this NCR5380 code duplication ... My reading is that if mac_scsi can work with atari_NCR5380 then the same is very likely true for the ARM drivers (oak, cumana_1) and also dmx3191d. To merge atari_NCR5380.c with NCR5380.c would be much more difficult. If someone who has the hardware were to take an interest in the remaining drivers (just the ISA card drivers, dtc, pas16, g_NCR5380, t128), perhaps it would be feasible. I have some NCR5380 ISA cards and did some fixes to the driver a couple of years ago. -- Ondrej Zary -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [dm-devel] [PATCH 1/1] multipath-tools: Change path checker for IBM IPR devices
On 10/01/2014 07:51 AM, Christoph Hellwig wrote: Unfortunately the patch wasn't quite correct - all TEST_UNIT_READY commands are sent as BLOCK_PC, so this would basically revert James' original fix for the SATL case. Am I right to assume you only need the call to scsi_dh-check_sense and not the rest of the handling for the multipath path checker? If that's the case something like the patch below sould work: This would work if we also duplicated the 02/04/02 K/C/Q check in alua_check_sense handler. Wendy - can you try my patch below, along with Christoph's latest patch here and see if that resolves the issue? Thanks, Brian diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 5db8454..399c1c8 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -459,14 +459,6 @@ static int scsi_check_sense(struct scsi_cmnd *scmd) if (! scsi_command_normalize_sense(scmd, sshdr)) return FAILED; /* no valid sense data */ - if (scmd-cmnd[0] == TEST_UNIT_READY scmd-scsi_done != scsi_eh_done) - /* - * nasty: for mid-layer issued TURs, we need to return the - * actual sense data without any recovery attempt. For eh - * issued ones, we need to try to recover and interpret - */ - return SUCCESS; - scsi_report_sense(sdev, sshdr); if (scsi_sense_is_deferred(sshdr)) @@ -482,6 +474,14 @@ static int scsi_check_sense(struct scsi_cmnd *scmd) /* handler does not care. Drop down to default handling */ } + if (scmd-cmnd[0] == TEST_UNIT_READY scmd-scsi_done != scsi_eh_done) + /* + * nasty: for mid-layer issued TURs, we need to return the + * actual sense data without any recovery attempt. For eh + * issued ones, we need to try to recover and interpret + */ + return SUCCESS; + /* * Previous logic looked for FILEMARK, EOM or ILI which are * mainly associated with tapes and returned SUCCESS. Signed-off-by: Brian King brk...@linux.vnet.ibm.com --- drivers/scsi/device_handler/scsi_dh_alua.c |7 +++ 1 file changed, 7 insertions(+) diff -puN drivers/scsi/device_handler/scsi_dh_alua.c~alua_allow_restart drivers/scsi/device_handler/scsi_dh_alua.c --- linux/drivers/scsi/device_handler/scsi_dh_alua.c~alua_allow_restart 2014-10-06 10:19:16.184798305 -0500 +++ linux-bjking1/drivers/scsi/device_handler/scsi_dh_alua.c2014-10-06 10:20:35.743165951 -0500 @@ -474,6 +474,13 @@ static int alua_check_sense(struct scsi_ * LUN Not Ready -- Offline */ return SUCCESS; + if (sdev-allow_restart + (sense_hdr-asc == 0x04) (sense_hdr-ascq == 0x02)) + /* +* if the device is not started, we need to wake +* the error handler to start the motor +*/ + return FAILED; break; case UNIT_ATTENTION: if (sense_hdr-asc == 0x29 sense_hdr-ascq == 0x00) _ -- 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 21/26] scsi: simplify scsi_log_(send|completion)
Rats. I found another case of (null) appearing in the log messages where I think you really just wanted no text at all to appear. Sorry I didn't find it last time along with the other one... -Ewan On Mon, 2014-10-06 at 11:01 +0200, Hannes Reinecke wrote: Simplify scsi_log_(send|completion) by externalizing scsi_mlreturn_string() and always print the command address. Reviewed-by: Robert Elliott elli...@hp.com Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Hannes Reinecke h...@suse.de --- drivers/scsi/constants.c | 39 --- drivers/scsi/scsi.c | 43 ++- drivers/scsi/scsi_lib.c | 13 ++--- include/scsi/scsi_dbg.h | 3 ++- 4 files changed, 54 insertions(+), 44 deletions(-) diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c index 1ebdc5d..59b7f9e 100644 --- a/drivers/scsi/constants.c +++ b/drivers/scsi/constants.c @@ -1439,19 +1439,52 @@ const char *scsi_driverbyte_string(int result) } EXPORT_SYMBOL(scsi_driverbyte_string); -void scsi_print_result(struct scsi_cmnd *cmd) +#ifdef CONFIG_SCSI_CONSTANTS +#define scsi_mlreturn_name(result) { result, #result } +static const struct value_name_pair scsi_mlreturn_arr[] = { + scsi_mlreturn_name(NEEDS_RETRY), + scsi_mlreturn_name(SUCCESS), + scsi_mlreturn_name(FAILED), + scsi_mlreturn_name(QUEUED), + scsi_mlreturn_name(SOFT_ERROR), + scsi_mlreturn_name(ADD_TO_MLQUEUE), + scsi_mlreturn_name(TIMEOUT_ERROR), + scsi_mlreturn_name(SCSI_RETURN_NOT_HANDLED), + scsi_mlreturn_name(FAST_IO_FAIL) +}; +#endif + +const char *scsi_mlreturn_string(int result) +{ +#ifdef CONFIG_SCSI_CONSTANTS + const struct value_name_pair *arr = scsi_mlreturn_arr; + int k; + + for (k = 0; k ARRAY_SIZE(scsi_mlreturn_arr); ++k, ++arr) { + if (result == arr-value) + return arr-name; + } +#endif + return NULL; +} +EXPORT_SYMBOL(scsi_mlreturn_string); + +void scsi_print_result(struct scsi_cmnd *cmd, const char *msg, int disposition) { + const char *mlret_string = scsi_mlreturn_string(disposition); const char *hb_string = scsi_hostbyte_string(cmd-result); const char *db_string = scsi_driverbyte_string(cmd-result); if (hb_string || db_string) scmd_printk(KERN_INFO, cmd, - Result: hostbyte=%s driverbyte=%s, + %s%s Result: hostbyte=%s driverbyte=%s, + msg, mlret_string ? mlret_string : UNKNOWN, hb_string ? hb_string : invalid, db_string ? db_string : invalid); else scmd_printk(KERN_INFO, cmd, - Result: hostbyte=0x%02x driverbyte=0x%02x, + %s%s Result: hostbyte=0x%02x driverbyte=0x%02x, + msg, mlret_string ? mlret_string : UNKNOWN, host_byte(cmd-result), driver_byte(cmd-result)); } EXPORT_SYMBOL(scsi_print_result); diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 61aeaf1..3d81a07 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -527,9 +527,9 @@ void scsi_log_send(struct scsi_cmnd *cmd) * * 1: nothing (match completion) * - * 2: log opcode + command of all commands + * 2: log opcode + command of all commands + cmd address * - * 3: same as 2 plus dump cmd address + * 3: same as 2 * * 4: same as 3 plus dump extra junk */ @@ -537,10 +537,8 @@ 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: ); - if (level 2) - printk(0x%p , cmd); - printk(\n); + scmd_printk(KERN_INFO, cmd, + Send: scmd 0x%p\n, cmd); scsi_print_command(cmd); if (level 3) { printk(KERN_INFO buffer = 0x%p, bufflen = %d, @@ -565,7 +563,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) * * 2: same as 1 but for all command completions. * - * 3: same as 2 plus dump cmd address + * 3: same as 2 * * 4: same as 3 plus dump extra junk */ @@ -574,36 +572,7 @@ 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: ); - if (level 2) -
Re: [PATCH v2 7/7] driver-core: add preferred async probe option for built-in and modules
Hello, Luis. The patchset generally looks good to me. Please feel free to add Reviewed-by: Tejun Heo t...@kernel.org A question below. On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote: + bus.enable_kern_async=1 [KNL] + This tells the kernel userspace has been vetted for + asynchronous probe support and can listen to the device + driver prefer_async_probe flag for both built-in device + drivers and modules. Do we intend to keep this param permanently? Isn't this more of a temp tool to be used during development? If so, maybe we should make that clear with __DEVEL__ too? Thanks. -- tejun -- 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 v2 7/7] driver-core: add preferred async probe option for built-in and modules
On Mon, Oct 06, 2014 at 04:19:26PM -0400, Tejun Heo wrote: Hello, Luis. The patchset generally looks good to me. Please feel free to add Reviewed-by: Tejun Heo t...@kernel.org Will do. A question below. On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote: + bus.enable_kern_async=1 [KNL] + This tells the kernel userspace has been vetted for + asynchronous probe support and can listen to the device + driver prefer_async_probe flag for both built-in device + drivers and modules. Do we intend to keep this param permanently? Isn't this more of a temp tool to be used during development? If so, maybe we should make that clear with __DEVEL__ too? As its designed right now no, its not a temp tool, its there to require compatibility with old userspace. For modules we can require the module parameter but for built-in we need something else and this is what came to mind. It is also what would allow the prefer_async_probe flag too as otherwise we won't know if userspace is prepared. If we want to get rid of it, it would mean that we're letting go of the idea that some userspace might exist which depends on *not* doing async probe. As such I would not consider it a __DEVEL__ param and it'd be a judgement call to eventually *not require* it. I can see that happening but perhaps a large series of kernels down the road? Luis -- 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 v2 7/7] driver-core: add preferred async probe option for built-in and modules
Hi Luis, On Fri, Oct 03, 2014 at 02:44:43PM -0700, Luis R. Rodriguez wrote: From: Luis R. Rodriguez mcg...@suse.com At times we may wish to express the desire to prefer to have a device driver probe asynchronously by default. We cannot simply enable all device drivers to do this without vetting that userspace is prepared for this though given that some old userspace is expected to exist which is not equipped to deal with broad async probe support. This defines a new kernel parameter, bus.enable_kern_async=1, to help address this both to help enable async probe support for built-in drivers and to enable drivers to specify a preference to opt in for async probe support. If you have a device driver that should use async probe support when possible enable the prefer_async_probe bool. Folks wishing to test enabling async probe for all built-in drivers can enable bus.__DEBUG__kernel_force_mod_async_probe=1, if you use that though you are on your own. Thank you for working on this. However there are still couple of issues with the async probe. 1. As far as I can see you only handle the case when device is already present and you load a driver. In this case we will do either async or sync probing, depending on the driver/module settings. However if driver has already been loaded/registered and we are adding a new device (another module load, for example you load i2c controller module and it enumerates its children, or driver signalled deferral during binding) the probe will be synchronous regardless of the settings. 2. I thin kin the current implementation deferred binding process is still single-threaded and basically synchronous. Both of these issues stem form the fact that you only plugging into bus_add_driver(), but you also need to plug into bus_probe_device(). I believe I handled these 2 cases properly in the version of patch I sent a couple of weeks ago so if you could incorporate that in your work that would be great. Thanks. -- Dmitry -- 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 v2 7/7] driver-core: add preferred async probe option for built-in and modules
Hello, On Mon, Oct 06, 2014 at 10:36:27PM +0200, Luis R. Rodriguez wrote: Do we intend to keep this param permanently? Isn't this more of a temp tool to be used during development? If so, maybe we should make that clear with __DEVEL__ too? As its designed right now no, its not a temp tool, its there to require compatibility with old userspace. For modules we can require the module parameter but for built-in we need something else and this is what came to mind. It is also what would allow the prefer_async_probe flag too as otherwise we won't know if userspace is prepared. I don't get it. For in-kernel stuff, we already have a clear synchronization point where we already synchronize all async calls. Shouldn't we be flushing these async probes there too? insmod'ing is userland visible but there's no reason this has to be for the built-in drivers. Thanks. -- tejun -- 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 v2 7/7] driver-core: add preferred async probe option for built-in and modules
On Mon, Oct 06, 2014 at 05:01:18PM -0400, Tejun Heo wrote: Hello, On Mon, Oct 06, 2014 at 10:36:27PM +0200, Luis R. Rodriguez wrote: Do we intend to keep this param permanently? Isn't this more of a temp tool to be used during development? If so, maybe we should make that clear with __DEVEL__ too? As its designed right now no, its not a temp tool, its there to require compatibility with old userspace. For modules we can require the module parameter but for built-in we need something else and this is what came to mind. It is also what would allow the prefer_async_probe flag too as otherwise we won't know if userspace is prepared. I don't get it. By prepared I meant that userspace can handle async probe, but you're right that we don't need to know that. I don't see how we'd be breaking old userspace by doing async probe of a driver is built-in right now... unless of course built-in always assumes all possible devices would be present after right before userspace init. For in-kernel stuff, we already have a clear synchronization point where we already synchronize all async calls. Shouldn't we be flushing these async probes there too? This seems to be addressing if what I meant by prepared, ready, so let me address this as I do think its important. By async calls do you mean users of async_schedule()? I see it also uses system_unbound_wq as well but I do not see anyone calling flush_workqueue(system_unbound_wq) on the kernel. We do use async_synchronize_full() on kernel_init() but that just waits. As it is we don't wait on init then, should we? Must we? Could / should we use bus.enable_kern_async=1 to enable avoiding having to wait ? At this point I'd prefer to address what we must do only. insmod'ing is userland visible but there's no reason this has to be for the built-in drivers. Good point. bus.enable_kern_async=1 would still also serve as a helper for the driver core to figure out if it should use async probe then on modules if prefer_async_probe was enabled. Let me know if you figure out a way to avoid it. Luis -- 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] target/file: fix inclusive vfs_fsync_range() end
Both of the file target's calls to vfs_fsync_range() got the end offset off by one. The range is inclusive, not exclusive. It would sync a bit more data than was required. The sync path already tested the length of the range and fell back to LLONG_MAX so I copied that pattern in the rw path. This is untested. I found the errors by inspection while following other code. Signed-off-by: Zach Brown z...@zabbo.net --- drivers/target/target_core_file.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 7d6cdda..176588f 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -415,7 +415,7 @@ fd_execute_sync_cache(struct se_cmd *cmd) } else { start = cmd-t_task_lba * dev-dev_attrib.block_size; if (cmd-data_length) - end = start + cmd-data_length; + end = start + cmd-data_length - 1; else end = LLONG_MAX; } @@ -680,7 +680,12 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents, struct fd_dev *fd_dev = FD_DEV(dev); loff_t start = cmd-t_task_lba * dev-dev_attrib.block_size; - loff_t end = start + cmd-data_length; + loff_t end; + + if (cmd-data_length) + end = start + cmd-data_length - 1; + else + end = LLONG_MAX; vfs_fsync_range(fd_dev-fd_file, start, end, 1); } -- 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