Clearing ch->device in ch_release() is wrong because that pointer must
remain valid until ch_remove() is called. This patch fixes the following
crash the second time a ch device is opened:
BUG: kernel NULL pointer dereference, address: 0000000000000790
RIP: 0010:scsi_device_get+0x5/0x60
Call Trace:
ch_open+0x4c/0xa0 [ch]
chrdev_open+0xa2/0x1c0
do_dentry_open+0x13a/0x380
path_openat+0x591/0x1470
do_filp_open+0x91/0x100
do_sys_open+0x184/0x220
do_syscall_64+0x5f/0x1a0
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Fixes: 085e56766f ("scsi: ch: add refcounting")
Cc: Hannes Reinecke <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Reported-by: Rob Turk <[email protected]>
Suggested-by: Rob Turk <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch fixes the lock inversion complaint:
============================================
WARNING: possible recursive locking detected
5.3.0-rc7-dbg+ #1 Not tainted
--------------------------------------------
kworker/u16:6/171 is trying to acquire lock:
00000000035c6e6c (&id_priv->handler_mutex){+.+.}, at: rdma_destroy_id+0x78/0x4a0 [rdma_cm]
but task is already holding lock:
00000000bc7c307d (&id_priv->handler_mutex){+.+.}, at: iw_conn_req_handler+0x151/0x680 [rdma_cm]
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&id_priv->handler_mutex);
lock(&id_priv->handler_mutex);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by kworker/u16:6/171:
#0: 00000000e2eaa773 ((wq_completion)iw_cm_wq){+.+.}, at: process_one_work+0x472/0xac0
#1: 000000001efd357b ((work_completion)(&work->work)#3){+.+.}, at: process_one_work+0x476/0xac0
#2: 00000000bc7c307d (&id_priv->handler_mutex){+.+.}, at: iw_conn_req_handler+0x151/0x680 [rdma_cm]
stack backtrace:
CPU: 3 PID: 171 Comm: kworker/u16:6 Not tainted 5.3.0-rc7-dbg+ #1
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
Workqueue: iw_cm_wq cm_work_handler [iw_cm]
Call Trace:
dump_stack+0x8a/0xd6
__lock_acquire.cold+0xe1/0x24d
lock_acquire+0x106/0x240
__mutex_lock+0x12e/0xcb0
mutex_lock_nested+0x1f/0x30
rdma_destroy_id+0x78/0x4a0 [rdma_cm]
iw_conn_req_handler+0x5c9/0x680 [rdma_cm]
cm_work_handler+0xe62/0x1100 [iw_cm]
process_one_work+0x56d/0xac0
worker_thread+0x7a/0x5d0
kthread+0x1bc/0x210
ret_from_fork+0x24/0x30
This is not a bug as there are actually two lock classes here.
Link: https://lore.kernel.org/r/[email protected]
Fixes: de910bd921 ("RDMA/cma: Simplify locking needed for serialization of callbacks")
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Jason Gunthorpe <[email protected]>
According to the firmware documentation a status type 0 IOCB can be
followed by one or more status continuation type 0 IOCBs. Hence do not
complain if the completion function is not called from inside the status
type 0 IOCB handler.
WARNING: CPU: 10 PID: 425 at drivers/scsi/qla2xxx/qla_isr.c:2784
qla2x00_status_entry.isra.7+0x484/0x17b0 [qla2xxx]
CPU: 10 PID: 425 Comm: kworker/10:1 Tainted: G E 5.3.0-rc4-next-20190813-autotest-autotest #1
Workqueue: qla2xxx_wq qla25xx_free_rsp_que [qla2xxx]
Call Trace:
qla2x00_status_entry.isra.7+0x1484/0x17b0 [qla2xxx] (unreliable)
qla24xx_process_response_queue+0x7d8/0xbd0 [qla2xxx]
qla25xx_free_rsp_que+0x1a0/0x220 [qla2xxx]
process_one_work+0x25c/0x520
worker_thread+0x8c/0x5e0
kthread+0x154/0x1a0
ret_from_kernel_thread+0x5c/0x7c
Cc: Himanshu Madhani <[email protected]>
Cc: Abdul Haleem <[email protected]>
Reported-by: Abdul Haleem <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Abdul Haleem <[email protected]>
Reviewed-by: Lee Duncan <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch fixes the following Coverity complaint:
Null-checking sp->u.iocb_cmd.u.ctarg.rsp suggests that it may be null, but
it has already been dereferenced on all paths leading to the check.
See also commit e374f9f592 ("scsi: qla2xxx: Migrate switch registration commands away from mailbox interface") # v4.16.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Instead of calling qla2x00_fcport_event_handler() and letting the switch
statement inside that function decide which other function to call, call
the latter function directly. Remove the event member from the event_arg
structure because it is no longer needed. Remove the
qla_handle_els_plogi_done() function because it is never called.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
The C language supports implicit casting of void pointers to non-void
pointers. Remove explicit sts_entry_* casts that are not necessary.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Not calling sp->done() from the command completion path is a severe bug.
Hence complain loudly if that happens.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
The LIO core requires that the target driver callback functions
.queue_data_in() and .queue_status() call target_put_sess_cmd() or
transport_generic_free_cmd(). These calls may happen synchronously or
asynchronously. Make sure that one of these LIO functions is called in case
a command has been aborted. This patch avoids that the code for removing a
session hangs due to commands that do not make progress.
Cc: Himanshu Madhani <[email protected]>
Fixes: 694833ee00 ("scsi: tcm_qla2xxx: Do not allow aborted cmd to advance.") # v4.13.
Fixes: a07100e00a ("qla2xxx: Fix TMR ABORT interaction issue between qla2xxx and TCM") # v4.5.
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since struct sg_table is used in nvme-fc-driver.h, include
<linux/scatterlist.h> from that header file.
Since no definitions or declarations from <linux/blk-mq.h> are used in the
qla_nvme.h header file, do not include <linux/blk-mq.h> from that header
file.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Instead of allocating a struct srb dynamically from inside .queuecommand(),
set qla2xxx_driver_template.cmd_size such that struct scsi_cmnd and struct
srb are contiguous. Do not call QLA_QPAIR_MARK_BUSY() /
QLA_QPAIR_MARK_NOT_BUSY() for SRBs associated with SCSI commands. That is
safe because scsi_remove_host() is called before queue pairs are deleted
and scsi_remove_host() waits for all outstanding SCSI commands to finish.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since all pointers passed to the srb_t.done() and srb_t.free() functions
have type srb_t, change the type of the first argument of these functions
from void * into struct srb *. This allows the compiler to verify the
argument types for these functions. This patch does not change any
functionality.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Use the same approach for encoding the destination ID as the approach used
by qlt_update_vp_map().
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
According to the firmware documentation responder mode must be set for ELS
pass-through IOCBs if a response is expected.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
According to the firmware documentation the firmware expects all ELS
pass-through IOCB parameters in little endian format. Make this explicit.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This change makes it easier to detect qla24xx_read_flash_data() failures
and also to handle such failures. This change does not modify the behavior
of the driver since all callers ignore the qla24xx_read_flash_data()
return value.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Introduce the be_id_t and le_id_t data types for Fibre Channel source and
destination ID formats supported by the firmware instead of using an
uint8_t[3] array. Introduce functions for converting from and to the
port_id_t data types. This patch does not change the behavior of the
qla2xxx driver but improves source code readability and also allows the
compiler to verify the endianness of Fibre Channel IDs.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Pass the output buffer size to the code that generates a PCI info string
and check the output buffer size while generating a PCI info string.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Such a timeout is a severe issue. Hence complain if waiting for pending
commands times out. This patch fixes a small bug: it modifies
qla82xx_chip_reset_cleanup() such that the "Done waiting" message is
reported if qla82xx_chip_reset_cleanup() succeeded instead of if that
function failed.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Make it clear to humans and also to the compiler that the string passed
as fourth argument is not modified.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch fixes the following Coverity complaint:
Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression dma_eng_num *
65536U with type unsigned int (32 bits, unsigned) is evaluated using 32-bit
arithmetic, and then used in a context that expects an expression of type
uint64_t (64 bits, unsigned).
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Set the r??_data_len variables before using these instead of after.
This patch fixes the following Coverity complaint:
const: At condition req_data_len != rsp_data_len, the value of req_data_len
must be equal to 0.
const: At condition req_data_len != rsp_data_len, the value of rsp_data_len
must be equal to 0.
dead_error_condition: The condition req_data_len != rsp_data_len cannot be
true.
Cc: Himanshu Madhani <[email protected]>
Fixes: a9b6f722f6 ("[SCSI] qla2xxx: Implementation of bidirectional.") # v3.7.
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Read the FC port state once instead of twice. This patch fixes the
following Coverity complaint:
Unchecked return value (CHECKED_RETURN)
check_return: Calling atomic_read without checking return value (as is
done elsewhere 80 out of 92 times).
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since sess == NULL before 'goto out_term2' is executed, the code under 'if
(sess)' cannot be reached. Hence remove that code. This was detected by
Coverity.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch fixes the following Coverity complaint:
Unchecked return value (CHECKED_RETURN)
check_return: Calling wait_for_completion_timeout without checking return
value (as is done elsewhere 14 out of 17 times).
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Delete the PLOGIN ACK data structure from the vha->plogi_ack_list before
freeing that data structure to avoid that that list gets corrupted.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
It is helpful when debugging this driver to have the firmware status code
available if a mailbox command fails. Hence report that firmware status
code.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since qlt_remove_target() only calls qlt_release() if
vha->vha_tgt.qla_tgt != NULL, checking that pointer inside qlt_release()
is not necessary. This patch fixes the following Coverity complaint:
CID 188348 (#1 of 1): Dereference after null check (FORWARD_NULL)
var_deref_model: Passing null pointer &vha->vha_tgt.qla_tgt->tgt_list_entry
to list_del, which dereferences it.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Simplify the implementation of this function by using the %phC format
specifier instead of using explicit for-loops.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch makes the code that parses the GID list easier to read without
changing the behavior of the code.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch does not change any functionality but fixes a Coverity complaint
about using a scalar as an array.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
The value returned by this function is not used. Hence change the return
type of this function into 'void' and remove the return statement.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This patch makes it clear to humans and also to the compiler that
ql_dump_buffer() does not modify the memory the @buf argument points at.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since qlt_make_local_sess() is defined before it is called, remove the
forward declaration of that function.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
There is no bsg code in the qla_mr.c source file. Hence do not include
the <linux/bsg-lib.h> header file from qla_mr.c.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since the put_unaligned_*() macros are used in this header file, include
the header file that defines these macros.
Cc: Himanshu Madhani <[email protected]>
Fixes: 15b7a68c1d ("scsi: qla2xxx: Introduce the dsd32 and dsd64 data structures") # v5.2-rc1.
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Insert a space where required, surround complex expressions in macros with
parentheses, use the UL suffix instead of the (unsigned long) cast, do not
use line continuations when not necessary and do not explicitly initialize
static variables to zero.
Cc: Himanshu Madhani <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
I'm not sure how this happened but the patch that was intended to fix abort
handling was incomplete. This patch fixes that patch as follows:
- If aborting the SCSI command failed, wait until the SCSI command
completes.
- Return SUCCESS instead of FAILED if an abort attempt races with SCSI
command completion.
- Since qla2xxx_eh_abort() increments the sp reference count by calling
sp_get(), decrement the sp reference count before returning.
Cc: Himanshu Madhani <[email protected]>
Fixes: 219d27d714 ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands")
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Since qla2x00_abort_srb() starts with increasing the reference count of
@sp, decrease that same reference count before returning.
Cc: Himanshu Madhani <[email protected]>
Fixes: 219d27d714 ("scsi: qla2xxx: Fix race conditions in the code for aborting SCSI commands") # v5.2.
Signed-off-by: Bart Van Assche <[email protected]>
Tested-by: Himanshu Madhani <[email protected]>
Reviewed-by: Himanshu Madhani <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
The only 'new_state' values passed by upstream kernel code to
scsi_internal_device_unblock_nowait() are SDEV_RUNNING and
SDEV_TRANSPORT_OFFLINE. These are the only values that should be passed to
this function. Hence check the value of the 'new_state' argument to avoid
that scsi_internal_device_unblock_nowait() would be used to trigger an
illegal SCSI device state transition. In this context 'illegal' means not
allowed by scsi_device_set_state().
Cc: Christoph Hellwig <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: Johannes Thumshirn <[email protected]>
Cc: Ming Lei <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Consider the following example:
* The logical block size is 4 KB.
* The physical block size is 8 KB.
* max_sectors equals (16 KB >> 9) sectors.
* A non-aligned 4 KB and an aligned 64 KB bio are merged into a single
non-aligned 68 KB bio.
The current behavior is to split such a bio into (16 KB + 16 KB + 16 KB
+ 16 KB + 4 KB). The start of none of these five bio's is aligned to a
physical block boundary.
This patch ensures that such a bio is split into four aligned and
one non-aligned bio instead of being split into five non-aligned bios.
This improves performance because most block devices can handle aligned
requests faster than non-aligned requests.
Since the physical block size is larger than or equal to the logical
block size, this patch preserves the guarantee that the returned
value is a multiple of the logical block size.
Cc: Christoph Hellwig <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Move the max_sectors check into bvec_split_segs() such that a single
call to that function can do all the necessary checks. This patch
optimizes the fast path further, namely if a bvec fits in a page.
Cc: Christoph Hellwig <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Simplify this function by by removing two if-tests. Other than requiring
that the @sectors pointer is not NULL, this patch does not change the
behavior of bvec_split_segs().
Reviewed-by: Johannes Thumshirn <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Make it clear to the compiler and also to humans that the functions
that query request queue properties do not modify any member of the
request_queue data structure.
Reviewed-by: Johannes Thumshirn <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Although commit 669de8bda8 ("kernel/workqueue: Use dynamic lockdep keys
for workqueues") unregisters dynamic lockdep keys when a workqueue is
destroyed, a side effect of that commit is that all stack traces
associated with the lockdep key are leaked when a workqueue is destroyed.
Fix this by storing each unique stack trace once. Other changes in this
patch are:
- Use NULL instead of { .nr_entries = 0 } to represent 'no trace'.
- Store a pointer to a stack trace in struct lock_class and struct
lock_list instead of storing 'nr_entries' and 'offset'.
This patch avoids that the following program triggers the "BUG:
MAX_STACK_TRACE_ENTRIES too low!" complaint:
#include <fcntl.h>
#include <unistd.h>
int main()
{
for (;;) {
int fd = open("/dev/infiniband/rdma_cm", O_RDWR);
close(fd);
}
}
Suggested-by: Peter Zijlstra <[email protected]>
Reported-by: Eric Biggers <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Waiman Long <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Yuyang Du <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
>From the NVMe 1.4 spec:
NSFEAT bit 4 if set to 1: indicates that the fields NPWG, NPWA, NPDG, NPDA,
and NOWS are defined for this namespace and should be used by the host for
I/O optimization;
[ ... ]
Namespace Preferred Write Granularity (NPWG): This field indicates the
smallest recommended write granularity in logical blocks for this namespace.
This is a 0's based value. The size indicated should be less than or equal
to Maximum Data Transfer Size (MDTS) that is specified in units of minimum
memory page size. The value of this field may change if the namespace is
reformatted. The size should be a multiple of Namespace Preferred Write
Alignment (NPWA). Refer to section 8.25 for how this field is utilized to
improve performance and endurance.
[ ... ]
Each Write, Write Uncorrectable, or Write Zeroes commands should address a
multiple of Namespace Preferred Write Granularity (NPWG) (refer to Figure
245) and Stream Write Size (SWS) (refer to Figure 515) logical blocks (as
expressed in the NLB field), and the SLBA field of the command should be
aligned to Namespace Preferred Write Alignment (NPWA) (refer to Figure 245)
for best performance.
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Several new fields have been introduced in version 1.4 of the NVMe spec
at offsets that were defined as reserved in version 1.3d of the NVMe
spec. Update the definition of the nvme_id_ns data structure such that
it is in sync with version 1.4 of the NVMe spec. This change preserves
backwards compatibility.
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Keith Busch <[email protected]>
Reviewed-by: Chaitanya Kulkarni <[email protected]>
Reviewed-by: Martin K. Petersen <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Make the NVMe NAWUN, NAWUPF, NACWU, NPWG, NPWA, NPDG and NOWS attributes
available to initator systems for the block backend.
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Martin K. Petersen <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
No code that occurs between blk_mq_get_ctx() and blk_mq_put_ctx() depends
on preemption being disabled for its correctness. Since removing the CPU
preemption calls does not measurably affect performance, simplify the
blk-mq code by removing the blk_mq_put_ctx() function and also by not
disabling preemption in blk_mq_get_ctx().
Cc: Hannes Reinecke <[email protected]>
Cc: Omar Sandoval <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Ming Lei <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Several block layer users who are not kernel developers do not know that
the word 'segment' refers to an element in a DMA scatter/gather list. Make
the block layer documentation easier to understand by stating explicitly
what the word 'segment' stands for.
Reviewed-by: Martin K. Petersen <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Commit f9824952ee ("block: update sysfs documentation") # v5.0 broke the
alphabetical order of the sysfs attribute names. List queue sysfs attribute
names alphabetically.
Cc: Damien Le Moal <[email protected]>
Reviewed-by: Martin K. Petersen <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Fix the spelling of the wbt_lat_usec sysfs attribute.
Fixes: 87760e5eef ("block: hook up writeback throttling") # v4.10.
Reviewed-by: Martin K. Petersen <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
The previous patch guarantees that srp_queuecommand() does not get
invoked while reconnecting occurs. Hence remove the code from
srp_queuecommand() that prevents command queueing while reconnecting.
This patch avoids that the following can appear in the kernel log:
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 0, pid: 5600, name: scsi_eh_9
1 lock held by scsi_eh_9/5600:
#0: (rcu_read_lock){....}, at: [<00000000cbb798c7>] __blk_mq_run_hw_queue+0xf1/0x1e0
Preemption disabled at:
[<00000000139badf2>] __blk_mq_delay_run_hw_queue+0x78/0xf0
CPU: 9 PID: 5600 Comm: scsi_eh_9 Tainted: G W 4.15.0-rc4-dbg+ #1
Hardware name: Dell Inc. PowerEdge R720/0VWT90, BIOS 2.5.4 01/22/2016
Call Trace:
dump_stack+0x67/0x99
___might_sleep+0x16a/0x250 [ib_srp]
__mutex_lock+0x46/0x9d0
srp_queuecommand+0x356/0x420 [ib_srp]
scsi_dispatch_cmd+0xf6/0x3f0
scsi_queue_rq+0x4a8/0x5f0
blk_mq_dispatch_rq_list+0x73/0x440
blk_mq_sched_dispatch_requests+0x109/0x1a0
__blk_mq_run_hw_queue+0x131/0x1e0
__blk_mq_delay_run_hw_queue+0x9a/0xf0
blk_mq_run_hw_queue+0xc0/0x1e0
blk_mq_start_hw_queues+0x2c/0x40
scsi_run_queue+0x18e/0x2d0
scsi_run_host_queues+0x22/0x40
scsi_error_handler+0x18d/0x5f0
kthread+0x11c/0x140
ret_from_fork+0x24/0x30
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Laurence Oberman <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
Several SCSI transport and LLD drivers surround code that does not
tolerate concurrent calls of .queuecommand() with scsi_target_block() /
scsi_target_unblock(). These last two functions use
blk_mq_quiesce_queue() / blk_mq_unquiesce_queue() for scsi-mq request
queues to prevent concurrent .queuecommand() calls. However, that is
not sufficient to prevent .queuecommand() calls from scsi_send_eh_cmnd().
Hence surround the .queuecommand() call from the SCSI error handler with
code that avoids that .queuecommand() gets called in the blocked state.
Note: converting the .queuecommand() call in scsi_send_eh_cmnd() into
code that calls blk_get_request() + blk_execute_rq() is not an option
since scsi_send_eh_cmnd() must be able to make forward progress even
if all requests have been allocated.
Cc: Ming Lei <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: Johannes Thumshirn <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
The ability to modify the SCSI device state was introduced by commit
638127e579a4 ("[PATCH] Fix error handler offline behaviour"; v2.6.12). That
same commit introduced the following device states:
{ SDEV_CREATED, "created" },
{ SDEV_RUNNING, "running" },
{ SDEV_CANCEL, "cancel" },
{ SDEV_DEL, "deleted" },
{ SDEV_QUIESCE, "quiesce" },
{ SDEV_OFFLINE, "offline" },
The SDEV_BLOCK state was introduced later to avoid that an FC cable pull
would immediately result in an I/O error (commit 1094e682310e; "[PATCH]
suspending I/Os to a device"; v2.6.12). That same patch introduced the
ability to set the SDEV_BLOCK state from user space. I'm not sure whether
that ability was introduced on purpose or accidentally.
Since there is agreement that only writing "running" or "offline" into
the SCSI sysfs device state attribute makes sense, restrict sysfs writes
to these values.
This patch makes sure that SDEV_BLOCK is only used for its original
purpose, namely to allow transport drivers and LLDs to block further
.queuecommand() calls while transport layer or adapter recovery is in
progress.
Note: a web search for "/sys/class/scsi_device" AND "device/state"
revealed several storage configuration guides. The instructions I found
in these guides tell users to write the value "running" or "offline" in
the SCSI device state sysfs attribute and no other values.
[mkp: typo]
Cc: Christoph Hellwig <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: Johannes Thumshirn <[email protected]>
Cc: James Smart <[email protected]>
Cc: Ewan D. Milne <[email protected]>
Cc: Laurence Oberman <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
As explained during the 2018 LSF/MM session about increasing SCSI disk
probing concurrency, the problems with the current probing approach are as
follows:
- The driver core is unaware of asynchronous SCSI LUN probing.
wait_for_device_probe() waits for all asynchronous probes except
asynchronous SCSI disk probes.
- There is unnecessary serialization between sd_probe() and sd_remove().
This can lead to a deadlock.
Hence this patch that modifies the sd driver such that it uses the driver
core framework for asynchronous probing. The async domain and
get_device()/put_device() pairs that became superfluous due to this change
are removed.
This patch does not affect the time needed for loading the scsi_debug
kernel module with parameters delay=0 and max_luns=256.
This patch depends on commit ef0ff68351 ("driver core: Probe devices
asynchronously instead of the driver") that went upstream in kernel version
v5.1-rc1.
Cc: Lee Duncan <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: Johannes Thumshirn <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Dan Williams <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
When debugging device driver power management code it is convenient to
know how much time is spent in the "suspend start" and "suspend end"
phases. Hence log the time spent in these phases.
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
The function srp_parse_in() is used both for parsing source address
specifications and for target address specifications. Target addresses
must have a port number. Having to specify a port number for source
addresses is inconvenient. Make sure that srp_parse_in() supports again
parsing addresses with no port number.
Cc: <[email protected]>
Fixes: c62adb7def ("IB/srp: Fix IPv6 address parsing")
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jason Gunthorpe <[email protected]>
Document the meaning of the blk_mq_hw_queue_to_node() arguments.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Change one occurrence of 'performace' into 'performance'.
Cc: Max Gurtovoy <[email protected]>
Fixes: fe631457ff ("blk-mq: map all HWQ also in hyperthreaded system") # v4.13.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
This patch avoids that the kernel-doc script complains about these
function headers when building with W=1.
Cc: Hannes Reinecke <[email protected]>
Cc: Keith Busch <[email protected]>
Fixes: ed76e329d7 ("blk-mq: abstract out queue map") # v5.0.
Fixes: e42b3867de ("blk-mq-rdma: pass in queue map to blk_mq_rdma_map_queues") # v5.0.
Reviewed-by: Chaitanya Kulkarni <[email protected]>
Signed-off-by: Bart Van Assche <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
Bart Van Assche