[Bug 188061] On quad port QLE2564 can't add in target only 2 ports

2017-03-03 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=188061

--- Comment #8 from Anthony (anthony.blood...@gmail.com) ---
I added output in text attachment. By construction this card have 3 PCI-E
switches onboard.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 188061] On quad port QLE2564 can't add in target only 2 ports

2017-03-03 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=188061

--- Comment #7 from Anthony (anthony.blood...@gmail.com) ---
Created attachment 255073
  --> https://bugzilla.kernel.org/attachment.cgi?id=255073=edit
Output lspci for 4-port card

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH v3 01/16] lpfc: Correct WQ creation for pagesize

2017-03-03 Thread Mauricio Faria de Oliveira 

Hi Martin and James,

On 02/12/2017 07:52 PM, James Smart wrote:

Correct WQ creation for pagesize


Reviewed-by: Mauricio Faria de Oliveira 


Please flag this patch for stable.

This patch resolves a serious problem on IBM Power systems at least.

An (apparently constant) series of invalid iotags cause a series of
SCSI command aborts that escalates into host reset, which fails too, 
bringing the adapter offline.


And it takes forever to boot, as udev waits on the initial SCSI I/O.

Thank you.


[   17.044690] lpfc :01:00.1: 1:0372 iotag x0 is out off range: max 
iotag (x840)

...
[  723.680700] lpfc :01:00.1: 1:(0):0714 SCSI layer issued Bus Reset 
Data: x2003
[  723.680873] lpfc :01:00.1: 1:(0):3172 SCSI layer issued Host 
Reset Data:
[  724.052506] lpfc :01:00.1: 1:0383 Error -5 during scsi sgl post 
operation
[  724.052701] lpfc :01:00.1: 1:(0):3323 Failed host reset, bring it 
offline



--
Mauricio Faria de Oliveira
IBM Linux Technology Center

Re: [PATCH v2 10/11] lpfc: Add missing memory barrier

2017-03-03 Thread Mauricio Faria de Oliveira 

On 12/19/2016 09:07 PM, James Smart wrote:

On loosely ordered memory systems (PPC for example), the WQE elements
were being updated in memory, but not necessarily flushed before the
separate doorbell was written to hw which would cause hw to dma the
WQE element. Thus, the hardware occasionally received partially
updated WQE data.

Add the memory barrier after updating the WQE memory.


Reviewed-by: Mauricio Faria de Oliveira 

Martin, may you please flag this patch for stable?

Thank you,

--
Mauricio Faria de Oliveira
IBM Linux Technology Center

[GIT PULL] final round of SCSI updates for the 4.10+ merge window

2017-03-03 Thread James Bottomley 
This is the set of stuff that didn't quite make the initial pull and a
set of fixes for stuff which did.  The new stuff is basically lpfc
(nvme), qedi and aacraid.  The fixes cover a lot of previously
submitted stuff, the most important of which probably covers some of
the failing irq vectors allocation and other fallout from having the
SCSI command allocated as part of the block allocation functions.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-misc

The short changelog is:

Arnd Bergmann (3):
  scsi: lpfc: use proper format string for dma_addr_t
  scsi: lpfc: use div_u64 for 64-bit division
  scsi: smartpqi: fix time handling

Christoph Hellwig (7):
  scsi: remove scsi_execute_req_flags
  scsi: merge __scsi_execute into scsi_execute
  scsi: simplify scsi_execute_req_flags
  scsi: make the sense header argument to scsi_test_unit_ready mandatory
  scsi: sd: improve TUR handling in sd_check_events
  scsi: always zero sshdr in scsi_normalize_sense
  scsi: lpfc: use pci_irq_alloc_vectors and pci_irq_free_vectors

Colin Ian King (3):
  scsi: aacraid: remove redundant zero check on ret
  scsi: qla2xxx: fix spelling mistake: "seperator" -> "separator"
  scsi: fix memory leak of sdpk on when gd fails to allocate

Dan Carpenter (1):
  scsi: scsi_dh_emc: return success in clariion_std_inquiry()

Don Brace (1):
  scsi: cciss: correct check map error.

Dupuis, Chad (3):
  scsi: qedi: Fix memory leak in tmf response processing.
  scsi: qedf: fixup compilation warning about atomic_t usage
  scsi: qedf: Add QLogic FastLinQ offload FCoE driver framework.

Finn Thain (1):
  scsi: mac_scsi: Fix MAC_SCSI=m option when SCSI=m

Hannes Reinecke (2):
  scsi: mpt3sas: switch to pci_alloc_irq_vectors
  scsi: use 'scsi_device_from_queue()' for scsi_dh

James Smart (16):
  scsi: lpfc: add missing Kconfig NVME dependencies
  scsi: lpfc: Update lpfc version to 11.2.0.7
  scsi: lpfc: Update copyrights
  scsi: lpfc: NVME Target: Add debugfs support
  scsi: lpfc: NVME Target: bind to nvmet_fc api
  scsi: lpfc: NVME Target: Merge into FC discovery
  scsi: lpfc: NVME Target: Receive buffer updates
  scsi: lpfc: NVME Target: Base modifications
  scsi: lpfc: NVME Initiator: Add debugfs support
  scsi: lpfc: NVME Initiator: bind to nvme_fc api
  scsi: lpfc: NVME Initiator: Merge into FC discovery
  scsi: lpfc: NVME Initiator: Base modifications
  scsi: lpfc: refactor debugfs queue dump routines
  scsi: lpfc: refactor debugfs queue prints
  scsi: lpfc: minor code cleanups
  scsi: lpfc: Correct WQ creation for pagesize

Matthew R. Ochs (1):
  scsi: cxlflash: Enable PCI device ID for future IBM CXL Flash AFU

Michael Hernandez (3):
  scsi: qla2xxx: Fix Regression introduced by 
pci_alloc_irq_vectors_affinity call.
  scsi: qla2xxx: Fix response queue count for Target mode.
  scsi: qla2xxx: Cleaned up queue configuration code.

Raghava Aditya Renukunta (16):
  scsi: aacraid: Fixed expander hotplug for SMART family
  scsi: aacraid: Update driver version
  scsi: aacraid: Fix a potential spinlock double unlock bug
  scsi: aacraid: Save adapter fib log before an IOP reset
  scsi: aacraid: Reorder Adapter status check
  scsi: aacraid: Skip IOP reset on controller panic(SMART Family)
  scsi: aacraid: Decrease adapter health check interval
  scsi: aacraid: Reload offlined drives after controller reset
  scsi: aacraid: Skip wellness sync on controller failure
  scsi: aacraid: Fix sync fibs time out on controller reset
  scsi: aacraid: Added sysfs for driver version
  scsi: aacraid: Fix memory leak in fib init path
  scsi: aacraid: Prevent E3 lockup when deleting units
  scsi: aacraid: Fix for excessive prints on EEH
  scsi: aacraid: Use correct channel number for raw srb
  scsi: aacraid: Fix camel case

Subhash Jadavani (1):
  scsi: ufs-qcom: remove redundant condition check

Wei Yongjun (1):
  scsi: sd: make sd_devt_release() static

And the diffstat

 MAINTAINERS |6 +
 drivers/ata/libata-scsi.c   |   12 +-
 drivers/block/cciss.c   |   32 +-
 drivers/scsi/Kconfig|4 +-
 drivers/scsi/Makefile   |1 +
 drivers/scsi/aacraid/aachba.c   |   59 +-
 drivers/scsi/aacraid/aacraid.h  |  107 +-
 drivers/scsi/aacraid/commctrl.c |2 +-
 drivers/scsi/aacraid/comminit.c |2 +-
 drivers/scsi/aacraid/commsup.c  |  118 +-
 drivers/scsi/aacraid/linit.c|   47 +-
 drivers/scsi/aacraid/rx.c   |2 +-
 drivers/scsi/aacraid/src.c  |   48 +-
 drivers/scsi/cxlflash/main.c|4 +
 drivers/scsi/cxlflash/main.h|1 +

[RFC] hv_storvsc: error handling.

2017-03-03 Thread Stephen Hemminger 
Needs more testing but this does fix the observed problem.

From: Stephen Hemminger 

Subject: [PATCH] hv_storvsc: fix error handling

The Hyper-V storvsc SCSI driver was hiding all errors in INQUIRY and
MODE_SENSE commands. This caused the scan process to incorrectly think
devices were present and online.  Also invalid LUN errors were not
being handled correctly.

This fixes problems booting a GEN2 VM on Hyper-V. It effectively
reverts commit 4ed51a21c0f69 ("Staging: hv: storvsc: Fixup
srb and scsi status for INQUIRY and MODE_SENSE")

Signed-off-by: Stephen Hemminger 
---
 drivers/scsi/storvsc_drv.c | 48 --
 1 file changed, 4 insertions(+), 44 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 638e5f427c90..8cc241fc54b8 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -543,28 +543,6 @@ static void storvsc_host_scan(struct work_struct *work)
kfree(wrk);
 }
 
-static void storvsc_remove_lun(struct work_struct *work)
-{
-   struct storvsc_scan_work *wrk;
-   struct scsi_device *sdev;
-
-   wrk = container_of(work, struct storvsc_scan_work, work);
-   if (!scsi_host_get(wrk->host))
-   goto done;
-
-   sdev = scsi_device_lookup(wrk->host, 0, wrk->tgt_id, wrk->lun);
-
-   if (sdev) {
-   scsi_remove_device(sdev);
-   scsi_device_put(sdev);
-   }
-   scsi_host_put(wrk->host);
-
-done:
-   kfree(wrk);
-}
-
-
 /*
  * We can get incoming messages from the host that are not in response to
  * messages that we have sent out. An example of this would be messages
@@ -955,8 +933,7 @@ static void storvsc_handle_error(struct vmscsi_request 
*vm_srb,
}
break;
case SRB_STATUS_INVALID_LUN:
-   do_work = true;
-   process_err_fn = storvsc_remove_lun;
+   set_host_byte(scmnd, DID_NO_CONNECT);
break;
case SRB_STATUS_ABORTED:
if (vm_srb->srb_status & SRB_STATUS_AUTOSENSE_VALID &&
@@ -1050,32 +1027,15 @@ static void storvsc_on_io_completion(struct 
storvsc_device *stor_device,
 
stor_pkt = >vstor_packet;
 
-   /*
-* The current SCSI handling on the host side does
-* not correctly handle:
-* INQUIRY command with page code parameter set to 0x80
-* MODE_SENSE command with cmd[2] == 0x1c
-*
-* Setup srb and scsi status so this won't be fatal.
-* We do this so we can distinguish truly fatal failues
-* (srb status == 0x4) and off-line the device in that case.
-*/
-
-   if ((stor_pkt->vm_srb.cdb[0] == INQUIRY) ||
-  (stor_pkt->vm_srb.cdb[0] == MODE_SENSE)) {
-   vstor_packet->vm_srb.scsi_status = 0;
-   vstor_packet->vm_srb.srb_status = SRB_STATUS_SUCCESS;
-   }
-
-
/* Copy over the status...etc */
stor_pkt->vm_srb.scsi_status = vstor_packet->vm_srb.scsi_status;
stor_pkt->vm_srb.srb_status = vstor_packet->vm_srb.srb_status;
stor_pkt->vm_srb.sense_info_length =
vstor_packet->vm_srb.sense_info_length;
 
-   if (vstor_packet->vm_srb.scsi_status != 0 ||
-   vstor_packet->vm_srb.srb_status != SRB_STATUS_SUCCESS)
+   if (stor_pkt->vm_srb.cdb[0] != INQUIRY &&
+   (vstor_packet->vm_srb.scsi_status != 0 ||
+vstor_packet->vm_srb.srb_status != SRB_STATUS_SUCCESS))
storvsc_log(device, STORVSC_LOGGING_WARN,
"cmd 0x%x scsi status 0x%x srb status 0x%x\n",
stor_pkt->vm_srb.cdb[0],
-- 
2.11.0

Re: SCSI regression in 4.11

2017-03-03 Thread Stephen Hemminger 
On Thu, 02 Mar 2017 11:18:23 -0800
James Bottomley  wrote:

> On March 2, 2017 11:05:05 AM PST, Stephen Hemminger 
>  wrote:
> >On Thu, 02 Mar 2017 10:36:17 -0800
> >James Bottomley  wrote:
> >  
> >> On March 2, 2017 10:23:24 AM PST, Stephen Hemminger  
> > wrote:  
> >> >On Thu, 2 Mar 2017 14:25:14 +0100
> >> >Hannes Reinecke  wrote:
> >> >
> >> >> On 03/02/2017 02:40 AM, Stephen Hemminger wrote:
> >> >> > On Thu, 2 Mar 2017 01:56:15 +0100
> >> >> > Christoph Hellwig  wrote:
> >> >> >  
> >> >> >> On Thu, Mar 02, 2017 at 01:01:35AM +0100, Christoph Hellwig  
> >wrote:
> >> > 
> >> >> >>> On Wed, Mar 01, 2017 at 07:54:12AM -0800, Stephen Hemminger
> >> >wrote:  
> >> >> >
> >>
> >>
> >> http://git.infradead.org/users/hch/block.git/commitdiff/148cff67b401e2229c076c0ea418712654be77e4
> >>   
> >> > 
> >> >> 
> >> >>  It appears that is already in the code I am testing in
> >> >linux-next...  
> >> >> >>>
> >> >> >>> It's in -next now, but it wasn't at the time you reported the   
> >  
> >> >bug.
> >> >> >>>
> >> >> >>> And it would sortof explain the bug if the INQUIRY data is
> >> >correct
> >> >> >>> in the scatterlist, but we ignore it, given that  
> >scsi_probe_lun  
> >> >> >>> ignores the result based on sense data.
> >> >> >>>
> >> >> >>> Can you check what happens with the horrible hack below:  
> >> >> >>
> >> >> >> Strike that - we're checking result later, so this can't be the  
> >   
> >> >case.
> >> >> >>
> >> >> >> Now the other interesting thing is the memset in  
> >__scsi_exectute,  
> >> >> >> which looks very suspicious.  Try the following please:
> >> >> >>
> >> >> >> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> >> >> >> index 3e32dc954c3c..22f4fb550561 100644
> >> >> >> --- a/drivers/scsi/scsi_lib.c
> >> >> >> +++ b/drivers/scsi/scsi_lib.c
> >> >> >> @@ -253,7 +253,8 @@ static int __scsi_execute(struct  
> >scsi_device
> >> >*sdev, const unsigned char *cmd,
> >> >> >>   * and prevent security leaks by zeroing out the excess data.
> >> >> >>   */
> >> >> >>  if (unlikely(rq->resid_len > 0 && rq->resid_len <= bufflen))
> >> >> >> -memset(buffer + (bufflen - rq->resid_len), 0,  
> >rq->resid_len);  
> >> >> >> +//  memset(buffer + (bufflen - rq->resid_len), 0,  
> >rq->resid_len);  
> >> >> >> +printk_ratelimited("%s: got resid %d\n", __func__,
> >> >rq->resid_len);
> >> >> >>
> >> >> >>  if (resid)
> >> >> >>  *resid = rq->resid_len;  
> >> >> >
> >> >> >
> >> >> > Still fails but does print resid on some of the later INQUIRY
> >> >commands (not the initial one).
> >> >> >  
> >> >> Can you test what happens if you blank out the storvsc_drv
> >> >workaround:
> >> >> 
> >> >> diff --git a/drivers/scsi/storvsc_drv.c  
> >b/drivers/scsi/storvsc_drv.c  
> >> >> index 585e54f..c36f42d 100644
> >> >> --- a/drivers/scsi/storvsc_drv.c
> >> >> +++ b/drivers/scsi/storvsc_drv.c
> >> >> @@ -1060,13 +1060,13 @@ static void  
> >storvsc_on_io_completion(struct   
> >> >> storvsc_device *stor_device,
> >> >>   * We do this so we can distinguish truly fatal failues
> >> >>   * (srb status == 0x4) and off-line the device in that  
> >case.  
> >> >>   */
> >> >> -
> >> >> +#if 0
> >> >>  if ((stor_pkt->vm_srb.cdb[0] == INQUIRY) ||
> >> >> (stor_pkt->vm_srb.cdb[0] == MODE_SENSE)) {
> >> >>  vstor_packet->vm_srb.scsi_status = 0;
> >> >>  vstor_packet->vm_srb.srb_status =
> >> >SRB_STATUS_SUCCESS;
> >> >>  }
> >> >> -
> >> >> +#endif
> >> >> 
> >> >>  /* Copy over the status...etc */
> >> >>  stor_pkt->vm_srb.scsi_status =
> >> >vstor_packet->vm_srb.scsi_status;
> >> >> 
> >> >> It might thappen that we're fail to interpret the 'Device not
> >> >present' 
> >> >> status correctly (which will happen for non-connected DVDs)  
> >causing
> >> >the 
> >> >> SCSI stack to make incorrect decisions later on.
> >> >> 
> >> >> Cheers,
> >> >> 
> >> >> Hannes
> >> >
> >> >There are several oddities about the host SCSI interface that I see:
> >> > 1. The host bus seems to report up to 6 devices even though only 2  
> >are  
> >> > present (Disk and CDROM).
> >> >2. The CDROM emulation doesn't report the same status as a real  
> >device.  
> >> > 3. The host emulation of SCSI doesn't support all the page codes  
> >which  
> >> > is why there is the hack.
> >> >
> >> >But as James said, these don't appear to be related to the failure
> >> >because
> >> >the code worked before and only in post 4.11 merege is there a  
> >problem.
> >> 
> >> Your wait for the hang trace is the most suggestive.   It says we're  
>

RE: [PATCH] qla2xxx: Fix ql_dump_buffer

2017-03-03 Thread Madhani, Himanshu 


> -Original Message-
> From: Joe Perches [mailto:j...@perches.com]
> Sent: Thursday, March 2, 2017 5:15 PM
> To: qla2xxx-upstr...@qlogic.com
> Cc: James E.J. Bottomley ; Martin K. Petersen
> ; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] qla2xxx: Fix ql_dump_buffer
> 
> Recent printk changes for KERN_CONT cause this logging to be defectively
> emitted on multiple lines.  Fix it.
> 
> Also reduces object size a trivial amount.
> 
> $ size drivers/scsi/qla2xxx/qla_dbg.o*
>text  data bss dec hex filename
>   39125 0   0   3912598d5 
> drivers/scsi/qla2xxx/qla_dbg.o.new
>   39164 0   0   3916498fc 
> drivers/scsi/qla2xxx/qla_dbg.o.old
> 
> Signed-off-by: Joe Perches 
> ---
>  drivers/scsi/qla2xxx/qla_dbg.c | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_dbg.c b/drivers/scsi/qla2xxx/qla_dbg.c
> index 21d9fb7fc887..51b4179469d1 100644
> --- a/drivers/scsi/qla2xxx/qla_dbg.c
> +++ b/drivers/scsi/qla2xxx/qla_dbg.c
> @@ -2707,13 +2707,9 @@ ql_dump_buffer(uint32_t level, scsi_qla_host_t
> *vha, int32_t id,
>   "%-+5d  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F\n", size);
>   ql_dbg(level, vha, id,
>   "- ---\n");
> - for (cnt = 0; cnt < size; cnt++, buf++) {
> - if (cnt % 16 == 0)
> - ql_dbg(level, vha, id, "%04x:", cnt & ~0xFU);
> - printk(" %02x", *buf);
> - if (cnt % 16 == 15)
> - printk("\n");
> + for (cnt = 0; cnt < size; cnt += 16) {
> + ql_dbg(level, vha, id, "%04x: ", cnt);
> + print_hex_dump(KERN_CONT, "", DUMP_PREFIX_NONE, 16,
> 1,
> +buf + cnt, min(16U, size - cnt), false);
>   }
> - if (cnt % 16 != 0)
> - printk("\n");
>  }
> --
> 2.10.0.rc2.1.g053435c

Looks Good. 

Acked-by: Himanshu Madhani

Re: [PATCH] target/user: Fix possible overwrite of t_data_sg's last iov[]

2017-03-03 Thread Andy Grover 

On 02/27/2017 09:47 PM, lixi...@cmss.chinamobile.com wrote:

From: Xiubo Li 

If there has BIDI data, its first iov[] will overwrite the last
iov[] for se_cmd->t_data_sg.


(+CCing orig BIDI and data block code authors)

Yeah. It looks like because alloc_and_scatter_data_area() (hereafter 
"aasda") is called twice in the BIDI case, and both times iov_cnt is 0, 
the new_iov() call doesn't increment the iov ptr and the first bidi iov 
overwrites the last data iov. Maybe fix this by exiting aasda() with iov 
pointing at the next unused iov in the array? Probably also want to zero 
the iov.



To fix this, we can just increase the iov pointer, but this may
introuduce a new memory leakage bug: If the se_cmd->data_length
and se_cmd->t_bidi_data_sg->length are all not aligned up to the
DATA_BLOCK_SIZE, the actual length needed maybe larger than just
sum of them.


This also sounds right. But the solution below rounds up both data and 
bidi lengths to DATA_BLOCK_SIZE separately. That's not quite right 
either, since the current code (once aasda is fixed) allows the last 
data and first bidi iovs to both have allocations from the same data 
block. Shouldn't we be rounding up the sum of data and bidi data_lengths 
to DATA_BLOCK_SIZE? Call this Option A.


Option B is we go with changing the implementation to always use a 
separate data block for BIDI data (BIDI cmds are rare so no big deal), 
but then also please look into simplifying code in aasda() and 
tcmu_queue_cmd_ring that may now be overly complex.


Thanks -- Regards -- Andy



So, this could be avoided by rounding all the data lengthes up
to DATA_BLOCK_SIZE.

Signed-off-by: Xiubo Li 
---
 drivers/target/target_core_user.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_user.c 
b/drivers/target/target_core_user.c
index 2e33100..59a18fd 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -429,10 +429,11 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, 
size_t cmd_size, size_t d

mb = udev->mb_addr;
cmd_head = mb->cmd_head % udev->cmdr_size; /* UAM */
-   data_length = se_cmd->data_length;
+   data_length = round_up(se_cmd->data_length, DATA_BLOCK_SIZE);
if (se_cmd->se_cmd_flags & SCF_BIDI) {
BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
-   data_length += se_cmd->t_bidi_data_sg->length;
+   data_length += round_up(se_cmd->t_bidi_data_sg->length,
+   DATA_BLOCK_SIZE);
}
if ((command_size > (udev->cmdr_size / 2)) ||
data_length > udev->data_size) {
@@ -503,10 +504,14 @@ static bool is_ring_space_avail(struct tcmu_dev *udev, 
size_t cmd_size, size_t d
entry->req.iov_dif_cnt = 0;

/* Handle BIDI commands */
-   iov_cnt = 0;
-   alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
-   se_cmd->t_bidi_data_nents, , _cnt, false);
-   entry->req.iov_bidi_cnt = iov_cnt;
+   if (se_cmd->se_cmd_flags & SCF_BIDI) {
+   iov_cnt = 0;
+   iov++;
+   alloc_and_scatter_data_area(udev, se_cmd->t_bidi_data_sg,
+   se_cmd->t_bidi_data_nents, , _cnt,
+   false);
+   entry->req.iov_bidi_cnt = iov_cnt;
+   }

/* cmd's data_bitmap is what changed in process */
bitmap_xor(tcmu_cmd->data_bitmap, old_bitmap, udev->data_bitmap,

RE: [PATCH] aacraid: Fix typo in blink status

2017-03-03 Thread Dave Carroll 
> Subject: [PATCH] aacraid: Fix typo in blink status
> 
> The return status of the adapter check on KERNEL_PANIC is supposed to be
> the upper 16 bits of the OMR status register.
> 
> Fixes: c421530bf848604e (scsi: aacraid: Reorder Adpater status check)
> Reported-by: Dan Carpenter 
> Signed-off-by: Raghava Aditya Renukunta
> 
> ---
>  drivers/scsi/aacraid/src.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Dave Carroll

[Bug 188061] On quad port QLE2564 can't add in target only 2 ports

2017-03-03 Thread bugzilla-daemon 
https://bugzilla.kernel.org/show_bug.cgi?id=188061

--- Comment #6 from himanshu.madh...@cavium.com (himanshu.madh...@qlogic.com) 
---
(In reply to Anthony from comment #5)
> I've installed another 4-port card and got same issue - port names have
> variable part at highest bits. On 2-port card port names have variable part
> at lower bits:
> port_name   = "0x2124ff3bcb3e"
> port_name   = "0x2124ff3bcb3f"
> I'm trying to change a port name in NVRAM with FLASUTIL.EXE with no luck.

Hi Anthony, 

Can you please attach output of following command from your 4-port card.

# lspci -vvv -s 

So for example 

# lspci -vvv -s 09.00.0 and so on for each of the 4 ports of 4-port card. 

Thanks,
Himanshu

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[patch] check length passed to SG_NEXT_CMD_LEN

2017-03-03 Thread Peter Chang 
now that i think i've got gmail not marking everything as spam...

\p
From 93409c62db49d15105390315a685e54083029bee Mon Sep 17 00:00:00 2001
From: peter chang 
Date: Wed, 15 Feb 2017 14:11:54 -0800
Subject: [PATCH] [sg] check length passed to SG_NEXT_CMD_LEN

the user can control the size of the next command passed along, but
the value passed to the ioctl isn't checked against the usable
max command size.

Change-Id: I9ac2ae07c35cf5fda62d7afad32c8d9ab6a9ea1d
Tested: sanity checked w/ calling the ioctl w/ a bogus size
---
 drivers/scsi/sg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 9c5c5f2b3962..b47a369cb71c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -976,6 +976,8 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 		result = get_user(val, ip);
 		if (result)
 			return result;
+		if (val > SG_MAX_CDB_SIZE)
+			return -ENOMEM;
 		sfp->next_cmd_len = (val > 0) ? val : 0;
 		return 0;
 	case SG_GET_VERSION_NUM:
-- 
2.12.0.rc1.440.g5b76565f74-goog

[PATCH] vmw_pvscsi: handle the return value from pci_alloc_irq_vectors correctly

2017-03-03 Thread Christoph Hellwig 
It returns the number of vectors allocated when successful, so check for
a negative error only.

Fixes: 2e48e349 ("scsi: vmw_pvscsi: switch to pci_alloc_irq_vectors")
Signed-off-by: Christoph Hellwig 
Reported-by: Loïc Yhuel 
Tested-by: Loïc Yhuel 
---
 drivers/scsi/vmw_pvscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index ef474a748744..c374e3b5c678 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -1487,7 +1487,7 @@ static int pvscsi_probe(struct pci_dev *pdev, const 
struct pci_device_id *id)
irq_flag &= ~PCI_IRQ_MSI;
 
error = pci_alloc_irq_vectors(adapter->dev, 1, 1, irq_flag);
-   if (error)
+   if (error < 0)
goto out_reset_adapter;
 
adapter->use_req_threshold = pvscsi_setup_req_threshold(adapter, true);
-- 
2.11.0

Re: [PATCH] scsi: lpfc: replace init_timer by setup_timer

2017-03-03 Thread James Smart 

looks good

-- james

Signed-off-by:  James Smart 

On 3/3/2017 4:45 AM, Jiri Slaby wrote:

From: Tomas Jasek 

This patch shortens every init_timer in lpfc module
followed by function and data assignment using setup_timer.
This is purely cleanup patch, it does not add new functionality
nor remove any existing functionality.

An init_timer call in this form:

 init_timer(>fc_disctmo);
 vport->fc_disctmo.function = lpfc_disc_timeout;
 vport->fc_disctmo.data = vport;

is shortened to:

 setup_timer(>fc_disctmo, lpfc_disc_timeout, vport);

It increases readability and reduces chances of mistakes done by
developers.

Signed-off-by: Tomas Jasek 
Signed-off-by: Jiri Slaby 
Cc: James Smart 
Cc: Dick Kennedy 
Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
Cc: 
---
  drivers/scsi/lpfc/lpfc_hbadisc.c |  5 ++---
  drivers/scsi/lpfc/lpfc_init.c| 47 +++-
  2 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 194a14d5f8a9..2612dac75186 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -4344,9 +4344,8 @@ lpfc_initialize_node(struct lpfc_vport *vport, struct 
lpfc_nodelist *ndlp,
  {
INIT_LIST_HEAD(>els_retry_evt.evt_listp);
INIT_LIST_HEAD(>dev_loss_evt.evt_listp);
-   init_timer(>nlp_delayfunc);
-   ndlp->nlp_delayfunc.function = lpfc_els_retry_delay;
-   ndlp->nlp_delayfunc.data = (unsigned long)ndlp;
+   setup_timer(>nlp_delayfunc, lpfc_els_retry_delay,
+   (unsigned long)ndlp);
ndlp->nlp_DID = did;
ndlp->vport = vport;
ndlp->phba = vport->phba;
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 0ee429d773f3..f395f2e4aa97 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -3734,17 +3734,14 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, 
struct device *dev)
INIT_LIST_HEAD(>rcv_buffer_list);
spin_lock_init(>work_port_lock);
  
-	init_timer(>fc_disctmo);

-   vport->fc_disctmo.function = lpfc_disc_timeout;
-   vport->fc_disctmo.data = (unsigned long)vport;
+   setup_timer(>fc_disctmo, lpfc_disc_timeout,
+   (unsigned long)vport);
  
-	init_timer(>els_tmofunc);

-   vport->els_tmofunc.function = lpfc_els_timeout;
-   vport->els_tmofunc.data = (unsigned long)vport;
+   setup_timer(>els_tmofunc, lpfc_els_timeout,
+   (unsigned long)vport);
  
-	init_timer(>delayed_disc_tmo);

-   vport->delayed_disc_tmo.function = lpfc_delayed_disc_tmo;
-   vport->delayed_disc_tmo.data = (unsigned long)vport;
+   setup_timer(>delayed_disc_tmo, lpfc_delayed_disc_tmo,
+   (unsigned long)vport);
  
  	error = scsi_add_host_with_dma(shost, dev, >pcidev->dev);

if (error)
@@ -5406,21 +5403,15 @@ lpfc_setup_driver_resource_phase1(struct lpfc_hba *phba)
INIT_LIST_HEAD(>luns);
  
  	/* MBOX heartbeat timer */

-   init_timer(>mbox_tmo);
-   psli->mbox_tmo.function = lpfc_mbox_timeout;
-   psli->mbox_tmo.data = (unsigned long) phba;
+   setup_timer(>mbox_tmo, lpfc_mbox_timeout, (unsigned long)phba);
/* Fabric block timer */
-   init_timer(>fabric_block_timer);
-   phba->fabric_block_timer.function = lpfc_fabric_block_timeout;
-   phba->fabric_block_timer.data = (unsigned long) phba;
+   setup_timer(>fabric_block_timer, lpfc_fabric_block_timeout,
+   (unsigned long)phba);
/* EA polling mode timer */
-   init_timer(>eratt_poll);
-   phba->eratt_poll.function = lpfc_poll_eratt;
-   phba->eratt_poll.data = (unsigned long) phba;
+   setup_timer(>eratt_poll, lpfc_poll_eratt,
+   (unsigned long)phba);
/* Heartbeat timer */
-   init_timer(>hb_tmofunc);
-   phba->hb_tmofunc.function = lpfc_hb_timeout;
-   phba->hb_tmofunc.data = (unsigned long)phba;
+   setup_timer(>hb_tmofunc, lpfc_hb_timeout, (unsigned long)phba);
  
  	return 0;

  }
@@ -5446,9 +5437,8 @@ lpfc_sli_driver_resource_setup(struct lpfc_hba *phba)
 */
  
  	/* FCP polling mode timer */

-   init_timer(>fcp_poll_timer);
-   phba->fcp_poll_timer.function = lpfc_poll_timeout;
-   phba->fcp_poll_timer.data = (unsigned long) phba;
+   setup_timer(>fcp_poll_timer, lpfc_poll_timeout,
+   (unsigned long)phba);
  
  	/* Host attention work mask setup */

phba->work_ha_mask = (HA_ERATT | HA_MBATT | HA_LATT);
@@ -5617,14 +5607,11 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 * Initialize timers used by driver
 */
  
-

Re: [PATCHv4 12/12] mpt3sas: lockless command submission

2017-03-03 Thread Hannes Reinecke 

On 03/03/2017 01:32 PM, Sreekanth Reddy wrote:

On Wed, Feb 22, 2017 at 4:01 PM, Hannes Reinecke  wrote:

Instead of holding 'struct scsiio_tracker' in its own pool we
can embed it into the payload of the scsi command. This allows
us to get rid of the lock when submitting and receiving commands
and streamline operations.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c  | 120 +--
 drivers/scsi/mpt3sas/mpt3sas_base.h  |  23 +++---
 drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  17 +++--
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 118 +-
 drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  35 +
 5 files changed, 105 insertions(+), 208 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 169d185..966a775 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -863,12 +863,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 }

 struct scsiio_tracker *
-mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
+_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)


Can we rename the name of this function as  _base_get_st_from_smid(),
Since we always try to follow below notations,
 * prefix with "_" if the function is used
within this file other wise prefix with "mpt3sas_" if this
function is shared between more than one file.


Sure, will do.


 {
+   struct scsi_cmnd *cmd;
+
if (WARN_ON(!smid) ||
WARN_ON(smid >= ioc->hi_priority_smid))
return NULL;
-   return >scsi_lookup[smid - 1];
+
+   cmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
+   if (cmd)
+   return scsi_cmd_priv(cmd);
+
+   return NULL;
 }

 /**
@@ -889,7 +896,7 @@ struct scsiio_tracker *
struct scsiio_tracker *st;

if (smid < ctl_smid) {
-   st = mpt3sas_get_st_from_smid(ioc, smid);
+   st = _get_st_from_smid(ioc, smid);
if (st)
cb_idx = st->cb_idx;


I think, here we can safely assign "cb_idx"  as "ioc->scsi_io_cb_idx",
no need to get it from scsiio_tracker. since all the smid's below
ctl_smid will we used for SCSI IO's recieved from scsih_qcmd.


In theory, yes.
However, 'cb_idx' get reset to 0xFF once we're done with the command,
so this serves the dual purpose of getting us the callback index _and_ 
telling us if the command is in flight.


[ .. ]

@@ -1297,9 +1305,7 @@ struct scsiio_tracker *
chain_req = list_entry(ioc->free_chain_list.next,
struct chain_tracker, tracker_list);
list_del_init(_req->tracker_list);
-   st = mpt3sas_get_st_from_smid(ioc, smid);
-   if (st)
-   list_add_tail(_req->tracker_list, >chain_list);
+   list_add_tail(_req->tracker_list, >chain_list);
spin_unlock_irqrestore(>scsi_lookup_lock, flags);


Still we have this lock in the IO path. May be we can remove this lock
too by allocating "ioc->ioc->chains_needed_per_io * shost->can_queue"
number of chain buffers and we can acces the required chain buffer
using scmd's tag pluse chain_offset_per_io coressponding to that
repective tag.

May be we will post a separate patch for this after some time.

Actually, I tried this (and I have a patch to move that to use sbitmap), 
but didn't notice any significant improvement.
(But then as I only ran against a middle-range SSD setup this wasn't 
really surprising).

But sure, I can dig it up and include in this patchset.

[ .. ]

@@ -2359,9 +2316,8 @@ int mpt3sas_scsih_issue_locked_tm(struct MPT3SAS_ADAPTER 
*ioc, u16 handle,
goto out;
}

-   /* search for the command */
-   st = _scsih_scsi_lookup_find_by_scmd(ioc, scmd);
-   if (!st) {
+   /* check for completed command */
+   if (st->cb_idx == 0xFF); {


";" should be removed from above line, otherwise always we return task
abort TM with "SUCCESS" status without issueing the TM to Firmware.



Ouch. Of course. Thanks for catching this.

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCHv4 11/12] mpt3sas: simplify _wait_for_commands_to_complete()

2017-03-03 Thread Hannes Reinecke 

On 03/03/2017 12:57 PM, Sreekanth Reddy wrote:

On Wed, Feb 22, 2017 at 4:01 PM, Hannes Reinecke  wrote:

Use 'host_busy' instead of counting outstanding commands by hand.

Suggested-by: Christoph Hellwig 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index e6aafa5..169d185 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2408,9 +2408,9 @@ struct scsiio_tracker *
 * See _wait_for_commands_to_complete() call with regards to this code.
 */
if (ioc->shost_recovery && ioc->pending_io_count) {
-   if (ioc->pending_io_count == 1)
+   ioc->pending_io_count = atomic_read(>shost->host_busy);


This won't consider the scsi IO issued from ioctl path. If that scsi
io is still outstanding then here we will return without waiting for
it to complete.


True. Will be fixing it up.

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries & Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

[PATCH] scsi: lpfc: replace init_timer by setup_timer

2017-03-03 Thread Jiri Slaby 
From: Tomas Jasek 

This patch shortens every init_timer in lpfc module
followed by function and data assignment using setup_timer.
This is purely cleanup patch, it does not add new functionality
nor remove any existing functionality.

An init_timer call in this form:

init_timer(>fc_disctmo);
vport->fc_disctmo.function = lpfc_disc_timeout;
vport->fc_disctmo.data = vport;

is shortened to:

setup_timer(>fc_disctmo, lpfc_disc_timeout, vport);

It increases readability and reduces chances of mistakes done by
developers.

Signed-off-by: Tomas Jasek 
Signed-off-by: Jiri Slaby 
Cc: James Smart 
Cc: Dick Kennedy 
Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
Cc: 
---
 drivers/scsi/lpfc/lpfc_hbadisc.c |  5 ++---
 drivers/scsi/lpfc/lpfc_init.c| 47 +++-
 2 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 194a14d5f8a9..2612dac75186 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -4344,9 +4344,8 @@ lpfc_initialize_node(struct lpfc_vport *vport, struct 
lpfc_nodelist *ndlp,
 {
INIT_LIST_HEAD(>els_retry_evt.evt_listp);
INIT_LIST_HEAD(>dev_loss_evt.evt_listp);
-   init_timer(>nlp_delayfunc);
-   ndlp->nlp_delayfunc.function = lpfc_els_retry_delay;
-   ndlp->nlp_delayfunc.data = (unsigned long)ndlp;
+   setup_timer(>nlp_delayfunc, lpfc_els_retry_delay,
+   (unsigned long)ndlp);
ndlp->nlp_DID = did;
ndlp->vport = vport;
ndlp->phba = vport->phba;
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 0ee429d773f3..f395f2e4aa97 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -3734,17 +3734,14 @@ lpfc_create_port(struct lpfc_hba *phba, int instance, 
struct device *dev)
INIT_LIST_HEAD(>rcv_buffer_list);
spin_lock_init(>work_port_lock);
 
-   init_timer(>fc_disctmo);
-   vport->fc_disctmo.function = lpfc_disc_timeout;
-   vport->fc_disctmo.data = (unsigned long)vport;
+   setup_timer(>fc_disctmo, lpfc_disc_timeout,
+   (unsigned long)vport);
 
-   init_timer(>els_tmofunc);
-   vport->els_tmofunc.function = lpfc_els_timeout;
-   vport->els_tmofunc.data = (unsigned long)vport;
+   setup_timer(>els_tmofunc, lpfc_els_timeout,
+   (unsigned long)vport);
 
-   init_timer(>delayed_disc_tmo);
-   vport->delayed_disc_tmo.function = lpfc_delayed_disc_tmo;
-   vport->delayed_disc_tmo.data = (unsigned long)vport;
+   setup_timer(>delayed_disc_tmo, lpfc_delayed_disc_tmo,
+   (unsigned long)vport);
 
error = scsi_add_host_with_dma(shost, dev, >pcidev->dev);
if (error)
@@ -5406,21 +5403,15 @@ lpfc_setup_driver_resource_phase1(struct lpfc_hba *phba)
INIT_LIST_HEAD(>luns);
 
/* MBOX heartbeat timer */
-   init_timer(>mbox_tmo);
-   psli->mbox_tmo.function = lpfc_mbox_timeout;
-   psli->mbox_tmo.data = (unsigned long) phba;
+   setup_timer(>mbox_tmo, lpfc_mbox_timeout, (unsigned long)phba);
/* Fabric block timer */
-   init_timer(>fabric_block_timer);
-   phba->fabric_block_timer.function = lpfc_fabric_block_timeout;
-   phba->fabric_block_timer.data = (unsigned long) phba;
+   setup_timer(>fabric_block_timer, lpfc_fabric_block_timeout,
+   (unsigned long)phba);
/* EA polling mode timer */
-   init_timer(>eratt_poll);
-   phba->eratt_poll.function = lpfc_poll_eratt;
-   phba->eratt_poll.data = (unsigned long) phba;
+   setup_timer(>eratt_poll, lpfc_poll_eratt,
+   (unsigned long)phba);
/* Heartbeat timer */
-   init_timer(>hb_tmofunc);
-   phba->hb_tmofunc.function = lpfc_hb_timeout;
-   phba->hb_tmofunc.data = (unsigned long)phba;
+   setup_timer(>hb_tmofunc, lpfc_hb_timeout, (unsigned long)phba);
 
return 0;
 }
@@ -5446,9 +5437,8 @@ lpfc_sli_driver_resource_setup(struct lpfc_hba *phba)
 */
 
/* FCP polling mode timer */
-   init_timer(>fcp_poll_timer);
-   phba->fcp_poll_timer.function = lpfc_poll_timeout;
-   phba->fcp_poll_timer.data = (unsigned long) phba;
+   setup_timer(>fcp_poll_timer, lpfc_poll_timeout,
+   (unsigned long)phba);
 
/* Host attention work mask setup */
phba->work_ha_mask = (HA_ERATT | HA_MBATT | HA_LATT);
@@ -5617,14 +5607,11 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 * Initialize timers used by driver
 */
 
-   init_timer(>rrq_tmr);
-   phba->rrq_tmr.function = lpfc_rrq_timeout;
-   phba->rrq_tmr.data = (unsigned

Re: [PATCHv4 12/12] mpt3sas: lockless command submission

2017-03-03 Thread Sreekanth Reddy 
On Wed, Feb 22, 2017 at 4:01 PM, Hannes Reinecke  wrote:
> Instead of holding 'struct scsiio_tracker' in its own pool we
> can embed it into the payload of the scsi command. This allows
> us to get rid of the lock when submitting and receiving commands
> and streamline operations.
>
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c  | 120 
> +--
>  drivers/scsi/mpt3sas/mpt3sas_base.h  |  23 +++---
>  drivers/scsi/mpt3sas/mpt3sas_ctl.c   |  17 +++--
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 118 +-
>  drivers/scsi/mpt3sas/mpt3sas_warpdrive.c |  35 +
>  5 files changed, 105 insertions(+), 208 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 169d185..966a775 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -863,12 +863,19 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
>  }
>
>  struct scsiio_tracker *
> -mpt3sas_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)
> +_get_st_from_smid(struct MPT3SAS_ADAPTER *ioc, u16 smid)

Can we rename the name of this function as  _base_get_st_from_smid(),
Since we always try to follow below notations,
 * prefix with "_" if the function is used
within this file other wise prefix with "mpt3sas_" if this
function is shared between more than one file.

>  {
> +   struct scsi_cmnd *cmd;
> +
> if (WARN_ON(!smid) ||
> WARN_ON(smid >= ioc->hi_priority_smid))
> return NULL;
> -   return >scsi_lookup[smid - 1];
> +
> +   cmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid);
> +   if (cmd)
> +   return scsi_cmd_priv(cmd);
> +
> +   return NULL;
>  }
>
>  /**
> @@ -889,7 +896,7 @@ struct scsiio_tracker *
> struct scsiio_tracker *st;
>
> if (smid < ctl_smid) {
> -   st = mpt3sas_get_st_from_smid(ioc, smid);
> +   st = _get_st_from_smid(ioc, smid);
> if (st)
> cb_idx = st->cb_idx;

I think, here we can safely assign "cb_idx"  as "ioc->scsi_io_cb_idx",
no need to get it from scsiio_tracker. since all the smid's below
ctl_smid will we used for SCSI IO's recieved from scsih_qcmd.

> } else if (smid == ctl_smid)
> @@ -1276,15 +1283,16 @@ struct scsiio_tracker *
>  /**
>   * _base_get_chain_buffer_tracker - obtain chain tracker
>   * @ioc: per adapter object
> - * @smid: smid associated to an IO request
> + * @scmd: SCSI commands of the IO request
>   *
>   * Returns chain tracker(from ioc->free_chain_list)
>   */
>  static struct chain_tracker *
> -_base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER *ioc, u16 smid)
> +_base_get_chain_buffer_tracker(struct MPT3SAS_ADAPTER *ioc,
> +  struct scsi_cmnd *scmd)
>  {
> struct chain_tracker *chain_req;
> -   struct scsiio_tracker *st;
> +   struct scsiio_tracker *st = scsi_cmd_priv(scmd);
> unsigned long flags;
>
> spin_lock_irqsave(>scsi_lookup_lock, flags);
> @@ -1297,9 +1305,7 @@ struct scsiio_tracker *
> chain_req = list_entry(ioc->free_chain_list.next,
> struct chain_tracker, tracker_list);
> list_del_init(_req->tracker_list);
> -   st = mpt3sas_get_st_from_smid(ioc, smid);
> -   if (st)
> -   list_add_tail(_req->tracker_list, >chain_list);
> +   list_add_tail(_req->tracker_list, >chain_list);
> spin_unlock_irqrestore(>scsi_lookup_lock, flags);

Still we have this lock in the IO path. May be we can remove this lock
too by allocating "ioc->ioc->chains_needed_per_io * shost->can_queue"
number of chain buffers and we can acces the required chain buffer
using scmd's tag pluse chain_offset_per_io coressponding to that
repective tag.

May be we will post a separate patch for this after some time.

> return chain_req;
>  }
> @@ -1485,7 +1491,7 @@ struct scsiio_tracker *
>
> /* initializing the chain flags and pointers */
> chain_flags = MPI2_SGE_FLAGS_CHAIN_ELEMENT << MPI2_SGE_FLAGS_SHIFT;
> -   chain_req = _base_get_chain_buffer_tracker(ioc, smid);
> +   chain_req = _base_get_chain_buffer_tracker(ioc, scmd);
> if (!chain_req)
> return -1;
> chain = chain_req->chain_buffer;
> @@ -1525,7 +1531,7 @@ struct scsiio_tracker *
> sges_in_segment--;
> }
>
> -   chain_req = _base_get_chain_buffer_tracker(ioc, smid);
> +   chain_req = _base_get_chain_buffer_tracker(ioc, scmd);
> if (!chain_req)
> return -1;
> chain = chain_req->chain_buffer;
> @@ -1619,7 +1625,7 @@ struct scsiio_tracker *
> }
>
> /* initializing the pointers */
> -   chain_req =

Re: [PATCH v4 09/19] scsi: csiostor: Replace PCI pool old API

2017-03-03 Thread Sergei Shtylyov 

Hello!

On 3/1/2017 6:55 PM, Romain Perier wrote:


The PCI pool API is deprecated. This commits replaces the PCI pool old


   Commit.


API by the appropriated function with the DMA pool API. It also updates


   Appropriate perhaps?


the name of some variables and the content of comments, accordingly.

Signed-off-by: Romain Perier 
Reviewed-by: Peter Senna Tschudin 

[...]

MBR, Sergei

Re: [PATCHv4 11/12] mpt3sas: simplify _wait_for_commands_to_complete()

2017-03-03 Thread Sreekanth Reddy 
On Wed, Feb 22, 2017 at 4:01 PM, Hannes Reinecke  wrote:
> Use 'host_busy' instead of counting outstanding commands by hand.
>
> Suggested-by: Christoph Hellwig 
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.c | 14 --
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index e6aafa5..169d185 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -2408,9 +2408,9 @@ struct scsiio_tracker *
>  * See _wait_for_commands_to_complete() call with regards to this 
> code.
>  */
> if (ioc->shost_recovery && ioc->pending_io_count) {
> -   if (ioc->pending_io_count == 1)
> +   ioc->pending_io_count = atomic_read(>shost->host_busy);

This won't consider the scsi IO issued from ioctl path. If that scsi
io is still outstanding then here we will return without waiting for
it to complete.

> +   if (ioc->pending_io_count == 0)
> wake_up(>reset_wq);
> -   ioc->pending_io_count--;
> }
>  }
>
> @@ -5687,15 +5687,13 @@ struct scsiio_tracker *
>   * _wait_for_commands_to_complete - reset controller
>   * @ioc: Pointer to MPT_ADAPTER structure
>   *
> - * This function waiting(3s) for all pending commands to complete
> + * This function is waiting 10s for all pending commands to complete
>   * prior to putting controller in reset.
>   */
>  static void
>  _wait_for_commands_to_complete(struct MPT3SAS_ADAPTER *ioc)
>  {
> u32 ioc_state;
> -   unsigned long flags;
> -   u16 i;
>
> ioc->pending_io_count = 0;
>
> @@ -5704,11 +5702,7 @@ struct scsiio_tracker *
> return;
>
> /* pending command count */
> -   spin_lock_irqsave(>scsi_lookup_lock, flags);
> -   for (i = 0; i < ioc->scsiio_depth; i++)
> -   if (ioc->scsi_lookup[i].cb_idx != 0xFF)
> -   ioc->pending_io_count++;
> -   spin_unlock_irqrestore(>scsi_lookup_lock, flags);
> +   ioc->pending_io_count = atomic_read(>shost->host_busy);
>
> if (!ioc->pending_io_count)
> return;
> --
> 1.8.5.6
>

[PATCH] scsi: sr: fix oob access in get_capabilities

2017-03-03 Thread Kefeng Wang 
'n = header_length + block_descriptor_length' could be greater than 512,
and will lead to oob access, so enlarge transfer buffer to fix it.

===
BUG: KASAN: slab-out-of-bounds in sr_probe+0x570/0xcc0 at addr 8809020e
Read of size 1 by task kworker/u48:2/188

Signed-off-by: Kefeng Wang 
---
 drivers/scsi/sr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 0b29b93..5a80aa6 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -852,7 +852,7 @@ static void get_capabilities(struct scsi_cd *cd)
 
 
/* allocate transfer buffer */
-   buffer = kmalloc(512, GFP_KERNEL | GFP_DMA);
+   buffer = kmalloc(1024, GFP_KERNEL | GFP_DMA);
if (!buffer) {
sr_printk(KERN_ERR, cd, "out of memory.\n");
return;
-- 
1.7.12.4

Re: [PATCH v4 08/19] scsi: be2iscsi: Replace PCI pool old API

2017-03-03 Thread Sergei Shtylyov 

On 3/1/2017 6:55 PM, Romain Perier wrote:


The PCI pool API is deprecated. This commits replaces the PCI pool old


   Commit.


API by the appropriated function with the DMA pool API.


   Appropriate perhaps?


Signed-off-by: Romain Perier 
Acked-by: Peter Senna Tschudin 
Tested-by: Peter Senna Tschudin 

[...]

MBR, Sergei

Re: [patch] check length passed to SG_NEXT_CMD_LEN

2017-03-03 Thread Dmitry Vyukov 
On Thu, Mar 2, 2017 at 7:29 PM, Peter Chang  wrote:
> now that i think i've got gmail not marking everything as spam...

+syzkaller mailing list as this does not seem to appear anywhere on open web
From 93409c62db49d15105390315a685e54083029bee Mon Sep 17 00:00:00 2001
From: peter chang 
Date: Wed, 15 Feb 2017 14:11:54 -0800
Subject: [PATCH] [sg] check length passed to SG_NEXT_CMD_LEN

the user can control the size of the next command passed along, but
the value passed to the ioctl isn't checked against the usable
max command size.

Change-Id: I9ac2ae07c35cf5fda62d7afad32c8d9ab6a9ea1d
Tested: sanity checked w/ calling the ioctl w/ a bogus size
---
 drivers/scsi/sg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 9c5c5f2b3962..b47a369cb71c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -976,6 +976,8 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
 		result = get_user(val, ip);
 		if (result)
 			return result;
+		if (val > SG_MAX_CDB_SIZE)
+			return -ENOMEM;
 		sfp->next_cmd_len = (val > 0) ? val : 0;
 		return 0;
 	case SG_GET_VERSION_NUM:
-- 
2.12.0.rc1.440.g5b76565f74-goog

Re: [PATCH] [v2] scsi: qedi: fix build error without DEBUG_FS

2017-03-03 Thread Rangankar, Manish 

On 02/03/17 8:28 PM, "Arnd Bergmann"  wrote:

>Without CONFIG_DEBUG_FS, we run into a link error:
>
>drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_poll':
>qedi_iscsi.c:(.text.qedi_ep_poll+0x134): undefined reference to
>`do_not_recover'
>drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_disconnect':
>qedi_iscsi.c:(.text.qedi_ep_disconnect+0x36c): undefined reference to
>`do_not_recover'
>drivers/scsi/qedi/qedi_iscsi.o: In function `qedi_ep_connect':
>qedi_iscsi.c:(.text.qedi_ep_connect+0x350): undefined reference to
>`do_not_recover'
>drivers/scsi/qedi/qedi_fw.o: In function `qedi_tmf_work':
>qedi_fw.c:(.text.qedi_tmf_work+0x3b4): undefined reference to
>`do_not_recover'
>
>This defines the symbol as a constant in this case, as there is no way to
>set it to anything other than zero without DEBUG_FS. In addition, I'm
>renaming
>it to qedi_do_not_recover in order to put it into a driver specific
>namespace,
>as "do_not_recover" is a really bad name for a kernel-wide global
>identifier
>when it is used only in one driver.
>
>Fixes: ace7f46ba5fd ("scsi: qedi: Add QLogic FastLinQ offload iSCSI
>driver framework.")
>Reviewed-by: Johannes Thumshirn 
>Signed-off-by: Arnd Bergmann 
>---
>v2: don't rename references to do_not_recover in string literals
>---
> drivers/scsi/qedi/qedi_debugfs.c | 16 
> drivers/scsi/qedi/qedi_fw.c  |  4 ++--
> drivers/scsi/qedi/qedi_gbl.h |  8 +++-
> drivers/scsi/qedi/qedi_iscsi.c   |  8 
> 4 files changed, 21 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/scsi/qedi/qedi_debugfs.c
>b/drivers/scsi/qedi/qedi_debugfs.c
>index 955936274241..59417199bf36 100644
>--- a/drivers/scsi/qedi/qedi_debugfs.c
>+++ b/drivers/scsi/qedi/qedi_debugfs.c
>@@ -14,7 +14,7 @@
> #include 
> #include 
> 
>-int do_not_recover;
>+int qedi_do_not_recover;
> static struct dentry *qedi_dbg_root;
> 
> void
>@@ -74,22 +74,22 @@ qedi_dbg_exit(void)
> static ssize_t
> qedi_dbg_do_not_recover_enable(struct qedi_dbg_ctx *qedi_dbg)
> {
>-  if (!do_not_recover)
>-  do_not_recover = 1;
>+  if (!qedi_do_not_recover)
>+  qedi_do_not_recover = 1;
> 
>   QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
>-do_not_recover);
>+qedi_do_not_recover);
>   return 0;
> }
> 
> static ssize_t
> qedi_dbg_do_not_recover_disable(struct qedi_dbg_ctx *qedi_dbg)
> {
>-  if (do_not_recover)
>-  do_not_recover = 0;
>+  if (qedi_do_not_recover)
>+  qedi_do_not_recover = 0;
> 
>   QEDI_INFO(qedi_dbg, QEDI_LOG_DEBUGFS, "do_not_recover=%d\n",
>-do_not_recover);
>+qedi_do_not_recover);
>   return 0;
> }
> 
>@@ -141,7 +141,7 @@ qedi_dbg_do_not_recover_cmd_read(struct file *filp,
>char __user *buffer,
>   if (*ppos)
>   return 0;
> 
>-  cnt = sprintf(buffer, "do_not_recover=%d\n", do_not_recover);
>+  cnt = sprintf(buffer, "do_not_recover=%d\n", qedi_do_not_recover);
>   cnt = min_t(int, count, cnt - *ppos);
>   *ppos += cnt;
>   return cnt;
>diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c
>index c9f0ef4e11b3..2bce3efc66a4 100644
>--- a/drivers/scsi/qedi/qedi_fw.c
>+++ b/drivers/scsi/qedi/qedi_fw.c
>@@ -1461,9 +1461,9 @@ static void qedi_tmf_work(struct work_struct *work)
> get_itt(tmf_hdr->rtt), get_itt(ctask->itt), cmd->task_id,
> qedi_conn->iscsi_conn_id);
> 
>-  if (do_not_recover) {
>+  if (qedi_do_not_recover) {
>   QEDI_ERR(>dbg_ctx, "DONT SEND CLEANUP/ABORT %d\n",
>-   do_not_recover);
>+   qedi_do_not_recover);
>   goto abort_ret;
>   }
> 
>diff --git a/drivers/scsi/qedi/qedi_gbl.h b/drivers/scsi/qedi/qedi_gbl.h
>index 8e488de88ece..63d793f46064 100644
>--- a/drivers/scsi/qedi/qedi_gbl.h
>+++ b/drivers/scsi/qedi/qedi_gbl.h
>@@ -12,8 +12,14 @@
> 
> #include "qedi_iscsi.h"
> 
>+#ifdef CONFIG_DEBUG_FS
>+extern int qedi_do_not_recover;
>+#else
>+#define qedi_do_not_recover (0)
>+#endif
>+
> extern uint qedi_io_tracing;
>-extern int do_not_recover;
>+
> extern struct scsi_host_template qedi_host_template;
> extern struct iscsi_transport qedi_iscsi_transport;
> extern const struct qed_iscsi_ops *qedi_ops;
>diff --git a/drivers/scsi/qedi/qedi_iscsi.c
>b/drivers/scsi/qedi/qedi_iscsi.c
>index b9f79d36142d..4cc474364c50 100644
>--- a/drivers/scsi/qedi/qedi_iscsi.c
>+++ b/drivers/scsi/qedi/qedi_iscsi.c
>@@ -833,7 +833,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct
>sockaddr *dst_addr,
>   return ERR_PTR(ret);
>   }
> 
>-  if (do_not_recover) {
>+  if (qedi_do_not_recover) {
>   ret = -ENOMEM;
>   return ERR_PTR(ret);
>   }
>@@ -957,7 +957,7 @@ static int qedi_ep_poll(struct iscsi_endpoint *ep,
>int timeout_ms)
>   struct qedi_endpoint *qedi_ep;
>   int ret = 0;
> 
>-