14 Jul, 2017
1 commit
-
Pull SCSI target updates from Nicholas Bellinger:
"It's been usually busy for summer, with most of the efforts centered
around TCMU developments and various target-core + fabric driver bug
fixing activities. Not particularly large in terms of LoC, but lots of
smaller patches from many different folks.The highlights include:
- ibmvscsis logical partition manager support (Michael Cyr + Bryant
Ly)- Convert target/iblock WRITE_SAME to blkdev_issue_zeroout (hch +
nab)- Add support for TMR percpu LUN reference counting (nab)
- Fix a potential deadlock between EXTENDED_COPY and iscsi shutdown
(Bart)- Fix COMPARE_AND_WRITE caw_sem leak during se_cmd quiesce (Jiang Yi)
- Fix TMCU module removal (Xiubo Li)
- Fix iser-target OOPs during login failure (Andrea Righi + Sagi)
- Breakup target-core free_device backend driver callback (mnc)
- Perform TCMU add/delete/reconfig synchronously (mnc)
- Fix TCMU multiple UIO open/close sequences (mnc)
- Fix TCMU CHECK_CONDITION sense handling (mnc)
- Fix target-core SAM_STAT_BUSY + TASK_SET_FULL handling (mnc + nab)
- Introduce TYPE_ZBC support in PSCSI (Damien Le Moal)
- Fix possible TCMU memory leak + OOPs when recalculating cmd base
size (Xiubo Li + Bryant Ly + Damien Le Moal + mnc)- Add login_keys_workaround attribute for non RFC initiators (Robert
LeBlanc + Arun Easi + nab)"* 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending: (68 commits)
iscsi-target: Add login_keys_workaround attribute for non RFC initiators
Revert "qla2xxx: Fix incorrect tcm_qla2xxx_free_cmd use during TMR ABORT"
tcmu: clean up the code and with one small fix
tcmu: Fix possbile memory leak / OOPs when recalculating cmd base size
target: export lio pgr/alua support as device attr
target: Fix return sense reason in target_scsi3_emulate_pr_out
target: Fix cmd size for PR-OUT in passthrough_parse_cdb
tcmu: Fix dev_config_store
target: pscsi: Introduce TYPE_ZBC support
target: Use macro for WRITE_VERIFY_32 operation codes
target: fix SAM_STAT_BUSY/TASK_SET_FULL handling
target: remove transport_complete
pscsi: finish cmd processing from pscsi_req_done
tcmu: fix sense handling during completion
target: add helper to copy sense to se_cmd buffer
target: do not require a transport_complete for SCF_TRANSPORT_TASK_SENSE
target: make device_mutex and device_list static
tcmu: Fix flushing cmd entry dcache page
tcmu: fix multiple uio open/close sequences
tcmu: drop configured check in destroy
...
13 Jul, 2017
1 commit
-
__GFP_REPEAT was designed to allow retry-but-eventually-fail semantic to
the page allocator. This has been true but only for allocations
requests larger than PAGE_ALLOC_COSTLY_ORDER. It has been always
ignored for smaller sizes. This is a bit unfortunate because there is
no way to express the same semantic for those requests and they are
considered too important to fail so they might end up looping in the
page allocator for ever, similarly to GFP_NOFAIL requests.Now that the whole tree has been cleaned up and accidental or misled
usage of __GFP_REPEAT flag has been removed for !costly requests we can
give the original flag a better name and more importantly a more useful
semantic. Let's rename it to __GFP_RETRY_MAYFAIL which tells the user
that the allocator would try really hard but there is no promise of a
success. This will work independent of the order and overrides the
default allocator behavior. Page allocator users have several levels of
guarantee vs. cost options (take GFP_KERNEL as an example)- GFP_KERNEL & ~__GFP_RECLAIM - optimistic allocation without _any_
attempt to free memory at all. The most light weight mode which even
doesn't kick the background reclaim. Should be used carefully because
it might deplete the memory and the next user might hit the more
aggressive reclaim- GFP_KERNEL & ~__GFP_DIRECT_RECLAIM (or GFP_NOWAIT)- optimistic
allocation without any attempt to free memory from the current
context but can wake kswapd to reclaim memory if the zone is below
the low watermark. Can be used from either atomic contexts or when
the request is a performance optimization and there is another
fallback for a slow path.- (GFP_KERNEL|__GFP_HIGH) & ~__GFP_DIRECT_RECLAIM (aka GFP_ATOMIC) -
non sleeping allocation with an expensive fallback so it can access
some portion of memory reserves. Usually used from interrupt/bh
context with an expensive slow path fallback.- GFP_KERNEL - both background and direct reclaim are allowed and the
_default_ page allocator behavior is used. That means that !costly
allocation requests are basically nofail but there is no guarantee of
that behavior so failures have to be checked properly by callers
(e.g. OOM killer victim is allowed to fail currently).- GFP_KERNEL | __GFP_NORETRY - overrides the default allocator behavior
and all allocation requests fail early rather than cause disruptive
reclaim (one round of reclaim in this implementation). The OOM killer
is not invoked.- GFP_KERNEL | __GFP_RETRY_MAYFAIL - overrides the default allocator
behavior and all allocation requests try really hard. The request
will fail if the reclaim cannot make any progress. The OOM killer
won't be triggered.- GFP_KERNEL | __GFP_NOFAIL - overrides the default allocator behavior
and all allocation requests will loop endlessly until they succeed.
This might be really dangerous especially for larger orders.Existing users of __GFP_REPEAT are changed to __GFP_RETRY_MAYFAIL
because they already had their semantic. No new users are added.
__alloc_pages_slowpath is changed to bail out for __GFP_RETRY_MAYFAIL if
there is no progress and we have already passed the OOM point.This means that all the reclaim opportunities have been exhausted except
the most disruptive one (the OOM killer) and a user defined fallback
behavior is more sensible than keep retrying in the page allocator.[akpm@linux-foundation.org: fix arch/sparc/kernel/mdesc.c]
[mhocko@suse.com: semantic fix]
Link: http://lkml.kernel.org/r/20170626123847.GM11534@dhcp22.suse.cz
[mhocko@kernel.org: address other thing spotted by Vlastimil]
Link: http://lkml.kernel.org/r/20170626124233.GN11534@dhcp22.suse.cz
Link: http://lkml.kernel.org/r/20170623085345.11304-3-mhocko@kernel.org
Signed-off-by: Michal Hocko
Acked-by: Vlastimil Babka
Cc: Alex Belits
Cc: Chris Wilson
Cc: Christoph Hellwig
Cc: Darrick J. Wong
Cc: David Daney
Cc: Johannes Weiner
Cc: Mel Gorman
Cc: NeilBrown
Cc: Ralf Baechle
Signed-off-by: Andrew Morton
Signed-off-by: Linus Torvalds
07 Jul, 2017
9 commits
-
If the scsi status was not SAM_STAT_GOOD or there was no transport
sense, we would ignore the scsi status and do a generic not ready
LUN communication failure check condition failure.The problem is that LUN COMM failure is treated as a hard error
sometimes and will cause apps to get IO errors instead of the OS's SCSI
layer retrying. For example, the tcmu daemon will return SAM_STAT_QUEUE_FULL
when memory runs low and can still make progress but wants the initiator to
reduce the work load. Windows will fail this error directly the app
instead of retrying.This patch is based on Nick's "target/iblock: Use -EAGAIN/-ENOMEM to
propigate SAM BUSY/TASK_SET_FULL" patch here:http://comments.gmane.org/gmane.linux.scsi.target.devel/11301
but instead of only setting SAM_STAT_GOOD, SAM_STAT_TASK_SET_FULL
and SAM_STAT_BUSY as success, it sets all non check condition status
as success so they are passed back to the initiator, so passthrough
type backends can return all SCSI status codes. Since only passthrough
uses this, I was not sure if we wanted to add checks for non-passthrough
and specific codes.Signed-off-by: Mike Christie
Signed-off-by: Nicholas Bellinger -
transport_complete is no longer used, so drop the code.
Signed-off-by: Mike Christie
Signed-off-by: Nicholas Bellinger -
This adds a helper to copy sense from backend module buffer to
the se_cmd's sense buffer.Signed-off-by: Mike Christie
Signed-off-by: Nicholas Bellinger -
tcmu needs to pass raw sense to target_complete_cmd, but a a
transport_complete callout is akward to implement for it.This moves the check for SCF_TRANSPORT_TASK_SENSE so any backend
can pass sense.Signed-off-by: Mike Christie
Signed-off-by: Nicholas Bellinger -
This patch addresses a COMPARE_AND_WRITE se_device->caw_sem leak,
that would be triggered during normal se_cmd shutdown or abort
via __transport_wait_for_tasks().This would occur because target_complete_cmd() would catch this
early and do complete_all(&cmd->t_transport_stop_comp), but since
target_complete_ok_work() or target_complete_failure_work() are
never called to invoke se_cmd->transport_complete_callback(),
the COMPARE_AND_WRITE specific callbacks never release caw_sem.To address this special case, go ahead and release caw_sem
directly from target_complete_cmd().(Remove '&& success' from check, to release caw_sem regardless
of scsi_status - nab)Signed-off-by: Jiang Yi
Cc: # 3.14+
Signed-off-by: Nicholas Bellinger -
Introduce target_show_cmd() and use it where appropriate. If
transport_wait_for_tasks() takes too long, make it show the
state of the command it is waiting for.(Add missing brackets around multi-line conditions - nab)
Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
Cc: Christoph Hellwig
Cc: Andy Grover
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
Avoid that aborting a command before it has been submitted onto
a workqueue triggers the following warning:INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 3 PID: 46 Comm: kworker/u8:1 Not tainted 4.12.0-rc2-dbg+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
Workqueue: tmr-iblock target_tmr_work [target_core_mod]
Call Trace:
dump_stack+0x86/0xcf
register_lock_class+0xe8/0x570
__lock_acquire+0xa1/0x11d0
lock_acquire+0x59/0x80
flush_work+0x42/0x2b0
__cancel_work_timer+0x10c/0x180
cancel_work_sync+0xb/0x10
core_tmr_lun_reset+0x352/0x740 [target_core_mod]
target_tmr_work+0xd6/0x130 [target_core_mod]
process_one_work+0x1ca/0x3f0
worker_thread+0x49/0x3b0
kthread+0x109/0x140
ret_from_fork+0x31/0x40Signed-off-by: Bart Van Assche
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
This patch introduces support in target_submit_tmr() for locating a
unpacked_lun from an existing se_cmd->tag during ABORT_TASK.When TARGET_SCF_LOOKUP_LUN_FROM_TAG is set, target_submit_tmr()
will do the extra lookup via target_lookup_lun_from_tag() and
subsequently invoke transport_lookup_tmr_lun() so a proper
percpu se_lun->lun_ref is taken before workqueue dispatch into
se_device->tmr_wq happens.Aside from the extra target_lookup_lun_from_tag(), the existing
code-path remains unchanged.Reviewed-by: Himanshu Madhani
Reviewed-by: Quinn Tran
Cc: Mike Christie
Cc: Hannes Reinecke
Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch introduces TMR percpu reference counting using
se_lun->lun_ref in transport_lookup_tmr_lun(), following
how existing non TMR per se_lun reference counting works
within transport_lookup_cmd_lun().It also adds explicit transport_lun_remove_cmd() calls to
drop the reference in the three tmr related locations that
invoke transport_cmd_check_stop_to_fabric();- target_tmr_work() during normal ->queue_tm_rsp()
- target_complete_tmr_failure() during error ->queue_tm_rsp()
- transport_generic_handle_tmr() during early failureAlso, note the exception paths in transport_generic_free_cmd()
and transport_cmd_finish_abort() already check SCF_SE_LUN_CMD,
and will invoke transport_lun_remove_cmd() when necessary.Reviewed-by: Himanshu Madhani
Reviewed-by: Quinn Tran
Cc: Mike Christie
Cc: Hannes Reinecke
Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger
09 Jun, 2017
1 commit
-
This patch fixes a se_cmd->cmd_kref underflow during CMD_T_ABORTED
when a fabric driver drops it's second reference from below the
target_core_tmr.c based callers of transport_cmd_finish_abort().Recently with the conversion of kref to refcount_t, this bug was
manifesting itself as:[705519.601034] refcount_t: underflow; use-after-free.
[705519.604034] INFO: NMI handler (kgdb_nmi_handler) took too long to run: 20116.512 msecs
[705539.719111] ------------[ cut here ]------------
[705539.719117] WARNING: CPU: 3 PID: 26510 at lib/refcount.c:184 refcount_sub_and_test+0x33/0x51Since the original kref atomic_t based kref_put() didn't check for
underflow and only invoked the final callback when zero was reached,
this bug did not manifest in practice since all se_cmd memory is
using preallocated tags.To address this, go ahead and propigate the existing return from
transport_put_cmd() up via transport_cmd_finish_abort(), and
change transport_cmd_finish_abort() + core_tmr_handle_tas_abort()
callers to only do their local target_put_sess_cmd() if necessary.Reported-by: Bart Van Assche
Tested-by: Bart Van Assche
Cc: Mike Christie
Cc: Hannes Reinecke
Cc: Christoph Hellwig
Cc: Himanshu Madhani
Cc: Sagi Grimberg
Cc: stable@vger.kernel.org # 3.14+
Tested-by: Gary Guo
Tested-by: Chu Yuan Lin
Signed-off-by: Nicholas Bellinger
16 May, 2017
1 commit
-
During v4.3 when the overflow/underflow check was relaxed by
commit c72c525022:commit c72c5250224d475614a00c1d7e54a67f77cd3410
Author: Roland Dreier
Date: Wed Jul 22 15:08:18 2015 -0700target: allow underflow/overflow for PR OUT etc. commands
to allow underflow/overflow for Windows compliance + FCP, a
consequence was to allow control CDBs to process overflow
data for iscsi-target with immediate data as well.As per Roland's original change, continue to allow underflow
cases for control CDBs to make Windows compliance + FCP happy,
but until overflow for control CDBs is supported tree-wide,
explicitly reject all control WRITEs with overflow following
pre v4.3.y logic.Reported-by: Bart Van Assche
Cc: Roland Dreier
Cc: # v4.3+
Signed-off-by: Nicholas Bellinger
02 May, 2017
1 commit
-
A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
Signed-off-by: Nicholas Bellinger
31 Mar, 2017
1 commit
-
This patch fixes a set of queue-full response handling
bugs, where outgoing responses are leaked when a fabric
driver is propagating non -EAGAIN or -ENOMEM errors
to target-core.It introduces TRANSPORT_COMPLETE_QF_ERR state used to
signal when CHECK_CONDITION status should be generated,
when fabric driver ->write_pending(), ->queue_data_in(),
or ->queue_status() callbacks fail with non -EAGAIN or
-ENOMEM errors, and data-transfer should not be retried.Note all fabric driver -EAGAIN and -ENOMEM errors are
still retried indefinately with associated data-transfer
callbacks, following existing queue-full logic.Also fix two missing ->queue_status() queue-full cases
related to CMD_T_ABORTED w/ TAS status handling.Reported-by: Potnuri Bharat Teja
Reviewed-by: Potnuri Bharat Teja
Tested-by: Potnuri Bharat Teja
Cc: Potnuri Bharat Teja
Reported-by: Steve Wise
Cc: Steve Wise
Cc: Sagi Grimberg
Signed-off-by: Nicholas Bellinger
19 Mar, 2017
1 commit
-
All in-tree fabric drivers provide a tfo->check_stop_free(),
so there is no need to do the extra check within existing
transport_cmd_check_stop_to_fabric() code.Just to be sure, add a check in target_fabric_tf_ops_check()
to notify any out-of-tree drivers that might be missing it.Signed-off-by: Nicholas Bellinger
27 Feb, 2017
1 commit
-
When transport_clear_lun_ref() is shutting down a se_lun via
configfs with new I/O in-flight, it's possible to trigger a
NULL pointer dereference in transport_lookup_cmd_lun() due
to the fact percpu_ref_get() doesn't do any __PERCPU_REF_DEAD
checking before incrementing lun->lun_ref.count after
lun->lun_ref has switched to atomic_t mode.This results in a NULL pointer dereference as LUN shutdown
code in core_tpg_remove_lun() continues running after the
existing ->release() -> core_tpg_lun_ref_release() callback
completes, and clears the RCU protected se_lun->lun_se_dev
pointer.During the OOPs, the state of lun->lun_ref in the process
which triggered the NULL pointer dereference looks like
the following on v4.1.y stable code:struct se_lun {
lun_link_magic = 4294932337,
lun_status = TRANSPORT_LUN_STATUS_FREE,.....
lun_se_dev = 0x0,
lun_sep = 0x0,.....
lun_ref = {
count = {
counter = 1
},
percpu_count_ptr = 3,
release = 0xffffffffa02fa1e0 ,
confirm_switch = 0x0,
force_atomic = false,
rcu = {
next = 0xffff88154fa1a5d0,
func = 0xffffffff8137c4c0
}
}
}To address this bug, use percpu_ref_tryget_live() to ensure
once __PERCPU_REF_DEAD is visable on all CPUs and ->lun_ref
has switched to atomic_t, all new I/Os will fail to obtain
a new lun->lun_ref reference.Also use an explicit percpu_ref_kill_and_confirm() callback
to block on ->lun_ref_comp to allow the first stage and
associated RCU grace period to complete, and then block on
->lun_ref_shutdown waiting for the final percpu_ref_put()
to drop the last reference via transport_lun_remove_cmd()
before continuing with core_tpg_remove_lun() shutdown.Reported-by: Rob Millner
Tested-by: Rob Millner
Cc: Rob Millner
Tested-by: Vaibhav Tandon
Cc: Vaibhav Tandon
Tested-by: Bryant G. Ly
Cc: # v3.14+
Signed-off-by: Nicholas Bellinger
21 Feb, 2017
1 commit
-
If a target driver (e.g. tcm_qla2xxx) calls
transport_generic_request_failure() to report that receiving data
has failed and that SCSI command has already been aborted by the
initiator, ensure that the SCSI status ABORTED is sent back to the
initiator instead of the sense code provided by the target driver.Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
Cc: Christoph Hellwig
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger
09 Feb, 2017
8 commits
-
The code that tests the CMD_T_DEV_ACTIVE flag has been removed. Hence
also remove the flag itself.Signed-off-by: Bart Van Assche
Reviewed-by: Christoph Hellwig
Reviewed-by: Andy Grover
Reviewed-by: Sagi Grimberg
Reviewed-by: Hannes Reinecke
Signed-off-by: Nicholas Bellinger -
The patch that reworks task management function handling guarantees
that target_remove_from_state_list() is always called with CMD_T_BUSY
cleared. Since that function is the only function that tests that flag
this means that that flag is now superfluous. Hence remove that flag.Signed-off-by: Bart Van Assche
Reviewed-by: Andy Grover
Reviewed-by: Christoph Hellwig
Reviewed-by: Sagi Grimberg
Reviewed-by: Hannes Reinecke
Signed-off-by: Nicholas Bellinger -
This patch does not change any functionality.
Signed-off-by: Bart Van Assche
Cc: Hannes Reinecke
Cc: Christoph Hellwig
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
The function transport_cmd_check_stop() has two callers. These callers
invoke this function as follows:
* transport_cmd_check_stop(cmd, true, false)
* transport_cmd_check_stop(cmd, false, true)
Hence inline this function into its callers.This patch does not change any functionality but improves source
code readability.Signed-off-by: Bart Van Assche
Reviewed-by: Hannes Reinecke
Reviewed-by: Bryant G. Ly
Cc: Christoph Hellwig
Cc: Andy Grover
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
Enabling dynamic debug for the target_core_mod kernel module
causes the system log to be spammed with the "Incremented ..."
message. Hence remove it.Signed-off-by: Bart Van Assche
Reviewed-by: Hannes Reinecke
Reviewed-by: Christoph Hellwig
Cc: Andy Grover
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
Stop execution if CMD_T_STOP has been set for a command just after
the command has been added to the device command list and before
.write_pending() is called. The following sequence can trigger this:
- transport_handle_cdb_direct() gets called. This function namely
sets CMD_T_ACTIVE before it calls transport_generic_new_cmd().
- __transport_wait_for_tasks() is called concurrently. This function
sets CMD_T_STOP for all active commands that have not been aborted.Signed-off-by: Bart Van Assche
Reviewed-by: Hannes Reinecke
Cc: Christoph Hellwig
Cc: Andy Grover
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
transport_wait_for_tasks() not only waits for command completion
but also sets CMD_T_STOP. Additionally, this function is not only
called by frontend drivers but also by the target core. Update
the transport_wait_for_tasks() documentation to reflect this.Signed-off-by: Bart Van Assche
Reviewed-by: Hannes Reinecke
Cc: Christoph Hellwig
Cc: Andy Grover
Cc: David Disseldorp
Signed-off-by: Nicholas Bellinger -
This patch addresses a long-standing bug with multi-session
(eg: iscsi-target + iser-target) se_node_acl dynamic free
withini transport_deregister_session().This bug is caused when a storage endpoint is configured with
demo-mode (generate_node_acls = 1 + cache_dynamic_acls = 1)
initiators, and initiator login creates a new dynamic node acl
and attaches two sessions to it.After that, demo-mode for the storage instance is disabled via
configfs (generate_node_acls = 0 + cache_dynamic_acls = 0) and
the existing dynamic acl is never converted to an explicit ACL.The end result is dynamic acl resources are released twice when
the sessions are shutdown in transport_deregister_session().If the storage instance is not changed to disable demo-mode,
or the dynamic acl is converted to an explict ACL, or there
is only a single session associated with the dynamic ACL,
the bug is not triggered.To address this big, move the release of dynamic se_node_acl
memory into target_complete_nacl() so it's only freed once
when se_node_acl->acl_kref reaches zero.(Drop unnecessary list_del_init usage - HCH)
Reported-by: Rob Millner
Tested-by: Rob Millner
Cc: Rob Millner
Cc: stable@vger.kernel.org # 4.1+
Signed-off-by: Nicholas Bellinger
08 Feb, 2017
1 commit
-
This patch fixes a bug where incoming task management requests
can be explicitly aborted during an active LUN_RESET, but who's
struct work_struct are canceled in-flight before execution.This occurs when core_tmr_drain_tmr_list() invokes cancel_work_sync()
for the incoming se_tmr_req->task_cmd->work, resulting in cmd->work
for target_tmr_work() never getting invoked and the aborted TMR
waiting indefinately within transport_wait_for_tasks().To address this case, perform a CMD_T_ABORTED check early in
transport_generic_handle_tmr(), and invoke the normal path via
transport_cmd_check_stop_to_fabric() to complete any TMR kthreads
blocked waiting for CMD_T_STOP in transport_wait_for_tasks().Also, move the TRANSPORT_ISTATE_PROCESSING assignment earlier
into transport_generic_handle_tmr() so the existing check in
core_tmr_drain_tmr_list() avoids attempting abort the incoming
se_tmr_req->task_cmd->work if it has already been queued into
se_device->tmr_wq.Reported-by: Rob Millner
Tested-by: Rob Millner
Cc: Rob Millner
Reviewed-by: Christoph Hellwig
Cc: stable@vger.kernel.org # 3.14+
Signed-off-by: Nicholas Bellinger
11 Jan, 2017
1 commit
-
As defined in http://www.t10.org/lists/asc-num.htm. To be used during
validation of XCOPY target and segment descriptor lists.Signed-off-by: David Disseldorp
Reviewed-by: Christoph Hellwig
Signed-off-by: Bart Van Assche
21 Oct, 2016
1 commit
-
target_sess_cmd_list_waiting() might hit on a condition where
the kref for the command is already 0, but the destructor has
not been called yet (or is stuck in waiting for a spin lock).
Rather than leaving the command on the list we should explicitly
remove it to avoid race issues later on.Signed-off-by: Hannes Reinecke
Signed-off-by: Nicholas Bellinger
20 Oct, 2016
3 commits
-
This reverts commit c1ccbfe0311e2380a6d2dcb0714b36904f5d586f.
Reverting this patch, as it incorrectly assumes the additional length
for INQUIRY in target_complete_cmd_with_length() is SCSI allocation
length, which breaks existing user-space code when SCSI allocation
length is smaller than additional length.root@scsi-mq:~# sg_inq --len=4 -vvvv /dev/sdb
found bsg_major=253
open /dev/sdb with flags=0x800
inquiry cdb: 12 00 00 00 04 00
duration=0 ms
inquiry: pass-through requested 4 bytes (data-in) but got -28 bytes
inquiry: pass-through can't get negative bytes, say it got none
inquiry: got too few bytes (0)
INQUIRY resid (32) should never exceed requested len=4
inquiry: failed requesting 4 byte response: Malformed response to
SCSI command [resid=32]AFAICT the original change was not to address a specific host issue,
so go ahead and revert to original logic for now.Cc: Douglas Gilbert
Cc: Martin K. Petersen
Cc: Sumit Rai
Cc: stable@vger.kernel.org # 4.8+
Signed-off-by: Nicholas Bellinger -
This patch addresses a bug where EXTENDED_COPY across multiple LUNs
results in a CHECK_CONDITION when the source + destination are not
located on the same physical node.ESX Host environments expect sense COPY_ABORTED w/ COPY TARGET DEVICE
NOT REACHABLE to be returned when this occurs, in order to signal
fallback to local copy method.As described in section 6.3.3 of spc4r22:
"If it is not possible to complete processing of a segment because the
copy manager is unable to establish communications with a copy target
device, because the copy target device does not respond to INQUIRY,
or because the data returned in response to INQUIRY indicates
an unsupported logical unit, then the EXTENDED COPY command shall be
terminated with CHECK CONDITION status, with the sense key set to
COPY ABORTED, and the additional sense code set to COPY TARGET DEVICE
NOT REACHABLE."Tested on v4.1.y with ESX v5.5u2+ with BlockCopy across multiple nodes.
Reported-by: Nixon Vincent
Tested-by: Nixon Vincent
Cc: Nixon Vincent
Tested-by: Dinesh Israni
Signed-off-by: Dinesh Israni
Cc: Dinesh Israni
Cc: stable@vger.kernel.org # 3.14+
Signed-off-by: Nicholas Bellinger -
This patch fixes a regression in >= v4.1.y code where the original
SCF_ACK_KREF assignment in target_get_sess_cmd() was dropped upstream
in commit 054922bb, but the series for addressing TMR ABORT_TASK +
LUN_RESET with fabric session reinstatement in commit febe562c20 still
depends on this code in transport_cmd_finish_abort().The regression manifests itself as a se_cmd->cmd_kref +1 leak, where
ABORT_TASK + LUN_RESET can hang indefinately for a specific I_T session
for drivers using SCF_ACK_KREF, resulting in hung kthreads.This patch has been verified with v4.1.y code.
Reported-by: Vaibhav Tandon
Tested-by: Vaibhav Tandon
Cc: Vaibhav Tandon
Cc: stable@vger.kernel.org # 4.1+
Signed-off-by: Nicholas Bellinger
24 Jul, 2016
1 commit
-
This patch fixes residual overflow handling to correctly set the
residual_count using SPDTL, instead of SCSI Allocation Length.Allocation Length is the maximum value of the SPDTL and not substitute
for it, hence it shouldn’t be used to calculate ResidualCount except for
cases where SPDTL > Allocation Length and Data is truncated (in that
case both Alloc Len and SPDTL are same). (SPC 5r01 Section 4.2.5.6).Thanks to Ajay Nair in assisting with this patch.
Signed-off-by: Sumit Rai
Signed-off-by: Nicholas Bellinger
20 Jul, 2016
4 commits
-
If a Simple command is sent with a failure, target_setup_cmd_from_cdb
returns with TCM_UNSUPPORTED_SCSI_OPCODE or TCM_INVALID_CDB_FIELD.So in the cases where target_setup_cmd_from_cdb returns an error, we
never get far enough to call target_execute_cmd to increment simple_cmds.
Since simple_cmds isn't incremented, the result of the failure from
target_setup_cmd_from_cdb causes transport_generic_request_failure to
decrement simple_cmds, due to call to transport_complete_task_attr.With this dev->simple_cmds or dev->dev_ordered_sync is now -1, not 0.
So when a subsequent command with an Ordered Task is sent, it causes
a hang, since dev->simple_cmds is at -1.Tested-by: Bryant G. Ly
Signed-off-by: Bryant G. Ly
Tested-by: Michael Cyr
Signed-off-by: Michael Cyr
Cc: stable@vger.kernel.org # 4.4+
Signed-off-by: Nicholas Bellinger -
If a command with a Simple task attribute is failed due to a Unit
Attention, then a subsequent command with an Ordered task attribute
will hang forever. The reason for this is that the Unit Attention
status is checked for in target_setup_cmd_from_cdb, before the call
to target_execute_cmd, which calls target_handle_task_attr, which
in turn increments dev->simple_cmds.However, transport_generic_request_failure still calls
transport_complete_task_attr, which will decrement dev->simple_cmds.
In this case, simple_cmds is now -1. So when a command with the
Ordered task attribute is sent, target_handle_task_attr sees that
dev->simple_cmds is not 0, so it decides it can't execute the
command until all the (nonexistent) Simple commands have completed.Reported-by: Michael Cyr
Tested-by: Michael Cyr
Reported-by: Bryant G. Ly
Tested-by: Bryant G. Ly
Cc: stable@vger.kernel.org # 4.4+
Signed-off-by: Nicholas Bellinger -
This patch fixes a race in iscsit_release_commands_from_conn() ->
iscsit_free_cmd() -> transport_generic_free_cmd() + wait_for_tasks=1,
where CMD_T_FABRIC_STOP could end up being set after the final
kref_put() is called from core_tmr_abort_task() context.This results in transport_generic_free_cmd() blocking indefinately
on se_cmd->cmd_wait_comp, because the target_release_cmd_kref()
check for CMD_T_FABRIC_STOP returns false.To address this bug, make iscsit_release_commands_from_conn()
do list_splice and set CMD_T_FABRIC_STOP early while holding
iscsi_conn->cmd_lock. Also make iscsit_aborted_task() only
remove iscsi_cmd_t if CMD_T_FABRIC_STOP has not already been
set.Finally in target_release_cmd_kref(), only honor fabric_stop
if CMD_T_ABORTED has been set.Cc: Mike Christie
Cc: Quinn Tran
Cc: Himanshu Madhani
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: stable@vger.kernel.org # 3.14+
Tested-by: Nicholas Bellinger
Signed-off-by: Nicholas Bellinger -
During transport_generic_free_cmd() with a concurrent TMR
ABORT_TASK and shutdown CMD_T_FABRIC_STOP bit set, the
caller will be blocked on se_cmd->cmd_wait_stop completion
until the final kref_put() -> target_release_cmd_kref()
has been invoked to call complete().However, when ABORT_TASK is completed with FUNCTION_COMPLETE
in core_tmr_abort_task(), the aborted se_cmd will have already
been removed from se_sess->sess_cmd_list via list_del_init().This results in target_release_cmd_kref() hitting the
legacy list_empty() == true check, invoking ->release_cmd()
but skipping complete() to wakeup se_cmd->cmd_wait_stop
blocked earlier in transport_generic_free_cmd() code.To address this bug, it's safe to go ahead and drop the
original list_empty() check so that fabric_stop invokes
the complete() as expected, since list_del_init() can
safely be used on a empty list.Cc: Mike Christie
Cc: Quinn Tran
Cc: Himanshu Madhani
Cc: Christoph Hellwig
Cc: Hannes Reinecke
Cc: stable@vger.kernel.org # 3.14+
Tested-by: Nicholas Bellinger
Signed-off-by: Nicholas Bellinger
29 May, 2016
1 commit
-
Pull SCSI target updates from Nicholas Bellinger:
"Here are the outstanding target pending updates for v4.7-rc1.The highlights this round include:
- Allow external PR/ALUA metadata path be defined at runtime via top
level configfs attribute (Lee)
- Fix target session shutdown bug for ib_srpt multi-channel (hch)
- Make TFO close_session() and shutdown_session() optional (hch)
- Drop se_sess->sess_kref + convert tcm_qla2xxx to internal kref
(hch)
- Add tcm_qla2xxx endpoint attribute for basic FC jammer (Laurence)
- Refactor iscsi-target RX/TX PDU encode/decode into common code
(Varun)
- Extend iscsit_transport with xmit_pdu, release_cmd, get_rx_pdu,
validate_parameters, and get_r2t_ttt for generic ISO offload
(Varun)
- Initial merge of cxgb iscsi-segment offload target driver (Varun)The bulk of the changes are Chelsio's new driver, along with a number
of iscsi-target common code improvements made by Varun + Co along the
way"* 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending: (29 commits)
iscsi-target: Fix early sk_data_ready LOGIN_FLAGS_READY race
cxgbit: Use type ISCSI_CXGBIT + cxgbit tpg_np attribute
iscsi-target: Convert transport drivers to signal rdma_shutdown
iscsi-target: Make iscsi_tpg_np driver show/store use generic code
tcm_qla2xxx Add SCSI command jammer/discard capability
iscsi-target: graceful disconnect on invalid mapping to iovec
target: need_to_release is always false, remove redundant check and kfree
target: remove sess_kref and ->shutdown_session
iscsi-target: remove usage of ->shutdown_session
tcm_qla2xxx: introduce a private sess_kref
target: make close_session optional
target: make ->shutdown_session optional
target: remove acl_stop
target: consolidate and fix session shutdown
cxgbit: add files for cxgbit.ko
iscsi-target: export symbols
iscsi-target: call complete on conn_logout_comp
iscsi-target: clear tx_thread_active
iscsi-target: add new offload transport type
iscsi-target: use conn_transport->transport_type in text rsp
...
14 May, 2016
1 commit
-
The SRP target driver will need to allocate and chain it's own SGLs soon.
For this export target_alloc_sgl, and add a new argument to it so that it
can allocate an additional chain entry that doesn't point to a page. Also
export transport_free_sgl after renaming it to target_free_sgl to free
these SGLs again.Signed-off-by: Christoph Hellwig
Reviewed-by: Bart Van Assche
Signed-off-by: Doug Ledford
10 May, 2016
1 commit
-
Both of them are unused now that drivers handle any delayed session
shutdown internally.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger