Re: [PATCH 0/7] SCSI: cleanup debugfs usage
On 22/01/2019 15:08, Greg Kroah-Hartman wrote: When calling debugfs code, there is no need to ever check the return value of the call, as no logic should ever change if a call works properly or not. Fix up a bunch of x86-specific code to not care about the results of debugfs. Greg Kroah-Hartman (7): scsi: bfa: no need to check return value of debugfs_create functions scsi: csiostor: no need to check return value of debugfs_create functions scsi: fnic: no need to check return value of debugfs_create functions scsi: snic: no need to check return value of debugfs_create functions scsi: lpfc: no need to check return value of debugfs_create functions scsi: qlogic: no need to check return value of debugfs_create functions scsi: qla2xxx: no need to check return value of debugfs_create functions JFYI, Martin has some more debugfs usage queued for 5.1 here: https://kernel.googlesource.com/pub/scm/linux/kernel/git/mkp/scsi/+log/5.1/scsi-queue/drivers/scsi/hisi_sas I'll send a patch to tidy-up, as above. It would not be x86 specific, but I am not sure what above is. Thanks, John drivers/scsi/bfa/bfad_debugfs.c | 17 --- drivers/scsi/csiostor/csio_init.c | 6 +- drivers/scsi/fnic/fnic_debugfs.c | 88 +--- drivers/scsi/fnic/fnic_main.c | 7 +- drivers/scsi/fnic/fnic_stats.h| 2 +- drivers/scsi/fnic/fnic_trace.c| 17 +-- drivers/scsi/fnic/fnic_trace.h| 4 +- drivers/scsi/lpfc/lpfc_debugfs.c | 170 -- drivers/scsi/qedf/qedf_debugfs.c | 18 +--- drivers/scsi/qedi/qedi_debugfs.c | 17 +-- drivers/scsi/qla2xxx/qla_dfs.c| 43 +--- drivers/scsi/snic/snic_debugfs.c | 133 +-- drivers/scsi/snic/snic_main.c | 14 +-- drivers/scsi/snic/snic_stats.h| 2 +- drivers/scsi/snic/snic_trc.c | 12 +-- drivers/scsi/snic/snic_trc.h | 4 +- 16 files changed, 48 insertions(+), 506 deletions(-)
[PATCH] scsi: dpt_i2o: clean up indentation issues, remove spaces
From: Colin Ian King There are several lines where the indentation has an extra space, fix this by removing the spaces. Signed-off-by: Colin Ian King --- drivers/scsi/dpt_i2o.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/dpt_i2o.c b/drivers/scsi/dpt_i2o.c index 70d1a18278af..f9560a183c12 100644 --- a/drivers/scsi/dpt_i2o.c +++ b/drivers/scsi/dpt_i2o.c @@ -877,8 +877,8 @@ static void adpt_i2o_sys_shutdown(void) adpt_hba *pHba, *pNext; struct adpt_i2o_post_wait_data *p1, *old; -printk(KERN_INFO"Shutting down Adaptec I2O controllers.\n"); -printk(KERN_INFO" This could take a few minutes if there are many devices attached\n"); + printk(KERN_INFO "Shutting down Adaptec I2O controllers.\n"); + printk(KERN_INFO " This could take a few minutes if there are many devices attached\n"); /* Delete all IOPs from the controller chain */ /* They should have already been released by the * scsi-core @@ -901,7 +901,7 @@ static void adpt_i2o_sys_shutdown(void) // spin_unlock_irqrestore(&adpt_post_wait_lock, flags); adpt_post_wait_queue = NULL; -printk(KERN_INFO "Adaptec I2O controllers down.\n"); + printk(KERN_INFO "Adaptec I2O controllers down.\n"); } static int adpt_install_hba(struct scsi_host_template* sht, struct pci_dev* pDev) @@ -3427,7 +3427,7 @@ static int adpt_i2o_issue_params(int cmd, adpt_hba* pHba, int tid, return -((res[1] >> 16) & 0xFF); /* -BlockStatus */ } -return 4 + ((res[1] & 0x) << 2); /* bytes used in resblk */ + return 4 + ((res[1] & 0x) << 2); /* bytes used in resblk */ } @@ -3500,8 +3500,8 @@ static int adpt_i2o_enable_hba(adpt_hba* pHba) static int adpt_i2o_systab_send(adpt_hba* pHba) { -u32 msg[12]; -int ret; + u32 msg[12]; + int ret; msg[0] = I2O_MESSAGE_SIZE(12) | SGL_OFFSET_6; msg[1] = I2O_CMD_SYS_TAB_SET<<24 | HOST_TID<<12 | ADAPTER_TID; -- 2.19.1
[PATCH] scsi: qlogicfas408: clean up a couple of indentation issues
From: Colin Ian King An if statement is indented correctly and an outb statement has a redundant empty comment and incorrect indentation. Fix these. Signed-off-by: Colin Ian King --- drivers/scsi/qlogicfas408.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/qlogicfas408.c b/drivers/scsi/qlogicfas408.c index 8b471a925b43..136681ad18a5 100644 --- a/drivers/scsi/qlogicfas408.c +++ b/drivers/scsi/qlogicfas408.c @@ -139,7 +139,7 @@ static int ql_pdma(struct qlogicfas408_priv *priv, int phase, char *request, int } else {/* out */ #if QL_TURBO_PDMA rtrc(4) - if (reqlen >= 128 && inb(qbase + 8) & 0x10) { /* empty */ + if (reqlen >= 128 && inb(qbase + 8) & 0x10) { /* empty */ outsl(qbase + 4, request, 32); reqlen -= 128; request += 128; @@ -240,7 +240,7 @@ static void ql_icmd(struct scsi_cmnd *cmd) outb(0x40 | qlcfg8 | priv->qinitid, qbase + 8); outb(qlcfg7, qbase + 7); outb(qlcfg6, qbase + 6); -/**/ outb(qlcfg5, qbase + 5); /* select timer */ + outb(qlcfg5, qbase + 5);/* select timer */ outb(qlcfg9 & 7, qbase + 9);/* prescaler */ /* outb(0x99, qbase + 5); */ outb(scmd_id(cmd), qbase + 4); -- 2.19.1
Re: [PATCH for-5.0] scsi: communicate max segment size to the DMA mapping code
On 01/16/2019 05:12 PM, Christoph Hellwig wrote: When a host driver sets a maximum segment size we should not only propagate that setting to the block layer, which can merge segments, but also to the DMA mapping layer which can merge segments as well. Fixes: 50c2e9107f ("scsi: introduce a max_segment_size host_template parameters") Signed-off-by: Christoph Hellwig --- drivers/ata/pata_macio.c | 9 - drivers/ata/sata_inic162x.c | 22 +- drivers/firewire/sbp2.c | 5 + drivers/scsi/aacraid/linit.c | 9 - drivers/scsi/scsi_lib.c | 4 ++-- 5 files changed, 20 insertions(+), 29 deletions(-) diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index 09b845e90114..a785ffd5af89 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1144,10 +1144,6 @@ static int sbp2_probe(struct fw_unit *unit, const struct ieee1394_device_id *id) if (device->is_local) return -ENODEV; - if (dma_get_max_seg_size(device->card->device) > SBP2_MAX_SEG_SIZE) - WARN_ON(dma_set_max_seg_size(device->card->device, -SBP2_MAX_SEG_SIZE)); - shost = scsi_host_alloc(&scsi_driver_template, sizeof(*tgt)); if (shost == NULL) return -ENOMEM; @@ -1610,6 +1606,7 @@ static struct scsi_host_template scsi_driver_template = { .eh_abort_handler = sbp2_scsi_abort, .this_id= -1, .sg_tablesize = SG_ALL, + .max_segment_size = SBP2_MAX_SEG_SIZE, .can_queue = 1, .sdev_attrs = sbp2_scsi_sysfs_attrs, }; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index b13cc9288ba0..6d65ac584eba 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1842,8 +1842,8 @@ void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q) blk_queue_segment_boundary(q, shost->dma_boundary); dma_set_seg_boundary(dev, shost->dma_boundary); - blk_queue_max_segment_size(q, - min(shost->max_segment_size, dma_get_max_seg_size(dev))); + blk_queue_max_segment_size(q, shost->max_segment_size); + dma_set_max_seg_size(dev, shost->max_segment_size); Zfcp can only have max_segment_size of one page (4kB). Officially announced through dev.dma_parms since v2.6.35 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=683229845f1780b10041ee7a1043fc8f10061455. With Martin's 5.0/scsi-fixes which includes your above patch, I now get 64kB instead of 4kB for the respective queue limit because __scsi_ini_queue overwrites the dma parameter: [root@host:/sys/bus/ccw/drivers/zfcp/0.0.3c40/host5/rport-5:0-4/target5:0:4/5:0:4:10/block/sdi/queue](0)# cat max_segment_size 65536 To my surprise, I don't get IO errors with zfcp. Maybe my IO pattern does not cause too large segments to be created. I would have expected the FCP channel complaining rightly so if we pass segments larger than one page. Maybe the additional dma_boundary of pagesize-1 helped the too large max_segment_size to not become effective? A quick attempt to adapt zfcp to your patch would be to set scsi_host_template.max_segment_size = ZFCP_QDIO_SBALE_LEN. Ideas? -- Mit freundlichen Gruessen / Kind regards Steffen Maier Linux on IBM Z Development https://www.ibm.com/privacy/us/en/ IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
[Bug 199435] HPSA + P420i resetting logical Direct-Access never complete
https://bugzilla.kernel.org/show_bug.cgi?id=199435 --- Comment #28 from Anthony Hausman (anthonyhaussm...@gmail.com) --- I have actually compiled the hpsa driver 3.4.20.141 into the kernel 4.19.13. I still have the same behavior, a heavy load (3000) and all disk of the controller unavailable. But this time, it's not the reset who trigger the bug, here the log that I have. First of all, one disk returns a lot of critical medium error: ``` [Wed Jan 23 15:55:34 2019] print_req_error: critical medium error, dev sdt, sector 13836632 [Wed Jan 23 15:55:34 2019] sd 3:1:0:19: [sdt] Unaligned partial completion (resid=52, sector_sz=512) [Wed Jan 23 15:55:35 2019] sd 3:1:0:19: [sdt] Unaligned partial completion (resid=48, sector_sz=512) [Wed Jan 23 15:55:35 2019] sd 3:1:0:19: [sdt] Unaligned partial completion (resid=32, sector_sz=512) [Wed Jan 23 15:55:35 2019] sd 3:1:0:19: [sdt] Unaligned partial completion (resid=52, sector_sz=512) [Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] Unaligned partial completion (resid=32, sector_sz=512) [Wed Jan 23 15:55:52 2019] scsi_io_completion_action: 5 callbacks suppressed [Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 Sense Key : Medium Error [current] [Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 Add. Sense: Unrecovered read error [Wed Jan 23 15:55:52 2019] sd 3:1:0:19: [sdt] tag#23 CDB: Read(16) 88 00 00 00 00 00 00 d3 21 58 00 00 00 08 00 00 [Wed Jan 23 15:55:52 2019] print_req_error: 5 callbacks suppressed [Wed Jan 23 15:55:52 2019] print_req_error: critical medium error, dev sdt, sector 13836632 ``` After this, hpsa show sone failed inquiry: ``` [Wed Jan 23 15:57:07 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR TAG:0x:0770 LUN:00c03901 CDB:12003100 [Wed Jan 23 15:57:07 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry failed, device will be skipped. [Wed Jan 23 15:57:08 2019] hpsa :08:00.0: removed scsi 3:0:50:0: Direct-Access ATA MB4000GCWDC PHYS DRV SSDSmartPathCap- En- Exp=0 qd=14 [Wed Jan 23 15:57:31 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR TAG:0x:15c0 LUN:00c03901 CDB:12003100 [Wed Jan 23 15:57:31 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry failed, device will be skipped. [Wed Jan 23 15:57:54 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR TAG:0x:0e70 LUN:00c03901 CDB:12003100 [Wed Jan 23 15:57:54 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry failed, device will be skipped. [Wed Jan 23 15:59:04 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR TAG:0x:2650 LUN:00c03901 CDB:12003100 [Wed Jan 23 15:59:04 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry failed, device will be skipped. [Wed Jan 23 15:59:28 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR TAG:0x:1400 LUN:00c03901 CDB:12003100 [Wed Jan 23 15:59:28 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry failed, device will be skipped. [Wed Jan 23 15:59:51 2019] hpsa :08:00.0: aborted: NULL_SDEV_PTR TAG:0x:1400 LUN:00c03901 CDB:12003100 [Wed Jan 23 15:59:51 2019] hpsa :08:00.0: hpsa_update_device_info: inquiry failed, device will be skipped. ``` And following this Call Trace: ``` Wed Jan 23 16:00:19 2019] INFO: task task:12406 blocked for more than 120 seconds. [Wed Jan 23 16:00:19 2019] Not tainted 4.19.13-dailymotion #1 [Wed Jan 23 16:00:19 2019] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [Wed Jan 23 16:00:19 2019] taskD0 12406 12384 0x [Wed Jan 23 16:00:19 2019] Call Trace: [Wed Jan 23 16:00:19 2019] ? __schedule+0x2b7/0x880 [Wed Jan 23 16:00:19 2019] ? bit_wait+0x50/0x50 [Wed Jan 23 16:00:19 2019] schedule+0x28/0x80 [Wed Jan 23 16:00:19 2019] io_schedule+0x12/0x40 [Wed Jan 23 16:00:19 2019] bit_wait_io+0xd/0x50 [Wed Jan 23 16:00:19 2019] __wait_on_bit+0x44/0x80 [Wed Jan 23 16:00:19 2019] out_of_line_wait_on_bit+0x91/0xb0 [Wed Jan 23 16:00:19 2019] ? init_wait_var_entry+0x40/0x40 [Wed Jan 23 16:00:19 2019] __ext4_get_inode_loc+0x1a4/0x3f0 [Wed Jan 23 16:00:19 2019] ext4_iget+0x8f/0xbb0 [Wed Jan 23 16:00:19 2019] ? d_alloc_parallel+0x9d/0x4a0 [Wed Jan 23 16:00:19 2019] ext4_lookup+0xda/0x200 [Wed Jan 23 16:00:19 2019] __lookup_slow+0x97/0x150 [Wed Jan 23 16:00:19 2019] lookup_slow+0x35/0x50 [Wed Jan 23 16:00:19 2019] walk_component+0x1c4/0x340 [Wed Jan 23 16:00:19 2019] link_path_walk.part.33+0x2a6/0x510 [Wed Jan 23 16:00:19 2019] ? path_init+0x190/0x310 [Wed Jan 23 16:00:19 2019] path_openat+0xdd/0x1540 [Wed Jan 23 16:00:19 2019] ? get_futex_key+0x2ed/0x3d0 [Wed Jan 23 16:00:19 2019] do_filp_open+0x9b/0x110 [Wed Jan 23 16:00:19 2019] ? __check_object_size+
Re: [PATCH 0/7] Fix handling of bidi commands
On Tue, 2019-01-22 at 23:49 -0500, Douglas Gilbert wrote: > On 2019-01-22 7:56 p.m., Bart Van Assche wrote: > > On Tue, 2019-01-22 at 18:30 -0500, Douglas Gilbert wrote: > > > This patchset needs something like the following if UAS (USB Attached > > > SCSI) is configured in your kernel. > > > > > > Beware of tabs/spaces/line_wraps as this is a cut and paste: > > > > > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > > > index 36742e8e7edc..24f3f95917a5 100644 > > > --- a/drivers/usb/storage/uas.c > > > +++ b/drivers/usb/storage/uas.c > > > @@ -401,9 +401,9 @@ static void uas_data_cmplt(struct urb *urb) > > > if (status != -ENOENT && status != -ECONNRESET && > > > status != > > > -ESHUTDOWN) > > > uas_log_cmd_state(cmnd, "data cmplt err", > > > status); > > > /* error: no data transfered */ > > > - sdb->resid = sdb->length; > > > + scsi_in_set_resid(cmnd, sdb->length); > > > } else { > > > - sdb->resid = sdb->length - urb->actual_length; > > > + scsi_in_set_resid(cmnd, sdb->length - urb->actual_length); > > > } > > > uas_try_complete(cmnd, __func__); > > >out: > > > > Thanks Doug! I will fold a slightly modified version of this patch in. BTW, > > does this mean that you have been able to test this patch series? > > Yes, but I didn't get far. With my sg v4 driver, scsi_debug and my sg_tst_bidi > utility and this patchset after a short while I get a NULL pointer dereference > in scsi_mq_prep_fn() inside the bidi conditional, probably the memset(). > > RIP ---> 0x2ce7 > > > if (blk_bidi_rq(req)) { > 2c9a: 49 8b 85 20 01 00 00mov0x120(%r13),%rax > 2ca1: 48 85 c0test %rax,%rax > 2ca4: 74 60 je 2d06 > memset(&scsi_in_cmd(cmd)->sdb, 0, > 2ca6: 48 c7 80 28 02 00 00movq $0x0,0x228(%rax) > 2cad: 00 00 00 00 > 2cb1: 48 c7 80 30 02 00 00movq $0x0,0x230(%rax) > 2cb8: 00 00 00 00 > 2cbc: 48 c7 80 38 02 00 00movq $0x0,0x238(%rax) > 2cc3: 00 00 00 00 > 2cc7: 49 8b 85 60 02 00 00mov0x260(%r13),%rax > 2cce: 48 8b 90 20 01 00 00mov0x120(%rax),%rdx > 2cd5: 48 8d 82 28 01 00 00lea0x128(%rdx),%rax > 2cdc: 48 85 d2test %rdx,%rdx > 2cdf: 49 0f 44 c6 cmove %r14,%rax > cmd->device->host->hostt->cmd_size; > 2ce3: 48 8b 50 38 mov0x38(%rax),%rdx > 2ce7: 48 8b 12mov(%rdx),%rdx <== > 2cea: 48 8b 92 98 00 00 00mov0x98(%rdx),%rdx > 2cf1: 8b 92 30 01 00 00 mov0x130(%rdx),%edx > cmd->sdb.table.sgl = (void *)cmd + sizeof(struct scsi_cmnd) + > 2cf7: 48 8d 94 10 b0 01 00lea0x1b0(%rax,%rdx,1),%rdx > 2cfe: 00 > 2cff: 48 89 90 00 01 00 00mov%rdx,0x100(%rax) > blk_mq_start_request(req) > . > > It might not be your patchset, but the location does look suspicious. Hi Doug, I think this means that the scsi_in_cmd(cmd)->device pointer is NULL. I will address this in the next version of this patch series. Thanks, Bart.
Re: [PATCH for-5.0] scsi: communicate max segment size to the DMA mapping code
On Wed, Jan 23, 2019 at 02:45:49PM +0100, Steffen Maier wrote: > Zfcp can only have max_segment_size of one page (4kB). Officially announced > through dev.dma_parms since v2.6.35 > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/s390/scsi?id=683229845f1780b10041ee7a1043fc8f10061455. > > With Martin's 5.0/scsi-fixes which includes your above patch, I now get > 64kB instead of 4kB for the respective queue limit because __scsi_ini_queue > overwrites the dma parameter: Well, had the driver usd the proper dma_set_max_seg_size API instead of handcoding it I would have converted it.. > To my surprise, I don't get IO errors with zfcp. Maybe my IO pattern does > not cause too large segments to be created. I would have expected the FCP > channel complaining rightly so if we pass segments larger than one page. > Maybe the additional dma_boundary of pagesize-1 helped the too large > max_segment_size to not become effective? The driver already sets the dma_boundary field, which prevents from merging I/Os that span multiple pages, and thus limits each segment to a page or less. > A quick attempt to adapt zfcp to your patch would be to set > scsi_host_template.max_segment_size = ZFCP_QDIO_SBALE_LEN. I think we can just drop the dma_parms max_segment_size without replacement due to the dma_boundary. You do however still need to set up the dma_parms structure itself, as that is also used to communicate the boundary to the IOMMU. If would however be great if you moved that setup into the bus code instead of the driver, like we do for all other major hardware busses.
[PATCH] libsas: Remove scsi_to_u32()
Since the function scsi_to_u32() is identical to get_unaligned_be32(), change all scsi_to_u32() calls into get_unaligned_be32() calls. Cc: Jian Luo Cc: John Garry Signed-off-by: Bart Van Assche --- drivers/scsi/libsas/sas_expander.c | 9 + include/scsi/scsi.h| 6 -- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index 83e3715d30e4..b13b245791b0 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "sas_internal.h" @@ -696,10 +697,10 @@ int sas_smp_get_phy_events(struct sas_phy *phy) if (res) goto out; - phy->invalid_dword_count = scsi_to_u32(&resp[12]); - phy->running_disparity_error_count = scsi_to_u32(&resp[16]); - phy->loss_of_dword_sync_count = scsi_to_u32(&resp[20]); - phy->phy_reset_problem_count = scsi_to_u32(&resp[24]); + phy->invalid_dword_count = get_unaligned_be32(&resp[12]); + phy->running_disparity_error_count = get_unaligned_be32(&resp[16]); + phy->loss_of_dword_sync_count = get_unaligned_be32(&resp[20]); + phy->phy_reset_problem_count = get_unaligned_be32(&resp[24]); out: kfree(req); diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h index eb7853c1a23b..5339baadc082 100644 --- a/include/scsi/scsi.h +++ b/include/scsi/scsi.h @@ -274,10 +274,4 @@ static inline int scsi_is_wlun(u64 lun) /* Used to obtain the PCI location of a device */ #define SCSI_IOCTL_GET_PCI 0x5387 -/* Pull a u32 out of a SCSI message (using BE SCSI conventions) */ -static inline __u32 scsi_to_u32(__u8 *ptr) -{ - return (ptr[0]<<24) + (ptr[1]<<16) + (ptr[2]<<8) + ptr[3]; -} - #endif /* _SCSI_SCSI_H */ -- 2.20.1.321.g9e740568ce-goog
[PATCH v2 4/7] Introduce scsi_out_cmd()
This patch does not change any functionality. Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Signed-off-by: Bart Van Assche --- include/scsi/scsi_cmnd.h | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 78183e851a0d..213404163993 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -230,9 +230,14 @@ static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd) return &scsi_in_cmd(cmd)->sdb; } +static inline struct scsi_cmnd *scsi_out_cmd(struct scsi_cmnd *cmd) +{ + return cmd; +} + static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd) { - return &cmd->sdb; + return &scsi_out_cmd(cmd)->sdb; } static inline int scsi_sg_copy_from_buffer(struct scsi_cmnd *cmd, -- 2.20.1.321.g9e740568ce-goog
[PATCH v2 2/7] Change scsi_cmnd.prot_sdb from a pointer into a regular member
This patch slightly increases the size of struct scsi_cmnd if data protection is disabled, decreases the size of the data appended at the end of struct scsi_cmnd if data protection is enabled and simplifies the SCSI core. Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Signed-off-by: Bart Van Assche --- drivers/scsi/scsi_lib.c | 33 - include/scsi/scsi_cmnd.h | 8 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 94842b104bcc..6bfbe50ef38e 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -566,7 +566,7 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd) sg_free_table_chained(&sdb->table, true); } if (scsi_prot_sg_count(cmd)) - sg_free_table_chained(&cmd->prot_sdb->table, true); + sg_free_table_chained(&cmd->prot_sdb.table, true); } static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd) @@ -1065,10 +1065,10 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd) } if (blk_integrity_rq(rq)) { - struct scsi_data_buffer *prot_sdb = cmd->prot_sdb; + struct scsi_data_buffer *prot_sdb = &cmd->prot_sdb; int ivecs, count; - if (WARN_ON_ONCE(!prot_sdb)) { + if (WARN_ON_ONCE(!scsi_host_get_prot(cmd->device->host))) { /* * This can happen if someone (e.g. multipath) * queues a command to a device on an adapter @@ -1091,8 +1091,7 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd) BUG_ON(count > ivecs); BUG_ON(count > queue_max_integrity_segments(rq->q)); - cmd->prot_sdb = prot_sdb; - cmd->prot_sdb->table.nents = count; + prot_sdb->table.nents = count; } return BLK_STS_OK; @@ -1156,7 +1155,6 @@ void scsi_del_cmd_from_list(struct scsi_cmnd *cmd) void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) { void *buf = cmd->sense_buffer; - void *prot = cmd->prot_sdb; struct request *rq = blk_mq_rq_from_pdu(cmd); unsigned int flags = cmd->flags & SCMD_PRESERVED_FLAGS; unsigned long jiffies_at_alloc; @@ -1175,7 +1173,6 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd) cmd->device = dev; cmd->sense_buffer = buf; - cmd->prot_sdb = prot; cmd->flags = flags; INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler); cmd->jiffies_at_alloc = jiffies_at_alloc; @@ -1617,12 +1614,13 @@ static blk_status_t scsi_mq_prep_fn(struct request *req) sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size; cmd->sdb.table.sgl = sg; - if (scsi_host_get_prot(shost)) { - memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer)); - - cmd->prot_sdb->table.sgl = - (struct scatterlist *)(cmd->prot_sdb + 1); - } + /* +* Always initialize cmd->prot_sdb.nents such that +* scsi_prot_sg_count() does not have to call scsi_host_get_prot(). +*/ + memset(&cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer)); + if (scsi_host_get_prot(shost)) + cmd->prot_sdb.table.sgl = (void *)&sg + scsi_mq_sgl_size(shost); if (blk_bidi_rq(req)) { struct request *next_rq = req->next_rq; @@ -1781,7 +1779,6 @@ static int scsi_mq_init_request(struct blk_mq_tag_set *set, struct request *rq, struct Scsi_Host *shost = set->driver_data; const bool unchecked_isa_dma = shost->unchecked_isa_dma; struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq); - struct scatterlist *sg; if (unchecked_isa_dma) cmd->flags |= SCMD_UNCHECKED_ISA_DMA; @@ -1791,12 +1788,6 @@ static int scsi_mq_init_request(struct blk_mq_tag_set *set, struct request *rq, return -ENOMEM; cmd->req.sense = cmd->sense_buffer; - if (scsi_host_get_prot(shost)) { - sg = (void *)cmd + sizeof(struct scsi_cmnd) + - shost->hostt->cmd_size; - cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost); - } - return 0; } @@ -1893,7 +1884,7 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) sgl_size = scsi_mq_sgl_size(shost); cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size; if (scsi_host_get_prot(shost)) - cmd_size += sizeof(struct scsi_data_buffer) + sgl_size; + cmd_size += sgl_size; memset(&shost->tag_set, 0, sizeof(shost->tag_set)); shost->tag_set.ops = &scsi_mq_ops; diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index d85e6befa26b..0406c0fbee3e 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd
[PATCH v2 3/7] Fix bidi handling
Some code in the SCSI core interprets blk_mq_rq_to_pdu(cmd->request->next_rq) as a struct scsi_data_buffer, e.g. scsi_mq_prep_fn(). Other code in the SCSI core interprets the same data structure as a struct scsi_request, e.g. scsi_io_completion(). Avoid this confusion by using the SCSI data buffer associated with "next_rq" for bidi requests. This patch avoids that submitting a bidi command triggers a NULL pointer dereference. Reported-by: Douglas Gilbert Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Fixes: d285203cf647 ("scsi: add support for a blk-mq based I/O path.") # v3.17 Signed-off-by: Bart Van Assche --- drivers/scsi/scsi_lib.c | 35 +++ include/scsi/scsi_cmnd.h | 13 + 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 6bfbe50ef38e..bcbf266e4172 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -556,15 +556,10 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd) static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd) { - struct scsi_data_buffer *sdb; - if (cmd->sdb.table.nents) sg_free_table_chained(&cmd->sdb.table, true); - if (cmd->request->next_rq) { - sdb = cmd->request->next_rq->special; - if (sdb) - sg_free_table_chained(&sdb->table, true); - } + if (scsi_bidi_cmnd(cmd)) + sg_free_table_chained(&scsi_in(cmd)->table, true); if (scsi_prot_sg_count(cmd)) sg_free_table_chained(&cmd->prot_sdb.table, true); } @@ -1059,7 +1054,7 @@ blk_status_t scsi_init_io(struct scsi_cmnd *cmd) return ret; if (blk_bidi_rq(rq)) { - ret = scsi_init_sgtable(rq->next_rq, rq->next_rq->special); + ret = scsi_init_sgtable(rq->next_rq, scsi_in(cmd)); if (ret) goto out_free_sgtables; } @@ -1595,12 +1590,17 @@ static unsigned int scsi_mq_sgl_size(struct Scsi_Host *shost) sizeof(struct scatterlist); } +static void scsi_init_sdb(struct Scsi_Host *shost, struct scsi_cmnd *cmd) +{ + cmd->sdb.table.sgl = (void *)cmd + sizeof(struct scsi_cmnd) + + shost->hostt->cmd_size; +} + static blk_status_t scsi_mq_prep_fn(struct request *req) { struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req); struct scsi_device *sdev = req->q->queuedata; struct Scsi_Host *shost = sdev->host; - struct scatterlist *sg; scsi_init_command(sdev, cmd); @@ -1611,8 +1611,7 @@ static blk_status_t scsi_mq_prep_fn(struct request *req) cmd->tag = req->tag; cmd->prot_op = SCSI_PROT_NORMAL; - sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size; - cmd->sdb.table.sgl = sg; + scsi_init_sdb(shost, cmd); /* * Always initialize cmd->prot_sdb.nents such that @@ -1620,17 +1619,13 @@ static blk_status_t scsi_mq_prep_fn(struct request *req) */ memset(&cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer)); if (scsi_host_get_prot(shost)) - cmd->prot_sdb.table.sgl = (void *)&sg + scsi_mq_sgl_size(shost); + cmd->prot_sdb.table.sgl = (void *)&cmd->sdb + + scsi_mq_sgl_size(shost); if (blk_bidi_rq(req)) { - struct request *next_rq = req->next_rq; - struct scsi_data_buffer *bidi_sdb = blk_mq_rq_to_pdu(next_rq); - - memset(bidi_sdb, 0, sizeof(struct scsi_data_buffer)); - bidi_sdb->table.sgl = - (struct scatterlist *)(bidi_sdb + 1); - - next_rq->special = bidi_sdb; + memset(&scsi_in_cmd(cmd)->sdb, 0, + sizeof(scsi_in_cmd(cmd)->sdb)); + scsi_init_sdb(shost, scsi_in_cmd(cmd)); } blk_mq_start_request(req); diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 0406c0fbee3e..78183e851a0d 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -215,14 +215,19 @@ static inline int scsi_get_resid(struct scsi_cmnd *cmd) static inline int scsi_bidi_cmnd(struct scsi_cmnd *cmd) { - return blk_bidi_rq(cmd->request) && - (cmd->request->next_rq->special != NULL); + return blk_bidi_rq(cmd->request); +} + +static inline struct scsi_cmnd *scsi_in_cmd(struct scsi_cmnd *cmd) +{ + if (likely(!scsi_bidi_cmnd(cmd))) + return cmd; + return blk_mq_rq_to_pdu(cmd->request->next_rq); } static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd) { - return scsi_bidi_cmnd(cmd) ? - cmd->request->next_rq->special : &cmd->sdb; + return &scsi_in_cmd(cmd)->sdb; } static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd) -- 2.20.1.321.g9e740568ce-goog
[PATCH v2 0/7] Fix handling of bidi commands
Hi Martin, Recently Doug Gilbert reported that handling of bidi commands is broken in the scsi-mq code. This patch series fixes that bug and also simplifies bidi command handling. Please consider these patches for kernel v5.1. Thanks, Bart. Changes compared to v1: - Fixed a NULL pointer dereference in scsi_init_sdb(). Bart Van Assche (7): Introduce the bidi_supported flag in the host template structure Change scsi_cmnd.prot_sdb from a pointer into a regular member Fix bidi handling Introduce scsi_out_cmd() Move several function definitions in Introduce scsi_in_[sg]et_resid() and scsi_out_[sg]et_resid() Move the resid member from struct scsi_data_buffer into struct scsi_cmnd drivers/scsi/iscsi_tcp.c | 8 +--- drivers/scsi/libiscsi.c| 12 ++--- drivers/scsi/scsi_debug.c | 20 +++-- drivers/scsi/scsi_lib.c| 70 -- drivers/scsi/sd.c | 4 +- drivers/scsi/virtio_scsi.c | 4 +- drivers/target/loopback/tcm_loop.c | 8 +--- drivers/usb/storage/uas.c | 8 ++-- include/scsi/scsi_cmnd.h | 67 include/scsi/scsi_host.h | 2 + 10 files changed, 101 insertions(+), 102 deletions(-) -- 2.20.1.321.g9e740568ce-goog
[PATCH v2 1/7] Introduce the bidi_supported flag in the host template structure
This patch does not change any functionality but makes the drivers that support bidirectional commands more compact. Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Cc: Lee Duncan Cc: Chris Leech Signed-off-by: Bart Van Assche --- drivers/scsi/iscsi_tcp.c | 8 +--- drivers/scsi/scsi_debug.c | 11 +-- drivers/scsi/scsi_lib.c| 2 ++ drivers/target/loopback/tcm_loop.c | 8 +--- include/scsi/scsi_host.h | 2 ++ 5 files changed, 7 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c index cae6368ebb98..8c09e9e45a62 100644 --- a/drivers/scsi/iscsi_tcp.c +++ b/drivers/scsi/iscsi_tcp.c @@ -952,12 +952,6 @@ static umode_t iscsi_sw_tcp_attr_is_visible(int param_type, int param) return 0; } -static int iscsi_sw_tcp_slave_alloc(struct scsi_device *sdev) -{ - blk_queue_flag_set(QUEUE_FLAG_BIDI, sdev->request_queue); - return 0; -} - static int iscsi_sw_tcp_slave_configure(struct scsi_device *sdev) { struct iscsi_sw_tcp_host *tcp_sw_host = iscsi_host_priv(sdev->host); @@ -985,7 +979,7 @@ static struct scsi_host_template iscsi_sw_tcp_sht = { .eh_device_reset_handler= iscsi_eh_device_reset, .eh_target_reset_handler = iscsi_eh_recover_target, .dma_boundary = PAGE_SIZE - 1, - .slave_alloc= iscsi_sw_tcp_slave_alloc, + .bidi_supported = true, .slave_configure= iscsi_sw_tcp_slave_configure, .target_alloc = iscsi_target_alloc, .proc_name = "iscsi_tcp", diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 661512bec3ac..e253c0129b40 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -3948,15 +3948,6 @@ static struct sdebug_dev_info *find_build_dev_info(struct scsi_device *sdev) return open_devip; } -static int scsi_debug_slave_alloc(struct scsi_device *sdp) -{ - if (sdebug_verbose) - pr_info("slave_alloc <%u %u %u %llu>\n", - sdp->host->host_no, sdp->channel, sdp->id, sdp->lun); - blk_queue_flag_set(QUEUE_FLAG_BIDI, sdp->request_queue); - return 0; -} - static int scsi_debug_slave_configure(struct scsi_device *sdp) { struct sdebug_dev_info *devip = @@ -5834,7 +5825,7 @@ static struct scsi_host_template sdebug_driver_template = { .proc_name =sdebug_proc_name, .name = "SCSI DEBUG", .info = scsi_debug_info, - .slave_alloc = scsi_debug_slave_alloc, + .bidi_supported = true, .slave_configure = scsi_debug_slave_configure, .slave_destroy =scsi_debug_slave_destroy, .ioctl =scsi_debug_ioctl, diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 00cd365fb7d2..94842b104bcc 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1881,6 +1881,8 @@ struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev) sdev->request_queue->queuedata = sdev; __scsi_init_queue(sdev->host, sdev->request_queue); blk_queue_flag_set(QUEUE_FLAG_SCSI_PASSTHROUGH, sdev->request_queue); + if (sdev->host->hostt->bidi_supported) + blk_queue_flag_set(QUEUE_FLAG_BIDI, sdev->request_queue); return sdev->request_queue; } diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c index 7bd7c0c0db6f..e54a5c57a8bb 100644 --- a/drivers/target/loopback/tcm_loop.c +++ b/drivers/target/loopback/tcm_loop.c @@ -304,12 +304,6 @@ static int tcm_loop_target_reset(struct scsi_cmnd *sc) return FAILED; } -static int tcm_loop_slave_alloc(struct scsi_device *sd) -{ - blk_queue_flag_set(QUEUE_FLAG_BIDI, sd->request_queue); - return 0; -} - static struct scsi_host_template tcm_loop_driver_template = { .show_info = tcm_loop_show_info, .proc_name = "tcm_loopback", @@ -325,7 +319,7 @@ static struct scsi_host_template tcm_loop_driver_template = { .cmd_per_lun= 1024, .max_sectors= 0x, .dma_boundary = PAGE_SIZE - 1, - .slave_alloc= tcm_loop_slave_alloc, + .bidi_supported = true, .module = THIS_MODULE, .track_queue_depth = 1, }; diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 6ca954e9f752..384b50992a56 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -430,6 +430,8 @@ struct scsi_host_template { /* True if the low-level driver supports blk-mq only */ unsigned force_blk_mq:1; + unsigned bidi_supported:1; + /* * Countdown for host blocking with no commands outstanding. */ -- 2.20.1.321.g9e740568ce-goog
[PATCH v2 7/7] Move the resid member from struct scsi_data_buffer into struct scsi_cmnd
This patch does not change any functionality but reduces the size of struct scsi_cmnd. Cc: Oliver Neukum Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Cc: linux-...@vger.kernel.org Signed-off-by: Bart Van Assche --- drivers/scsi/scsi_lib.c | 2 -- drivers/usb/storage/uas.c | 8 +--- include/scsi/scsi_cmnd.h | 9 - 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index bcbf266e4172..4feba3b5aff1 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -945,14 +945,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) * scsi_result_to_blk_status may have reset the host_byte */ scsi_req(req)->result = cmd->result; - scsi_req(req)->resid_len = scsi_get_resid(cmd); if (unlikely(scsi_bidi_cmnd(cmd))) { /* * Bidi commands Must be complete as a whole, * both sides at once. */ - scsi_req(req->next_rq)->resid_len = scsi_in(cmd)->resid; if (scsi_end_request(req, BLK_STS_OK, blk_rq_bytes(req), blk_rq_bytes(req->next_rq))) WARN_ONCE(true, diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 36742e8e7edc..ea40fd78e6ff 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -365,7 +365,7 @@ static void uas_stat_cmplt(struct urb *urb) static void uas_data_cmplt(struct urb *urb) { - struct scsi_cmnd *cmnd = urb->context; + struct scsi_cmnd *cmnd = urb->context, *cmpl_cmd = NULL; struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; struct scsi_data_buffer *sdb = NULL; @@ -375,10 +375,12 @@ static void uas_data_cmplt(struct urb *urb) spin_lock_irqsave(&devinfo->lock, flags); if (cmdinfo->data_in_urb == urb) { + cmpl_cmd = scsi_in_cmd(cmnd); sdb = scsi_in(cmnd); cmdinfo->state &= ~DATA_IN_URB_INFLIGHT; cmdinfo->data_in_urb = NULL; } else if (cmdinfo->data_out_urb == urb) { + cmpl_cmd = scsi_out_cmd(cmnd); sdb = scsi_out(cmnd); cmdinfo->state &= ~DATA_OUT_URB_INFLIGHT; cmdinfo->data_out_urb = NULL; @@ -401,9 +403,9 @@ static void uas_data_cmplt(struct urb *urb) if (status != -ENOENT && status != -ECONNRESET && status != -ESHUTDOWN) uas_log_cmd_state(cmnd, "data cmplt err", status); /* error: no data transfered */ - sdb->resid = sdb->length; + cmpl_cmd->req.resid_len = sdb->length; } else { - sdb->resid = sdb->length - urb->actual_length; + cmpl_cmd->req.resid_len = sdb->length - urb->actual_length; } uas_try_complete(cmnd, __func__); out: diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 8f3ed55a5ee5..9035c760cae0 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -35,7 +35,6 @@ struct scsi_driver; struct scsi_data_buffer { struct sg_table table; unsigned length; - int resid; }; /* embedded in scsi_cmnd */ @@ -229,22 +228,22 @@ static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd) static inline void scsi_in_set_resid(struct scsi_cmnd *cmd, int resid) { - scsi_in_cmd(cmd)->sdb.resid = resid; + scsi_in_cmd(cmd)->req.resid_len = resid; } static inline int scsi_in_get_resid(struct scsi_cmnd *cmd) { - return scsi_in_cmd(cmd)->sdb.resid; + return scsi_in_cmd(cmd)->req.resid_len; } static inline void scsi_out_set_resid(struct scsi_cmnd *cmd, int resid) { - cmd->sdb.resid = resid; + cmd->req.resid_len = resid; } static inline int scsi_out_get_resid(struct scsi_cmnd *cmd) { - return cmd->sdb.resid; + return cmd->req.resid_len; } static inline void scsi_set_resid(struct scsi_cmnd *cmd, int resid) -- 2.20.1.321.g9e740568ce-goog
[PATCH v2 6/7] Introduce scsi_in_[sg]et_resid() and scsi_out_[sg]et_resid()
This patch does not change any functionality. Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Cc: Lee Duncan Cc: Chris Leech Cc: Paolo Bonzini Signed-off-by: Bart Van Assche --- drivers/scsi/libiscsi.c| 12 ++-- drivers/scsi/scsi_debug.c | 9 + drivers/scsi/sd.c | 4 ++-- drivers/scsi/virtio_scsi.c | 4 ++-- include/scsi/scsi_cmnd.h | 24 ++-- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index b8d325ce8754..fe5b3371553c 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -650,8 +650,8 @@ static void fail_scsi_task(struct iscsi_task *task, int err) if (!scsi_bidi_cmnd(sc)) scsi_set_resid(sc, scsi_bufflen(sc)); else { - scsi_out(sc)->resid = scsi_out(sc)->length; - scsi_in(sc)->resid = scsi_in(sc)->length; + scsi_out_set_resid(sc, scsi_out(sc)->length); + scsi_in_set_resid(sc, scsi_in(sc)->length); } /* regular RX path uses back_lock */ @@ -912,7 +912,7 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr, if (scsi_bidi_cmnd(sc) && res_count > 0 && (rhdr->flags & ISCSI_FLAG_CMD_BIDI_OVERFLOW || res_count <= scsi_in(sc)->length)) - scsi_in(sc)->resid = res_count; + scsi_in_set_resid(sc, res_count); else sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status; } @@ -962,7 +962,7 @@ iscsi_data_in_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr, if (res_count > 0 && (rhdr->flags & ISCSI_FLAG_CMD_OVERFLOW || res_count <= scsi_in(sc)->length)) - scsi_in(sc)->resid = res_count; + scsi_in_set_resid(sc, res_count); else sc->result = (DID_BAD_TARGET << 16) | rhdr->cmd_status; } @@ -1807,8 +1807,8 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc) if (!scsi_bidi_cmnd(sc)) scsi_set_resid(sc, scsi_bufflen(sc)); else { - scsi_out(sc)->resid = scsi_out(sc)->length; - scsi_in(sc)->resid = scsi_in(sc)->length; + scsi_out_set_resid(sc, scsi_out(sc)->length); + scsi_in_set_resid(sc, scsi_in(sc)->length); } sc->scsi_done(sc); return 0; diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index e253c0129b40..822aed12ca31 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -1019,7 +1019,7 @@ static int fill_from_dev_buffer(struct scsi_cmnd *scp, unsigned char *arr, act_len = sg_copy_from_buffer(sdb->table.sgl, sdb->table.nents, arr, arr_len); - sdb->resid = scsi_bufflen(scp) - act_len; + scsi_in_set_resid(scp, scsi_bufflen(scp) - act_len); return 0; } @@ -1044,9 +1044,10 @@ static int p_fill_from_dev_buffer(struct scsi_cmnd *scp, const void *arr, act_len = sg_pcopy_from_buffer(sdb->table.sgl, sdb->table.nents, arr, arr_len, skip); pr_debug("%s: off_dst=%u, scsi_bufflen=%u, act_len=%u, resid=%d\n", -__func__, off_dst, scsi_bufflen(scp), act_len, sdb->resid); +__func__, off_dst, scsi_bufflen(scp), act_len, +scsi_in_get_resid(scp)); n = (int)scsi_bufflen(scp) - ((int)off_dst + act_len); - sdb->resid = min(sdb->resid, n); + scsi_in_set_resid(scp, min(scsi_in_get_resid(scp), n)); return 0; } @@ -2774,7 +2775,7 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) if (unlikely(ret == -1)) return DID_ERROR << 16; - scsi_in(scp)->resid = scsi_bufflen(scp) - ret; + scsi_in_set_resid(scp, scsi_bufflen(scp) - ret); if (unlikely(sqcp)) { if (sqcp->inj_recovered) { diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index b0eb83526c54..2219ba81f0b3 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -844,7 +844,7 @@ static blk_status_t sd_setup_unmap_cmnd(struct scsi_cmnd *cmd) cmd->allowed = SD_MAX_RETRIES; cmd->transfersize = data_len; rq->timeout = SD_TIMEOUT; - scsi_req(rq)->resid_len = data_len; + scsi_set_resid(cmd, data_len); return scsi_init_io(cmd); } @@ -908,7 +908,7 @@ static blk_status_t sd_setup_write_same10_cmnd(struct scsi_cmnd *cmd, cmd->allowed = SD_MAX_RETRIES; cmd->transfersize = data_len; rq->timeout = unmap ? SD_TIMEOUT : SD_WRITE_SAME_TIMEOUT; - scsi_req(rq)->resid_len = data_len; + scsi_set_resid(cmd, data_len); return scsi_init_io(cmd); } diff -
[PATCH v2 5/7] Move several function definitions in
Since the next patch will call scsi_out_cmd() from scsi_[gs]et_resid(), reorder the function definitions in . Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Signed-off-by: Bart Van Assche --- include/scsi/scsi_cmnd.h | 50 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h index 213404163993..673980ed7db6 100644 --- a/include/scsi/scsi_cmnd.h +++ b/include/scsi/scsi_cmnd.h @@ -185,61 +185,61 @@ static inline int scsi_dma_map(struct scsi_cmnd *cmd) { return -ENOSYS; } static inline void scsi_dma_unmap(struct scsi_cmnd *cmd) { } #endif /* !CONFIG_SCSI_DMA */ -static inline unsigned scsi_sg_count(struct scsi_cmnd *cmd) +static inline int scsi_bidi_cmnd(struct scsi_cmnd *cmd) { - return cmd->sdb.table.nents; + return blk_bidi_rq(cmd->request); } -static inline struct scatterlist *scsi_sglist(struct scsi_cmnd *cmd) +static inline struct scsi_cmnd *scsi_in_cmd(struct scsi_cmnd *cmd) { - return cmd->sdb.table.sgl; + if (likely(!scsi_bidi_cmnd(cmd))) + return cmd; + return blk_mq_rq_to_pdu(cmd->request->next_rq); } -static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd) +static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd) { - return cmd->sdb.length; + return &scsi_in_cmd(cmd)->sdb; } -static inline void scsi_set_resid(struct scsi_cmnd *cmd, int resid) +static inline struct scsi_cmnd *scsi_out_cmd(struct scsi_cmnd *cmd) { - cmd->sdb.resid = resid; + return cmd; } -static inline int scsi_get_resid(struct scsi_cmnd *cmd) +static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd) { - return cmd->sdb.resid; + return &scsi_out_cmd(cmd)->sdb; } -#define scsi_for_each_sg(cmd, sg, nseg, __i) \ - for_each_sg(scsi_sglist(cmd), sg, nseg, __i) - -static inline int scsi_bidi_cmnd(struct scsi_cmnd *cmd) +static inline unsigned scsi_sg_count(struct scsi_cmnd *cmd) { - return blk_bidi_rq(cmd->request); + return cmd->sdb.table.nents; } -static inline struct scsi_cmnd *scsi_in_cmd(struct scsi_cmnd *cmd) +static inline struct scatterlist *scsi_sglist(struct scsi_cmnd *cmd) { - if (likely(!scsi_bidi_cmnd(cmd))) - return cmd; - return blk_mq_rq_to_pdu(cmd->request->next_rq); + return cmd->sdb.table.sgl; } -static inline struct scsi_data_buffer *scsi_in(struct scsi_cmnd *cmd) +static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd) { - return &scsi_in_cmd(cmd)->sdb; + return cmd->sdb.length; } -static inline struct scsi_cmnd *scsi_out_cmd(struct scsi_cmnd *cmd) +static inline void scsi_set_resid(struct scsi_cmnd *cmd, int resid) { - return cmd; + cmd->sdb.resid = resid; } -static inline struct scsi_data_buffer *scsi_out(struct scsi_cmnd *cmd) +static inline int scsi_get_resid(struct scsi_cmnd *cmd) { - return &scsi_out_cmd(cmd)->sdb; + return cmd->sdb.resid; } +#define scsi_for_each_sg(cmd, sg, nseg, __i) \ + for_each_sg(scsi_sglist(cmd), sg, nseg, __i) + static inline int scsi_sg_copy_from_buffer(struct scsi_cmnd *cmd, void *buf, int buflen) { -- 2.20.1.321.g9e740568ce-goog
[PATCH] sd: Protect against submitting READ(6) or WRITE(6) with 256 logical blocks
Since the READ(6) and WRITE(6) commands interpret a zero in the transfer length field in the CDB as 256 logical blocks, avoid submitting such commands. Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Reported-by: Douglas Gilbert Signed-off-by: Bart Van Assche --- drivers/scsi/sd.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 4e69f182a1e5..b0eb83526c54 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1129,6 +1129,10 @@ static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd *cmd, bool write, sector_t lba, unsigned int nr_blocks, unsigned char flags) { + /* Avoid that 0 blocks gets translated into 256 blocks. */ + if (WARN_ON_ONCE(nr_blocks == 0)) + return BLK_STS_IOERR; + if (unlikely(flags & 0x8)) { /* * This happens only if this drive failed 10byte rw -- 2.20.1.321.g9e740568ce-goog
Re: [PATCH] sd: Protect against submitting READ(6) or WRITE(6) with 256 logical blocks
On 2019-01-23 2:12 p.m., Bart Van Assche wrote: Since the READ(6) and WRITE(6) commands interpret a zero in the transfer length field in the CDB as 256 logical blocks, avoid submitting such commands. Cc: Douglas Gilbert Cc: Hannes Reinecke Cc: Christoph Hellwig Reported-by: Douglas Gilbert Signed-off-by: Bart Van Assche Reviewed-by: Douglas Gilbert --- drivers/scsi/sd.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 4e69f182a1e5..b0eb83526c54 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -1129,6 +1129,10 @@ static blk_status_t sd_setup_rw6_cmnd(struct scsi_cmnd *cmd, bool write, sector_t lba, unsigned int nr_blocks, unsigned char flags) { + /* Avoid that 0 blocks gets translated into 256 blocks. */ + if (WARN_ON_ONCE(nr_blocks == 0)) + return BLK_STS_IOERR; + if (unlikely(flags & 0x8)) { /* * This happens only if this drive failed 10byte rw
[PATCH v2 0/9] phy: qcom-ufs: Enable regulators to be off in suspend
The goal with this series is to enable shutting off regulators that power UFS during system suspend. In "the good life" version of this, we'd just disable the regulators in phy_poweroff and be done with it. Unfortunately, that's not symmetric, as regulators are not enabled during phy_poweron. Ok, so you might think we could just move the regulator enable and anything else that needs to come along into phy_poweron, so that we can then undo it all in phy_poweroff. That's where things get tricky. The qcom-qmp-phy overloaded the phy_init and phy_poweron callbacks, basically to mean "init phase 1" and "init phase 2". There are two phases because they have this phy_reset bit outside of the phy (in the UFS controller registers), and they need to make sure this bit is toggled at specific points in the phy init sequence. So there's this implicit sequence in the init dance between ufs-qcom.c and phy-qcom-qmp.c: 1) ufs-qcom asserts the PHY reset bit. 2) phy-qcom-qmp phy_init does most of its initialization, but exits early. 3) ufs-qcom deasserts the PHY reset bit. 4) phy-qcom-qmp phy_poweron finishes its initialization. This init dance is very difficult to follow in the code (since it's split between two drivers and not spelled out well), and arguably represents a deficiency in the hardware description of these devices. In this series I'm proposing tweaking the bindings for the Qualcomm UFS controller and PHY. In it we expose a reset controller from the UFS controller, that is then picked up and used from the PHY code. With this, the phy code can be reorganized to complete its initialization in a single function, removing the implicit two-phase overloading. Then I can move most of the phy initialization, including enabling the regulators, into phy_poweron. Now, when phy_poweroff is called, the phy actually powers off. This finally disables the regulators and allows me to save power in system suspend. Because the UFS PHY reset bit is now toggled in the PHY, rather than in ufs-qcom, this also percolated to all other PHYs using ufs-qcom, which from what I can see is just 8996. There are a couple of tradeoffs in this series that I'd welcome feedback on. First, it breaks compatibility with device trees that don't expose this new reset controller. Making this work with older device trees would be pretty ugly in the code, and given that the SDM845 UFS DT nodes aren't accepted upstream yet, the breakage seemed worth it. I'm not as sure about 8996. Second, I removed the calls to phy_poweroff during clock gating. This was originally dialing down a clock or two, while leaving the phy powered. I've now changed the semantics of phy_poweroff to, well, actually power off. This works great for userlands that have set UFS's spm_lvl to 5 (off) like I have, but maybe changes power consumption for devices that have spm_lvl set to 3. I could try to use phy_init and phy_poweron as the two different possible transitions (fully off, and clocks off respectively), but I'm not sure if it actually matters, and I like the idea that phy_poweroff really does power the thing off. Also, I don't have an 8996 device to test. If someone is able to test this out and perhaps point out any (hopefully obvious) bugs in the 8996 portion, I'd be grateful. This patch is based atop phy-next, plus the UFS DT nodes, which are now patch 3, 4, 5 of [1]. [1] https://lore.kernel.org/lkml/20181210192826.241350-1-evgr...@chromium.org/ Changes in v2: - Added resets to example (Stephen). - Remove include of reset.h (Stephen) - Fix error print of phy_power_on (Stephen) - Comment for reset controller warnings on id != 0 (Stephen) - Add static to ufs_qcom_reset_ops (Stephen). - Use devm_* to get the reset (Stephen) - Clear ufs_reset on error getting it - Remove needless error print (Stephen) - Removed whitespace changes (Stephen) - Use devm_ to get the reset (Stephen) Evan Green (9): dt-bindings: ufs: Add #reset-cells for Qualcomm controllers dt-bindings: phy-qcom-qmp: Add UFS PHY reset dt-bindings: phy: qcom-ufs: Add resets property arm64: dts: sdm845: Add UFS PHY reset arm64: dts: msm8996: Add UFS PHY reset controller scsi: ufs: qcom: Expose the reset controller for PHY phy: qcom-qmp: Utilize UFS reset controller phy: qcom-qmp: Move UFS phy to phy_poweron/off phy: qcom-ufs: Refactor all init steps into phy_poweron .../devicetree/bindings/phy/qcom-qmp-phy.txt | 6 +- .../devicetree/bindings/ufs/ufs-qcom.txt | 5 +- .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 3 + arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 3 + drivers/phy/qualcomm/phy-qcom-qmp.c | 122 ++ drivers/phy/qualcomm/phy-qcom-ufs-i.h | 5 +- drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c | 25 +--- drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c | 25 +--- drivers/phy/qualcomm/phy-qcom-ufs.c | 57 ++-- drivers/scsi/ufs/Kconfig | 1 +
[PATCH v2 6/9] scsi: ufs: qcom: Expose the reset controller for PHY
Expose a reset controller that the phy can use to perform its initialization in a single callback. Also, change the use of the phy functions from ufs-qcom such that phy_poweron actually fires up the phy, and phy_poweroff actually powers it down. Signed-off-by: Evan Green --- Note: This change depends on the remaining changes in this series, since UFS PHY reset now needs to be done by the PHY driver. Changes in v2: - Remove include of reset.h (Stephen) - Fix error print of phy_power_on (Stephen) - Comment for reset controller warnings on id != 0 (Stephen) - Add static to ufs_qcom_reset_ops (Stephen). drivers/scsi/ufs/Kconfig| 1 + drivers/scsi/ufs/ufs-qcom.c | 111 ++-- drivers/scsi/ufs/ufs-qcom.h | 4 ++ 3 files changed, 72 insertions(+), 44 deletions(-) diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig index 2ddbb26d9c265..63c5c4115981f 100644 --- a/drivers/scsi/ufs/Kconfig +++ b/drivers/scsi/ufs/Kconfig @@ -100,6 +100,7 @@ config SCSI_UFS_QCOM tristate "QCOM specific hooks to UFS controller platform driver" depends on SCSI_UFSHCD_PLATFORM && ARCH_QCOM select PHY_QCOM_UFS + select RESET_CONTROLLER help This selects the QCOM specific additions to UFSHCD platform driver. UFS host on QCOM needs some vendor specific configuration before diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 3aeadb14aae1e..277ed6ad71c9b 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -49,6 +49,11 @@ static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, u32 clk_cycles); +static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd) +{ + return container_of(rcd, struct ufs_qcom_host, rcdev); +} + static void ufs_qcom_dump_regs_wrapper(struct ufs_hba *hba, int offset, int len, const char *prefix, void *priv) { @@ -255,11 +260,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) if (is_rate_B) phy_set_mode(phy, PHY_MODE_UFS_HS_B); - /* Assert PHY reset and apply PHY calibration values */ - ufs_qcom_assert_reset(hba); - /* provide 1ms delay to let the reset pulse propagate */ - usleep_range(1000, 1100); - /* phy initialization - calibrate the phy */ ret = phy_init(phy); if (ret) { @@ -268,15 +268,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) goto out; } - /* De-assert PHY reset and start serdes */ - ufs_qcom_deassert_reset(hba); - - /* -* after reset deassertion, phy will need all ref clocks, -* voltage, current to settle down before starting serdes. -*/ - usleep_range(1000, 1100); - /* power on phy - start serdes and phy's power and clocks */ ret = phy_power_on(phy); if (ret) { @@ -290,7 +281,6 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) return 0; out_disable_phy: - ufs_qcom_assert_reset(hba); phy_exit(phy); out: return ret; @@ -554,21 +544,10 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufs_qcom_disable_lane_clks(host); phy_power_off(phy); - /* Assert PHY soft reset */ - ufs_qcom_assert_reset(hba); - goto out; - } - - /* -* If UniPro link is not active, PHY ref_clk, main PHY analog power -* rail and low noise analog power rail for PLL can be switched off. -*/ - if (!ufs_qcom_is_link_active(hba)) { + } else if (!ufs_qcom_is_link_active(hba)) { ufs_qcom_disable_lane_clks(host); - phy_power_off(phy); } -out: return ret; } @@ -578,21 +557,26 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) struct phy *phy = host->generic_phy; int err; - err = phy_power_on(phy); - if (err) { - dev_err(hba->dev, "%s: failed enabling regs, err = %d\n", - __func__, err); - goto out; - } + if (ufs_qcom_is_link_off(hba)) { + err = phy_power_on(phy); + if (err) { + dev_err(hba->dev, "%s: failed PHY power on: %d\n", + __func__, err); + return err; + } - err = ufs_qcom_enable_lane_clks(host); - if (err) - goto out; + err = ufs_qcom_enable_lane_clks(host); + if (err) + return err; - hba->is_sys_suspended = false; + } else if (!ufs_qcom_is_link_active(hba)) { + err = ufs_qcom_enable_lane_clk
[PATCH] Fix sense handling in __scsi_execute()
Since blk_execute_rq() no longer allocates a sense buffer and no longer initializes the sense pointer the callers of blk_execute_rq() have to do initialize the sense pointer. Hence this patch that initializes rq->sense and that removes a superfluous memcpy() statement. Cc: Christoph Hellwig Cc: Douglas Gilbert Cc: # v4.11+ Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request") Signed-off-by: Bart Van Assche --- drivers/scsi/scsi_lib.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 4feba3b5aff1..8b9f4b1bca35 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -271,6 +271,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, rq->cmd_len = COMMAND_SIZE(cmd[0]); memcpy(rq->cmd, cmd, rq->cmd_len); rq->retries = retries; + rq->sense = sense; req->timeout = timeout; req->cmd_flags |= flags; req->rq_flags |= rq_flags | RQF_QUIET; @@ -291,8 +292,6 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd, if (resid) *resid = rq->resid_len; - if (sense && rq->sense_len) - memcpy(sense, rq->sense, SCSI_SENSE_BUFFERSIZE); if (sshdr) scsi_normalize_sense(rq->sense, rq->sense_len, sshdr); ret = rq->result; -- 2.20.1.321.g9e740568ce-goog
Re: Kernel v5.0, scsi_debug and libiscsi
On Fri, 2019-01-11 at 13:01 -0500, Douglas Gilbert wrote: > On 2019-01-10 6:22 p.m., Bart Van Assche wrote: > > Hi Doug, > > > > Have you ever tried to run the libiscsi conformance tests against > > the scsi_debug driver? I tried the following: > > > > modprobe scsi_debug delay=0 max_luns=3 > > dev=$(for f in > > /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/[0-9]*/block/*; > > do echo $f; break; done) > > dev=/dev/$(basename $dev) > > libiscsi/test-tool/iscsi-test-cu --dataloss --allow-sanitize "$dev" > > > > That test triggers the following output: > > > > BUG: unable to handle kernel paging request at a8d741235e00 > > PGD 13b141067 P4D 13b141067 PUD 13b146067 PMD 6fc5a067 PTE 0 > > Oops: 0002 [#1] SMP PTI > > CPU: 3 PID: 4967 Comm: iscsi-test-cu Not tainted 4.18.0-13-generic > > #14-Ubuntu > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 > > 04/01/2014 > > RIP: 0010:memcpy_erms+0x6/0x10 > > Since memory corruption errors have been found elsewhere in > lk 5.0-rc1 and a fix looks like it is pending, I will leave this > one alone as I can't replicate it. Hi Doug, I can replicate this crash easily. I also noticed that this crash only occurs if the scsi_debug driver is loaded with fake_rw=0. It does not occur with fake_rw=1. It seems like the following code in resp_write_same() assumes that fake_storep != NULL? /* if ndob then zero 1 logical block, else fetch 1 logical block */ if (ndob) { memset(fake_storep + lba_off, 0, sdebug_sector_size); ret = 0; } else ret = fetch_to_dev_buffer(scp, fake_storep + lba_off, sdebug_sector_size); Bart.
Re: [PATCH] Fix sense handling in __scsi_execute()
On Wed, 2019-01-23 at 14:42 -0800, Bart Van Assche wrote: > Since blk_execute_rq() no longer allocates a sense buffer and no longer > initializes the sense pointer the callers of blk_execute_rq() have to do > initialize the sense pointer. Hence this patch that initializes rq->sense > and that removes a superfluous memcpy() statement. > > Cc: Christoph Hellwig > Cc: Douglas Gilbert > Cc: # v4.11+ > Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request") > Signed-off-by: Bart Van Assche > --- > drivers/scsi/scsi_lib.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 4feba3b5aff1..8b9f4b1bca35 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -271,6 +271,7 @@ int __scsi_execute(struct scsi_device *sdev, const > unsigned char *cmd, > rq->cmd_len = COMMAND_SIZE(cmd[0]); > memcpy(rq->cmd, cmd, rq->cmd_len); > rq->retries = retries; > + rq->sense = sense; > req->timeout = timeout; > req->cmd_flags |= flags; > req->rq_flags |= rq_flags | RQF_QUIET; > @@ -291,8 +292,6 @@ int __scsi_execute(struct scsi_device *sdev, const > unsigned char *cmd, > > if (resid) > *resid = rq->resid_len; > - if (sense && rq->sense_len) > - memcpy(sense, rq->sense, SCSI_SENSE_BUFFERSIZE); > if (sshdr) > scsi_normalize_sense(rq->sense, rq->sense_len, sshdr); > ret = rq->result; Please ignore this patch - I just realized that this is not the right way to fix the issue I ran into. Bart.
[LSF/MM TOPIC] SCSI Error Handling and HBA Recovery
Several SCSI low-level drivers need to suspend .queuecommand() calls while HBA or transport layer recovery happens. The iSCSI and SRP initiator drivers use scsi_target_block() to block new .queuecommand() calls while recovery happens. scsi_target_block() prevents that the block layer core triggers new .queuecommand() calls but does not prevent that the SCSI error handler calls .queuecommand(). SCSI LLD authors have the choice of either hoping that .queuecommand() calls from the SCSI error handler won't happen while transport layer recovery is in progress or to add code in the .queuecommand() function that detects from which context that call comes and to delay such .queuecommand() calls. In the SRP initiator driver that code looks as follows: const bool in_scsi_eh = !in_interrupt() && current == shost->ehandler; /* * The SCSI EH thread is the only context from which srp_queuecommand() * can get invoked for blocked devices (SDEV_BLOCK / * SDEV_CREATED_BLOCK). Avoid racing with srp_reconnect_rport() by * locking the rport mutex if invoked from inside the SCSI EH. */ if (in_scsi_eh) mutex_lock(&rport->mutex); In my opinion the SCSI core should make it easy for LLD authors to prevent that the error handler calls .queuecommand() while transport layer recovery is in progress. So considerable time ago I posted several patches that modify the SCSI error handler and that avoid that SCSI LLDs have to detect the context a .queuecommand() call comes from. None of these patches were accepted and no alternative approach was proposed. Hence the proposal to discuss this topic in person during LSF/MM. See also "[PATCH 1/2] RDMA/srp: Avoid calling mutex_lock() from inside scsi_queue_rq()" (https://www.spinics.net/lists/linux-rdma/msg73842.html). Thanks, Bart.
Re: [PATCH v2 0/7] Fix handling of bidi commands
On 2019-01-23 2:10 p.m., Bart Van Assche wrote: Hi Martin, Recently Doug Gilbert reported that handling of bidi commands is broken in the scsi-mq code. This patch series fixes that bug and also simplifies bidi command handling. Please consider these patches for kernel v5.1. Thanks, Bart. Changes compared to v1: - Fixed a NULL pointer dereference in scsi_init_sdb(). Bart Van Assche (7): Introduce the bidi_supported flag in the host template structure Change scsi_cmnd.prot_sdb from a pointer into a regular member Fix bidi handling Introduce scsi_out_cmd() Move several function definitions in Introduce scsi_in_[sg]et_resid() and scsi_out_[sg]et_resid() Move the resid member from struct scsi_data_buffer into struct scsi_cmnd drivers/scsi/iscsi_tcp.c | 8 +--- drivers/scsi/libiscsi.c| 12 ++--- drivers/scsi/scsi_debug.c | 20 +++-- drivers/scsi/scsi_lib.c| 70 -- drivers/scsi/sd.c | 4 +- drivers/scsi/virtio_scsi.c | 4 +- drivers/target/loopback/tcm_loop.c | 8 +--- drivers/usb/storage/uas.c | 8 ++-- include/scsi/scsi_cmnd.h | 67 include/scsi/scsi_host.h | 2 + 10 files changed, 101 insertions(+), 102 deletions(-) I have been running v2 against the scsi_debug driver mainly doing XDWRITEREAD(10)s with the odd INQUIRY on Martin's 5.1/scsi-queue branch with my sg v4 driver (20190118 version) for 2 hours. So far no sign of problems or memory usage expansion. A script is running this command: sg_tst_bidi -d=4 -q=64 -N -vv -l=0x123 -Q -R=64 /dev/sg1 every 3 seconds was the test. That utility can be found in sg3_utils-1.45 beta rev 808 (testing directory) linked at the top of: http://sg.danny.cz/sg/ It is queuing up 64 SG_IOSUBMITs (nearly all XDWRITEREAD(10)s) then reading their responses back and checking for errors. That is repeated 64 times (-R=64). That was the test that crashed v1 of this patch. Also Boaz Harrosh confirmed to me that without this patchset (or v1) exofs tests "blew up" when SCSI errors were injected (as predicted). So Boaz, could you apply this patchset and try those tests again. So: Tested-by: Douglas Gilbert Please apply this to all patches in this set.
[PATCH] libfc free skb when receiving invalid flogi resp
From: Ming Lu The issue to be fixed in this commit is when libfc found it received a invalid FLOGI response from FC switch, it would return without freeing the fc frame, which is just the skb data. This would cause memory leak if FC switch keeps sending invalid FLOGI responses. This fix is just to make it execute `fc_frame_free(fp)` before returning from function `fc_lport_flogi_resp`. Signed-off-by: Ming Lu --- drivers/scsi/libfc/fc_lport.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index be83590ed955..ff943f477d6f 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -1726,14 +1726,14 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, fc_frame_payload_op(fp) != ELS_LS_ACC) { FC_LPORT_DBG(lport, "FLOGI not accepted or bad response\n"); fc_lport_error(lport, fp); - goto err; + goto out; } flp = fc_frame_payload_get(fp, sizeof(*flp)); if (!flp) { FC_LPORT_DBG(lport, "FLOGI bad response\n"); fc_lport_error(lport, fp); - goto err; + goto out; } mfs = ntohs(flp->fl_csp.sp_bb_data) & @@ -1743,7 +1743,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response, " "lport->mfs:%hu\n", mfs, lport->mfs); fc_lport_error(lport, fp); - goto err; + goto out; } if (mfs <= lport->mfs) { -- 2.17.1
Re: Kernel v5.0, scsi_debug and libiscsi
On 2019-01-23 5:56 p.m., Bart Van Assche wrote: On Fri, 2019-01-11 at 13:01 -0500, Douglas Gilbert wrote: On 2019-01-10 6:22 p.m., Bart Van Assche wrote: Hi Doug, Have you ever tried to run the libiscsi conformance tests against the scsi_debug driver? I tried the following: modprobe scsi_debug delay=0 max_luns=3 dev=$(for f in /sys/bus/pseudo/drivers/scsi_debug/adapter*/host*/target*/[0-9]*/block/*; do echo $f; break; done) dev=/dev/$(basename $dev) libiscsi/test-tool/iscsi-test-cu --dataloss --allow-sanitize "$dev" That test triggers the following output: BUG: unable to handle kernel paging request at a8d741235e00 PGD 13b141067 P4D 13b141067 PUD 13b146067 PMD 6fc5a067 PTE 0 Oops: 0002 [#1] SMP PTI CPU: 3 PID: 4967 Comm: iscsi-test-cu Not tainted 4.18.0-13-generic #14-Ubuntu Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 RIP: 0010:memcpy_erms+0x6/0x10 Since memory corruption errors have been found elsewhere in lk 5.0-rc1 and a fix looks like it is pending, I will leave this one alone as I can't replicate it. Hi Doug, I can replicate this crash easily. I also noticed that this crash only occurs if the scsi_debug driver is loaded with fake_rw=0. It does not occur with fake_rw=1. It seems like the following code in resp_write_same() assumes that fake_storep != NULL? /* if ndob then zero 1 logical block, else fetch 1 logical block */ if (ndob) { memset(fake_storep + lba_off, 0, sdebug_sector_size); ret = 0; } else ret = fetch_to_dev_buffer(scp, fake_storep + lba_off, sdebug_sector_size); It is table driven. It shouldn't call that function if FF_MEDIA_IO is part of that command's flag and fake_storep is NULL. Both WS10 and WS16 have that flag. But there is a problem if virtual_gb > 0 . Could you try the attached patch, it should wrap cleanly in the virtual_gb > 0 case. Doug Gilbert diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 661512bec3ac..b190277d945c 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -735,7 +735,7 @@ static inline bool scsi_debug_lbp(void) (sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10); } -static void *fake_store(unsigned long long lba) +static void *lba2fake_store(unsigned long long lba) { lba = do_div(lba, sdebug_store_sectors); @@ -2514,8 +2514,8 @@ static int do_device_access(struct scsi_cmnd *scmd, u32 sg_skip, u64 lba, return ret; } -/* If fake_store(lba,num) compares equal to arr(num), then copy top half of - * arr into fake_store(lba,num) and return true. If comparison fails then +/* If lba2fake_store(lba,num) compares equal to arr(num), then copy top half of + * arr into lba2fake_store(lba,num) and return true. If comparison fails then * return false. */ static bool comp_write_worker(u64 lba, u32 num, const u8 *arr) { @@ -2643,7 +2643,7 @@ static int prot_verify_read(struct scsi_cmnd *SCpnt, sector_t start_sec, if (sdt->app_tag == cpu_to_be16(0x)) continue; - ret = dif_verify(sdt, fake_store(sector), sector, ei_lba); + ret = dif_verify(sdt, lba2fake_store(sector), sector, ei_lba); if (ret) { dif_errors++; return ret; @@ -3261,10 +3261,12 @@ static int resp_write_scat(struct scsi_cmnd *scp, static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, u32 ei_lba, bool unmap, bool ndob) { + int ret; unsigned long iflags; unsigned long long i; - int ret; - u64 lba_off; + u32 lb_size = sdebug_sector_size; + u64 block, lbaa; + u8 *fs1p; ret = check_device_access_params(scp, lba, num); if (ret) @@ -3276,31 +3278,30 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, unmap_region(lba, num); goto out; } - - lba_off = lba * sdebug_sector_size; + lbaa = lba; + block = do_div(lbaa, sdebug_store_sectors); /* if ndob then zero 1 logical block, else fetch 1 logical block */ + fs1p = fake_storep + (block * lb_size); if (ndob) { - memset(fake_storep + lba_off, 0, sdebug_sector_size); + memset(fs1p, 0, lb_size); ret = 0; } else - ret = fetch_to_dev_buffer(scp, fake_storep + lba_off, - sdebug_sector_size); + ret = fetch_to_dev_buffer(scp, fs1p, lb_size); if (-1 == ret) { write_unlock_irqrestore(&atomic_rw, iflags); return DID_ERROR << 16; - } else if (sdebug_verbose && !ndob && (ret < sdebug_sector_size)) + } else if (sdebug_verbose && !ndob && (ret < lb_size)) sdev_printk(KERN_INFO, scp->device, "%s: %s: lb size=%u, IO sent=%d bytes\n", - my_name, "write same", - sdebug_sector_size, ret); + my_name, "write same", lb_size, ret); /* Copy first sector to remaining blocks */ - for (i = 1 ; i < num ; i++) - memcpy(fake_storep + ((lba + i) * sdebug_sector_size), - fake_storep + lba_off, - sdebug_sector_size); - + for (i = 1 ; i < num ; i++) { + lbaa = lba + i; + block = do_div(lbaa, sdebug_store_sectors); +
[ 0/1] Add support for bus voting in UFS
This patch adds bus voting support using the new interconnect framework. This is dependent on the below mentioned patch series: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=67417 This was tested using Qualcomm SDM 845 interconnect service provider patch. Asutosh Das (1): scsi: qcom-ufs: Add support for bus voting using ICB framework .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 12 ++ drivers/scsi/ufs/ufs-qcom.c| 234 - drivers/scsi/ufs/ufs-qcom.h| 20 ++ 3 files changed, 218 insertions(+), 48 deletions(-) -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
[ 1/1] scsi: qcom-ufs: Add support for bus voting using ICB framework
Adapt to the new ICB framework for bus bandwidth voting. This requires the source/destination port ids. Also this requires a tuple of values. The tuple is for two different paths - from UFS master to BIMC slave. The other is from CPU master to UFS slave. This tuple consists of the average and peak bandwidth. Signed-off-by: Asutosh Das --- .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 12 ++ drivers/scsi/ufs/ufs-qcom.c| 234 - drivers/scsi/ufs/ufs-qcom.h| 20 ++ 3 files changed, 218 insertions(+), 48 deletions(-) diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt index a99ed55..94249ef 100644 --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt @@ -45,6 +45,18 @@ Optional properties: Note: If above properties are not defined it can be assumed that the supply regulators or clocks are always on. +* Following bus parameters are required: +interconnects +interconnect-names +- Please refer to Documentation/devicetree/bindings/interconnect/ + for more details on the above. +qcom,msm-bus,name - string describing the bus path +qcom,msm-bus,num-cases - number of configurations in which ufs can operate in +qcom,msm-bus,num-paths - number of paths to vote for +qcom,msm-bus,vectors-KBps - Takes a tuple , (2 tuples for 2 num-paths) + The number of these entries *must* be same as + num-cases. + Example: ufshc@0xfc598000 { compatible = "jedec,ufs-1.1"; diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 2b38db2..213e975 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include "ufshcd.h" @@ -27,6 +28,10 @@ #define UFS_QCOM_DEFAULT_DBG_PRINT_EN \ (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN) +#define UFS_DDR "ufs-ddr" +#define CPU_UFS "cpu-ufs" + + enum { TSTBUS_UAWM, TSTBUS_UARM, @@ -714,7 +719,6 @@ static int ufs_qcom_get_pwr_dev_param(struct ufs_qcom_dev_params *qcom_param, return 0; } -#ifdef CONFIG_MSM_BUS_SCALING static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host, const char *speed_mode) { @@ -768,24 +772,83 @@ static void ufs_qcom_get_speed_mode(struct ufs_pa_layer_attr *p, char *result) } } +static int ufs_qcom_get_ib_ab(struct ufs_qcom_host *host, int index, + struct qcom_bus_vectors *ufs_ddr_vec, + struct qcom_bus_vectors *cpu_ufs_vec) +{ + struct qcom_bus_path *usecase; + + if (!host->qbsd) + return -EINVAL; + + if (index > host->qbsd->num_usecase) + return -EINVAL; + + usecase = host->qbsd->usecase; + + /* +* +* usecase:0 usecase:0 +* ufs->ddr cpu->ufs +* |vec[0&1] | vec[2&3]| +* +++++ +* | ab | ib | ab | ib | +* |++++ +* . +* . +* . +* usecase:n usecase:n +* ufs->ddr cpu->ufs +* |vec[0&1] | vec[2&3]| +* +++++ +* | ab | ib | ab | ib | +* |++++ +*/ + + /* index refers to offset in usecase */ + ufs_ddr_vec->ab = usecase[index].vec[0].ab; + ufs_ddr_vec->ib = usecase[index].vec[0].ib; + + cpu_ufs_vec->ab = usecase[index].vec[1].ab; + cpu_ufs_vec->ib = usecase[index].vec[1].ib; + + return 0; +} + static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote) { int err = 0; + struct qcom_bus_scale_data *d = host->qbsd; + struct qcom_bus_vectors path0, path1; + struct device *dev = host->hba->dev; - if (vote != host->bus_vote.curr_vote) { - err = msm_bus_scale_client_update_request( - host->bus_vote.client_handle, vote); - if (err) { - dev_err(host->hba->dev, - "%s: msm_bus_scale_client_update_request() failed: bus_client_handle=0x%x, vote=%d, err=%d\n", - __func__, host->bus_vote.client_handle, - vote, err); - goto out; - } + err = ufs_qcom_get_ib_ab(host, vote, &path0, &path1); + if (err) { + dev_err(dev, "Error: failed (%d) to get ib/ab\n", + err); + return err; + } - host->bus_vote.curr_vote = vote; + dev_dbg(dev, "Setting vote: %d: ufs-ddr: ab: %llu ib: %llu\n", vote, + path0.ab, path0.ib); + err = icc_set_bw(d->ufs_ddr, path0.ab, path0.ib); + if (err) { + dev_er
Re: [PATCH] libfc free skb when receiving invalid flogi resp
On 1/24/19 6:25 AM, ming.lu@gmail.com wrote: From: Ming Lu The issue to be fixed in this commit is when libfc found it received a invalid FLOGI response from FC switch, it would return without freeing the fc frame, which is just the skb data. This would cause memory leak if FC switch keeps sending invalid FLOGI responses. This fix is just to make it execute `fc_frame_free(fp)` before returning from function `fc_lport_flogi_resp`. Signed-off-by: Ming Lu --- drivers/scsi/libfc/fc_lport.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index be83590ed955..ff943f477d6f 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -1726,14 +1726,14 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, fc_frame_payload_op(fp) != ELS_LS_ACC) { FC_LPORT_DBG(lport, "FLOGI not accepted or bad response\n"); fc_lport_error(lport, fp); - goto err; + goto out; } flp = fc_frame_payload_get(fp, sizeof(*flp)); if (!flp) { FC_LPORT_DBG(lport, "FLOGI bad response\n"); fc_lport_error(lport, fp); - goto err; + goto out; } mfs = ntohs(flp->fl_csp.sp_bb_data) & @@ -1743,7 +1743,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, FC_LPORT_DBG(lport, "FLOGI bad mfs:%hu response, " "lport->mfs:%hu\n", mfs, lport->mfs); fc_lport_error(lport, fp); - goto err; + goto out; } if (mfs <= lport->mfs) { Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
blk-mq private tags for SCSI
Hi all, blk-mq has the concept of 'private' tags to handle driver-internal commands. Also quite some SCSI HBAs use internal commands for configuration, event handling etc. But sadly no interface to use the 'private' tags from the block layer exists, so quite some drivers like megaraid_sas, aacraid, or hpsa have to implement their own management of internal commands. I have made several prototypes on how private tags could be used in the SCSI context, but neither attempts turned out to be utterly convincing: a) create an abstraction in the block layer to get private tags directly from the tag map. Problem is that the tag allocation mechanism is deeply intertwined with request queues, so desegregating them would involve quite some churn in the block layer. b) create a separate admin queue for private commands. Problem is that then we always will have a SCSI command as the first payload, which in most cases isn't required as most of the internal commands are transport or HBA specific, not SCSI CDBs. Also for some drivers we need to send commands to figure out the configuration, which then influences the size of the tag space, leaving us with a catch-22 situation. I would like to discuss at LSF/MM if using private tags in the SCSI layer is something to be attempted, and if so what would be the best direction to take. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
Re: [PATCH] SCSI: fcoe: remove unneeded fcoe_ctlr_destroy_store export
On 1/22/19 3:28 PM, Greg Kroah-Hartman wrote: There's no need to export fcoe_ctlr_destroy_store as a symbol, so remove the EXPORT_SYMBOL() line for it. Cc: Johannes Thumshirn Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Signed-off-by: Greg Kroah-Hartman --- drivers/scsi/fcoe/fcoe_transport.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c index f4909cd206d3..6d3949b687ef 100644 --- a/drivers/scsi/fcoe/fcoe_transport.c +++ b/drivers/scsi/fcoe/fcoe_transport.c @@ -855,7 +855,6 @@ ssize_t fcoe_ctlr_destroy_store(struct bus_type *bus, mutex_unlock(&ft_mutex); return rc; } -EXPORT_SYMBOL(fcoe_ctlr_destroy_store); /** * fcoe_transport_create() - Create a fcoe interface Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)