From aeb6641f8ebdd61939f462a8255b316f9bfab707 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 14 Mar 2016 15:29:44 +0100 Subject: [PATCH 1/7] lpfc: fix misleading indentation gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array() call in lpfc_online(), which clearly doesn't look right: drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online': drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] lpfc_destroy_vport_work_array(phba, vports); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not if (vports != NULL) ^~ Looking at the patch that introduced this code, it's clear that the behavior is correct and the indentation is wrong. This fixes the indentation and adds curly braces around the previous if() block for clarity, as that is most likely what caused the code to be misindented in the first place. Signed-off-by: Arnd Bergmann Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list") Reviewed-by: Sebastian Herbszt Reviewed-by: Hannes Reinecke Reviewed-by: Ewan D. Milne Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_init.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index a544366a367e..f57d02c3b6cf 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -2860,7 +2860,7 @@ lpfc_online(struct lpfc_hba *phba) } vports = lpfc_create_vport_work_array(phba); - if (vports != NULL) + if (vports != NULL) { for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) { struct Scsi_Host *shost; shost = lpfc_shost_from_vport(vports[i]); @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba) } spin_unlock_irq(shost->host_lock); } - lpfc_destroy_vport_work_array(phba, vports); + } + lpfc_destroy_vport_work_array(phba, vports); lpfc_unblock_mgmt_io(phba); return 0; From 3deb9438d34a09f6796639b652a01d110aca9f75 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 14 Mar 2016 15:29:45 +0100 Subject: [PATCH 2/7] megaraid_sas: add missing curly braces in ioctl handler gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl function: drivers/scsi/megaraid/megaraid_sas_base.c: In function 'megasas_mgmt_fw_ioctl': drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation] kbuff_arr[i] = NULL; ^~~~~~~~~ drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if' clause, but it is not if (kbuff_arr[i]) ^~ The code is actually correct, as there is no downside in clearing a NULL pointer again. This clarifies the code and avoids the warning by adding extra curly braces. Signed-off-by: Arnd Bergmann Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption fix") Reviewed-by: Hannes Reinecke Acked-by: Sumit Saxena Signed-off-by: Martin K. Petersen --- drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 69d375b8f2e1..e6ebc7ae2df1 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -6656,12 +6656,13 @@ out: } for (i = 0; i < ioc->sge_count; i++) { - if (kbuff_arr[i]) + if (kbuff_arr[i]) { dma_free_coherent(&instance->pdev->dev, le32_to_cpu(kern_sge32[i].length), kbuff_arr[i], le32_to_cpu(kern_sge32[i].phys_addr)); kbuff_arr[i] = NULL; + } } megasas_return_cmd(instance, cmd); From bc7095a926b3148bdfc8b282474002abbda0f6b5 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 15 Mar 2016 22:40:31 +0100 Subject: [PATCH 3/7] qla2xxx: avoid maybe_uninitialized warning The qlt_check_reserve_free_req() function produces an incorrect warning when CONFIG_PROFILE_ANNOTATED_BRANCHES is set: drivers/scsi/qla2xxx/qla_target.c: In function 'qlt_check_reserve_free_req': drivers/scsi/qla2xxx/qla_target.c:1887:3: error: 'cnt_in' may be used uninitialized in this function [-Werror=maybe-uninitialized] ql_dbg(ql_dbg_io, vha, 0x305a, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ "qla_target(%d): There is no room in the request ring: vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d Req-Length=%d\n", ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vha->vp_idx, vha->req->ring_index, ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vha->req->cnt, req_cnt, cnt, cnt_in, vha->req->length); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/scsi/qla2xxx/qla_target.c:1887:3: error: 'cnt' may be used uninitialized in this function [-Werror=maybe-uninitialized] The problem is that gcc fails to track the state of the condition across an annotated branch. This slightly rearranges the code to move the second if() block into the first one, to avoid the warning while retaining the behavior of the code. Signed-off-by: Arnd Bergmann Acked-By: Himanshu Madhani Signed-off-by: Martin K. Petersen --- drivers/scsi/qla2xxx/qla_target.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index ee967becd257..124e8de290b3 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1872,15 +1872,17 @@ static int qlt_check_reserve_free_req(struct scsi_qla_host *vha, else vha->req->cnt = vha->req->length - (vha->req->ring_index - cnt); + + if (unlikely(vha->req->cnt < (req_cnt + 2))) { + ql_dbg(ql_dbg_io, vha, 0x305a, + "qla_target(%d): There is no room in the request ring: vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d Req-Length=%d\n", + vha->vp_idx, vha->req->ring_index, + vha->req->cnt, req_cnt, cnt, cnt_in, + vha->req->length); + return -EAGAIN; + } } - if (unlikely(vha->req->cnt < (req_cnt + 2))) { - ql_dbg(ql_dbg_io, vha, 0x305a, - "qla_target(%d): There is no room in the request ring: vha->req->ring_index=%d, vha->req->cnt=%d, req_cnt=%d Req-out=%d Req-in=%d Req-Length=%d\n", - vha->vp_idx, vha->req->ring_index, - vha->req->cnt, req_cnt, cnt, cnt_in, vha->req->length); - return -EAGAIN; - } vha->req->cnt -= req_cnt; return 0; From 14cee5b4de4b9e01438d58d1806e7eef78720405 Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Wed, 16 Mar 2016 14:44:08 +0100 Subject: [PATCH 4/7] fnic: move printk()s outside of the critical code section. This patch moves a printk() outside of the code section where interrupt are disabled. In some cases a flood of error messages may cause a kernel panic. It also removes one of the printk()s because the same error message was printed twice. [709686.317197] Kernel panic - not syncing: Watchdog detected hard LOCKUP on cpu 12 [709686.317200] CPU: 12 PID: 1963 Comm: systemd-journal Tainted: GF O-------------- 3.10.0-229.el7.x86_64 #1 [709686.317201] Hardware name: Cisco Systems Inc UCSB-B200-M3/UCSB-B200-M3, BIOS B200M3.2.2.3.6.030620151309 03/06/2015 [709686.317206] ffffffff8182b2e8 00000000392722ba ffff88046fcc5c48 ffffffff81603f36 [709686.317209] ffff88046fcc5cc8 ffffffff815fd7da 0000000000000010 ffff88046fcc5cd8 [709686.317211] ffff88046fcc5c78 00000000392722ba ffff88046fcc5c88 000000000000000c [709686.317212] Call Trace: [709686.317221] [] dump_stack+0x19/0x1b [709686.317223] [] panic+0xd8/0x1e7 [709686.317227] [] ? watchdog_enable_all_cpus.part.2+0x40/0x40 [709686.317229] [] watchdog_overflow_callback+0xc2/0xd0 [709686.317233] [] __perf_event_overflow+0xa1/0x250 [709686.317235] [] perf_event_overflow+0x14/0x20 [709686.317239] [] intel_pmu_handle_irq+0x1fd/0x410 [709686.317242] [] ? unmap_kernel_range_noflush+0x11/0x20 [709686.317246] [] ? ghes_copy_tofrom_phys+0x124/0x210 [709686.317249] [] perf_event_nmi_handler+0x2b/0x50 [709686.317251] [] nmi_handle.isra.0+0x69/0xb0 [709686.317252] [] do_nmi+0xd0/0x340 [709686.317256] [] end_repeat_nmi+0x1e/0x2e [709686.317260] [] ? memcpy+0xd/0x110 [709686.317263] [] ? memcpy+0xd/0x110 [709686.317265] [] ? memcpy+0xd/0x110 [709686.317269] <> [] ? vgacon_scroll+0x2d7/0x330 [709686.317273] [] scrup+0xfc/0x110 [709686.317275] [] lf+0xa0/0xb0 [709686.317278] [] vt_console_print+0x2d2/0x420 [709686.317283] [] call_console_drivers.constprop.15+0x91/0xf0 [709686.317287] [] console_unlock+0x3bf/0x400 [709686.317291] [] vprintk_emit+0x2b6/0x530 [709686.317294] [] printk_emit+0x44/0x5b [709686.317297] [] devkmsg_writev+0x158/0x1d0 [709686.317303] [] do_sync_readv_writev+0x79/0xd0 [709686.317307] [] do_readv_writev+0xce/0x260 [709686.317310] [] ? __sb_start_write+0x58/0x110 [709686.317314] [] vfs_writev+0x35/0x60 [709686.317318] [] SyS_writev+0x5c/0xd0 [709686.317322] [] system_call_fastpath+0x16/0x1b Signed-off-by: Maurizio Lombardi Reviewed-by: Laurence Oberman Signed-off-by: Martin K. Petersen --- drivers/scsi/fnic/fnic_scsi.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index 266b909fe854..f3032ca5051b 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -958,22 +958,21 @@ static void fnic_fcpio_icmnd_cmpl_handler(struct fnic *fnic, case FCPIO_INVALID_PARAM: /* some parameter in request invalid */ case FCPIO_REQ_NOT_SUPPORTED:/* request type is not supported */ default: - shost_printk(KERN_ERR, fnic->lport->host, "hdr status = %s\n", - fnic_fcpio_status_to_str(hdr_status)); sc->result = (DID_ERROR << 16) | icmnd_cmpl->scsi_status; break; } + /* Break link with the SCSI command */ + CMD_SP(sc) = NULL; + CMD_FLAGS(sc) |= FNIC_IO_DONE; + + spin_unlock_irqrestore(io_lock, flags); + if (hdr_status != FCPIO_SUCCESS) { atomic64_inc(&fnic_stats->io_stats.io_failures); shost_printk(KERN_ERR, fnic->lport->host, "hdr status = %s\n", fnic_fcpio_status_to_str(hdr_status)); } - /* Break link with the SCSI command */ - CMD_SP(sc) = NULL; - CMD_FLAGS(sc) |= FNIC_IO_DONE; - - spin_unlock_irqrestore(io_lock, flags); fnic_release_ioreq_buf(fnic, io_req, sc); From ef3fb2422ffe92ead0579c21a02095b329434900 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 16 Mar 2016 17:39:17 +0100 Subject: [PATCH 5/7] scsi: fc: use get/put_unaligned64 for wwn access A bug in the gcc-6.0 prerelease version caused at least one driver (lpfc) to have excessive stack usage when dealing with wwn data, on the ARM architecture. lpfc_scsi.c: In function 'lpfc_find_next_oas_lun': lpfc_scsi.c:117:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] I have reported this as a gcc regression in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232 However, using a better implementation of wwn_to_u64() not only helps with the particular gcc problem but also leads to better object code for any version or architecture. The kernel already provides get_unaligned_be64() and put_unaligned_be64() helper functions that provide an optimized implementation with the desired semantics. The lpfc_find_next_oas_lun() function in the example that grew from 1146 bytes to 5144 bytes when moving from gcc-5.3 to gcc-6.0 is now 804 bytes, as the optimized get_unaligned_be64() load can be done in three instructions. The stack usage is now down to 28 bytes from 128 bytes with gcc-5.3 before. Signed-off-by: Arnd Bergmann Reviewed-by: Hannes Reinicke Reviewed-by: Ewan Milne Signed-off-by: Martin K. Petersen --- include/scsi/scsi_transport_fc.h | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h index 784bc2c0929f..bf66ea6bed2b 100644 --- a/include/scsi/scsi_transport_fc.h +++ b/include/scsi/scsi_transport_fc.h @@ -28,6 +28,7 @@ #define SCSI_TRANSPORT_FC_H #include +#include #include #include @@ -797,22 +798,12 @@ fc_remote_port_chkready(struct fc_rport *rport) static inline u64 wwn_to_u64(u8 *wwn) { - return (u64)wwn[0] << 56 | (u64)wwn[1] << 48 | - (u64)wwn[2] << 40 | (u64)wwn[3] << 32 | - (u64)wwn[4] << 24 | (u64)wwn[5] << 16 | - (u64)wwn[6] << 8 | (u64)wwn[7]; + return get_unaligned_be64(wwn); } static inline void u64_to_wwn(u64 inm, u8 *wwn) { - wwn[0] = (inm >> 56) & 0xff; - wwn[1] = (inm >> 48) & 0xff; - wwn[2] = (inm >> 40) & 0xff; - wwn[3] = (inm >> 32) & 0xff; - wwn[4] = (inm >> 24) & 0xff; - wwn[5] = (inm >> 16) & 0xff; - wwn[6] = (inm >> 8) & 0xff; - wwn[7] = inm & 0xff; + put_unaligned_be64(inm, wwn); } /** From c80fa12e6d4ab07b6aff9be1b3be7df265b9497b Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Thu, 17 Mar 2016 13:29:52 +0100 Subject: [PATCH 6/7] scsi: ufs: select CONFIG_NLS A recent change to ufshcd introduced a call to utf16s_to_utf8s, a function that is provided by the NLS module, so we get a link error when that is not present: drivers/scsi/built-in.o: In function `ufshcd_read_string_desc': :(.text+0x124d0): undefined reference to `utf16s_to_utf8s' This adds a Kconfig 'select' statement to avoid the build error. Signed-off-by: Arnd Bergmann Fixes: b573d484e4ff ("scsi: ufs: add support to read device and string descriptors") Signed-off-by: Martin K. Petersen --- drivers/scsi/ufs/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig index 5f4530744e0a..097894a1fab5 100644 --- a/drivers/scsi/ufs/Kconfig +++ b/drivers/scsi/ufs/Kconfig @@ -37,6 +37,7 @@ config SCSI_UFSHCD depends on SCSI && SCSI_DMA select PM_DEVFREQ select DEVFREQ_GOV_SIMPLE_ONDEMAND + select NLS ---help--- This selects the support for UFS devices in Linux, say Y and make sure that you know the name of your UFS host adapter (the card From ba08311647892cc7912de74525fd78416caf544a Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 18 Mar 2016 14:55:38 +0100 Subject: [PATCH 7/7] scsi_common: do not clobber fixed sense information For fixed sense the information field is 32 bits, to we need to truncate the information field to avoid clobbering the sense code. Fixes: a1524f226a02 ("libata-eh: Set 'information' field for autosense") Cc: #v4.1+ Signed-off-by: Hannes Reinecke Reviewed-by: Lee Duncan Reviewed-by: Bart Van Assche Reviewed-by: Ewan D. Milne Signed-off-by: Martin K. Petersen --- drivers/scsi/scsi_common.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c index c126966130ab..ce79de822e46 100644 --- a/drivers/scsi/scsi_common.c +++ b/drivers/scsi/scsi_common.c @@ -278,8 +278,16 @@ int scsi_set_sense_information(u8 *buf, int buf_len, u64 info) ucp[3] = 0; put_unaligned_be64(info, &ucp[4]); } else if ((buf[0] & 0x7f) == 0x70) { - buf[0] |= 0x80; - put_unaligned_be64(info, &buf[3]); + /* + * Only set the 'VALID' bit if we can represent the value + * correctly; otherwise just fill out the lower bytes and + * clear the 'VALID' flag. + */ + if (info <= 0xffffffffUL) + buf[0] |= 0x80; + else + buf[0] &= 0x7f; + put_unaligned_be32((u32)info, &buf[3]); } return 0;