The expression start_idx - dbg_cnt is evaluated using unsigned int
arthithmetic (since these variables are unsigned ints) and hence can never
be less than zero, so the less than comparison is never true. Rewrite the
expression to check for start_idx being less than dbg_cnt.
After the logic was corrected, temp_idx wasn't working correctly. So fix it
as well.
Link: https://lore.kernel.org/r/20200706204246.130416-1-jsmart2021@gmail.com
Fixes: 372c187b8a ("scsi: lpfc: Add an internal trace log buffer")
CC: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Addresses-Coverity: ("Unsigned compared against 0")
With certain platforms its possible pci_alloc_irq_vectors() may affinitize
irq vectors to multiple (all?) CPUs. The driver is currently assuming
exclusivity and vectors being doled out to different CPUs and is assigning
primary ownership of each vector to the first CPU in the mask. The code
doesn't bother to check if the CPU already owns a vector and will
unconditionally overwrite the CPU to vector mapping. This causes the
relationships between eq's and cq's to get confused and gets worse when
CPUs start to offline. The net results are interrupts are skipped resulting
in mailbox timeouts and there are oopses in CPU offling flows.
Fix this changing up the primary vector assignment. Now assign the eq to a
CPU only if it is the CPU in the mask that does not have a prior
assignment. And once the primary ownership is assigned, break from the
loop. For CPUs that may have been set before but not the primary owner, the
lpfc_cpu_affinity_check() routine will balance the CPU to eq assignment.
Link: https://lore.kernel.org/r/20200706204230.130363-1-jsmart2021@gmail.com
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Introduce functions to manipulate UFS inline encryption hardware in line
with the JEDEC UFSHCI v2.1 specification and to work with the block keyslot
manager.
The UFS crypto API will assume by default that a vendor driver doesn't
support UFS crypto, even if the hardware advertises the capability, because
a lot of hardware requires some special handling that's not specified in
the aforementioned JEDEC spec. Each vendor driver must explicitly set
hba->caps |= UFSHCD_CAP_CRYPTO before ufshcd_hba_init_crypto_capabilities()
is called to opt-in to UFS crypto support.
Link: https://lore.kernel.org/r/20200706200414.2027450-3-satyat@google.com
Reviewed-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Fixes the following W=1 kernel build warning(s):
drivers/scsi/libfc/fc_disc.c:343: warning: Function parameter or member 'disc' not described in 'fc_disc_gpn_ft_req'
drivers/scsi/libfc/fc_disc.c:343: warning: Excess function parameter 'lport' description in 'fc_disc_gpn_ft_req'
drivers/scsi/libfc/fc_disc.c:380: warning: Function parameter or member 'disc' not described in 'fc_disc_gpn_ft_parse'
drivers/scsi/libfc/fc_disc.c:380: warning: Excess function parameter 'lport' description in 'fc_disc_gpn_ft_parse'
drivers/scsi/libfc/fc_disc.c:498: warning: Function parameter or member 'disc_arg' not described in 'fc_disc_gpn_ft_resp'
drivers/scsi/libfc/fc_disc.c:498: warning: Excess function parameter 'lp_arg' description in 'fc_disc_gpn_ft_resp'
Link: https://lore.kernel.org/r/20200707140055.2956235-10-lee.jones@linaro.org
Cc: Hannes Reinecke <hare@suse.de>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
There is no need to populate an unused variable, even if the read is
required.
Fixes the following W=1 kernel build warning(s):
drivers/scsi/pcmcia/nsp_cs.c: In function ‘nsp_cs_message’:
drivers/scsi/pcmcia/nsp_cs.c:143:2: warning: function ‘nsp_cs_message’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
drivers/scsi/pcmcia/nsp_cs.c: In function ‘nsp_fifo_count’:
drivers/scsi/pcmcia/nsp_cs.c:692:24: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
Link: https://lore.kernel.org/r/20200707140055.2956235-9-lee.jones@linaro.org
Cc: YOKOTA Hiroshi <yokota@netlab.is.tsukuba.ac.jp>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Fixes the following W=1 kernel build warning(s):
drivers/scsi/pcmcia/nsp_cs.c: In function ‘nsp_cs_message’:
drivers/scsi/pcmcia/nsp_cs.c:143:2: warning: function ‘nsp_cs_message’ might be a candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
drivers/scsi/pcmcia/nsp_cs.c: In function ‘nsp_fifo_count’:
drivers/scsi/pcmcia/nsp_cs.c:692:24: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
Link: https://lore.kernel.org/r/20200707140055.2956235-8-lee.jones@linaro.org
Cc: YOKOTA Hiroshi <yokota@netlab.is.tsukuba.ac.jp>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Not all source files which include 'fdomain.h' make use of 'fdomain_pm_ops'
leaving them defined but unused. Mark it as __maybe_unused to tell the
compiler this is not only acceptable, but expected.
Fixes the following W=1 kernel build warning(s):
In file included from drivers/scsi/pcmcia/fdomain_cs.c:16:
drivers/scsi/fdomain.h:106:32: warning: ‘fdomain_pm_ops’ defined but not used [-Wunused-const-variable=]
106 | static const struct dev_pm_ops fdomain_pm_ops;
| ^~~~~~~~~~~~~~
Link: https://lore.kernel.org/r/20200707140055.2956235-4-lee.jones@linaro.org
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
In _config_request(), the variable issue_reset is set using the macro
mpt3sas_check_cmd_timeout() but otherwise unused, causing a compiler
warning when compiling with W=1. Avoid this warning by removing this
variable, using the function mpt3sas_base_check_cmd_timeout() directly
instead of the mpt3sas_check_cmd_timeout() macro.
Link: https://lore.kernel.org/r/20200706123356.452135-1-damien.lemoal@wdc.com
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
zfcp_qdio_send() and zfcp_qdio_int_req() run concurrently, adding and
completing SBALs on the Request Queue. There's a theoretical race where
zfcp_qdio_int_req() completes a number of SBALs & increments the queue's
free-level _before_ zfcp_qdio_send() was able to decrement it.
This can cause ->req_q_free to momentarily hold a value larger than
QDIO_MAX_BUFFERS_PER_Q. Luckily zfcp_qdio_send() is always called under
->req_q_lock, and all readers of the free-level also take this lock. So we
can trust that zfcp_qdio_send() will clean up such a temporary overflow
before anyone can actually observe it.
But it's still confusing and annoying to worry about. So adjust the code to
avoid this race.
Link: https://lore.kernel.org/r/7f61f59a1f8db270312e64644f9173b8f1ac895f.1593780621.git.bblock@linux.ibm.com
Reviewed-by: Steffen Maier <maier@linux.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
IBM decided to retire a lot of the content that was previously hosted on
"developerworks", and so some of the links we've used for documentation are
now dead or redirect to some general landing page with no correlation to
what the links were meant to provide.
Change the provided link in the Kconfig file for zfcp to rather refer to
our device drivers book that we regularly update and publish for free, and
whose name hasn't been changed since it was first published.
Our hardware is also not called "IBM eServer zSeries" anymore - in fact, it
hasn't been called like that since 2006. Use a broader term that covers
different server names over time.
Lastly, add a short paragraph about how our HBAs are typically named, to
have some more tangible references.
Link: https://lore.kernel.org/r/96069b9f4c4f056a515b37e89b2bdfccc282e3d3.1593780621.git.bblock@linux.ibm.com
Reviewed-by: Julian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: Steffen Maier <maier@linux.ibm.com>
Signed-off-by: Benjamin Block <bblock@linux.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
SPC4 has:
The first ISCSI INITIATOR SESSION ID field byte containing an ASCII null
character terminates the ISCSI INITIATOR SESSION ID field without regard
for the specified length of the iSCSI TransportID or the contents of the
ADDITIONAL LENGTH field.
----------------------------------------
which sounds like we can get an iSID shorter than 12 chars. SPC and the
iSCSI RFC do not say how to handle that case other than just cutting off
the iSID. This patch just makes sure that if we get an iSID like that, we
only copy/send that string.
There is no OS that does this right now, so there was no test case. I did
test with sg utils to check it works as expected and nothing breaks.
Link: https://lore.kernel.org/r/1593654203-12442-8-git-send-email-michael.christie@oracle.com
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This fixes the following bugs with the transport id setup for iscsi:
1. Incorrectly adding NULL after initiator name for TPID format 1.
2. For TPID format 1 buffer setup we are doing off+len, off++ and then
also len+=some_value. This results in the isid going past buffer
boundaries when we then do buf[off+len]
3. The pr_reg_isid is the isid in string format which is 12 bytes, but we
are only copying 6 bytes.
Link: https://lore.kernel.org/r/1593654203-12442-6-git-send-email-michael.christie@oracle.com
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
__core_scsi3_add_registration clears the t10_pr_registration pr_reg_deve
and does a core_scsi3_lunacl_undepend_item which does an undepend and also
does a kref_put from the get done in __core_scsi3_alloc_registration. So
when we get to the bottom of core_scsi3_decode_spec_i_port the pr_reg_deve
is NULL and we crash when trying to access the local_pr_reg's pr_reg_deve.
We've also done an extra undepend for local_pr_reg and if we didn't crash
on the NULL we would have done an extra kref_put too.
This patch has us do a core_scsi3_lunacl_depend_item for local_pr_reg and
then let __core_scsi3_add_registration handle the cleanup for the
pr_reg_deve. We then just skip the undepend for the acl and tpg for the
local pr_reg.
The error path then works in a similar way, but we always do the
core_scsi3_lunacl_undepend_item since we never call
__core_scsi3_add_registration in that code path.
Link: https://lore.kernel.org/r/1593654203-12442-4-git-send-email-michael.christie@oracle.com
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Heavy testing indicates the irqsave() spinlock around the __set_bit() is
insufficient to stop following clear_bit() calls being rarely applied
out-of-order. Also the nearby failed kzalloc() path leading to
SCSI_MLQUEUE_HOST_BUSY does not properly undo the in_use bitmap and
num_in_q, fix.
Link: https://lore.kernel.org/r/20200702145355.522283-1-dgilbert@interlog.com
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
If we are doing async removal of the session, we could be doing a
scsi_remove_target from the removal workqueue, and for the offload case we
could be doing a new session addition and scan to the same host. The
add/scan might then end up trying to use the target_id of the target we are
removing.
This patch just has a delay the freeing of the target_id until after the
scsi_remove_target has completed, so we know it's no longer in use.
Link: https://lore.kernel.org/r/1593632868-6808-2-git-send-email-michael.christie@oracle.com
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The current logging methods typically end up requesting a reproduction with
a different logging level set to figure out what happened. This was mainly
by design to not clutter the kernel log messages with things that were
typically not interesting and the messages themselves could cause other
issues.
When looking to make a better system, it was seen that in many cases when
more data was wanted was when another message, usually at KERN_ERR level,
was logged. And in most cases, what the additional logging that was then
enabled was typically. Most of these areas fell into the discovery machine.
Based on this summary, the following design has been put in place: The
driver will maintain an internal log (256 elements of 256 bytes). The
"additional logging" messages that are usually enabled in a reproduction
will be changed to now log all the time to the internal log. A new logging
level is defined - LOG_TRACE_EVENT. When this level is set (it is not by
default) and a message marked as KERN_ERR is logged, all the messages in
the internal log will be dumped to the kernel log before the KERN_ERR
message is logged.
There is a timestamp on each message added to the internal log. However,
this timestamp is not converted to wall time when logged. The value of the
timestamp is solely to give a crude time reference for the messages.
Link: https://lore.kernel.org/r/20200630215001.70793-14-jsmart2021@gmail.com
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Although the existing implementation is very good at high I/O load, on
tests involving light load, especially on only a few hardware queues,
latency was a little higher than it can be due to using workqueue
scheduling. Other tasks in the system can delay handling.
Change the lower level to use irq_poll by default which uses a softirq for
I/O completion. This gives better latency as variance in when the cq is
processed is reduced over the workqueue interface. However, as high load is
better served by not being in softirq when the CPU is loaded, work queues
are still used under high I/O load.
Link: https://lore.kernel.org/r/20200630215001.70793-13-jsmart2021@gmail.com
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
