12 Jul, 2013
1 commit
-
Pull SCSI target updates from Nicholas Bellinger:
"Lots of activity this round on performance improvements in target-core
while benchmarking the prototype scsi-mq initiator code with
vhost-scsi fabric ports, along with a number of iscsi/iser-target
improvements and hardening fixes for exception path cases post v3.10
merge.The highlights include:
- Make persistent reservations APTPL buffer allocated on-demand, and
drop per t10_reservation buffer. (grover)
- Make virtual LUN=0 a NULLIO device, and skip allocation of NULLIO
device pages (grover)
- Add transport_cmd_check_stop write_pending bit to avoid extra
access of ->t_state_lock is WRITE I/O submission fast-path. (nab)
- Drop unnecessary CMD_T_DEV_ACTIVE check from
transport_lun_remove_cmd to avoid extra access of ->t_state_lock in
release fast-path. (nab)
- Avoid extra t_state_lock access in __target_execute_cmd fast-path
(nab)
- Drop unnecessary vhost-scsi wait_for_tasks=true usage +
->t_state_lock access in release fast-path. (nab)
- Convert vhost-scsi to use modern se_cmd->cmd_kref
TARGET_SCF_ACK_KREF usage (nab)
- Add tracepoints for SCSI commands being processed (roland)
- Refactoring of iscsi-target handling of ISCSI_OP_NOOP +
ISCSI_OP_TEXT to be transport independent (nab)
- Add iscsi-target SendTargets=$IQN support for in-band discovery
(nab)
- Add iser-target support for in-band discovery (nab + Or)
- Add iscsi-target demo-mode TPG authentication context support (nab)
- Fix isert_put_reject payload buffer post (nab)
- Fix iscsit_add_reject* usage for iser (nab)
- Fix iscsit_sequence_cmd reject handling for iser (nab)
- Fix ISCSI_OP_SCSI_TMFUNC handling for iser (nab)
- Fix session reset bug with RDMA_CM_EVENT_DISCONNECTED (nab)The last five iscsi/iser-target items are CC'ed to stable, as they do
address issues present in v3.10 code. They are certainly larger than
I'd like for stable patch set, but are important to ensure proper
REJECT exception handling in iser-target for 3.10.y"* 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending: (51 commits)
iser-target: Ignore non TEXT + LOGOUT opcodes for discovery
target: make queue_tm_rsp() return void
target: remove unused codes from enum tcm_tmrsp_table
iscsi-target: kstrtou* configfs attribute parameter cleanups
iscsi-target: Fix tfc_tpg_auth_cit configfs length overflow
iscsi-target: Fix tfc_tpg_nacl_auth_cit configfs length overflow
iser-target: Add support for ISCSI_OP_TEXT opcode + payload handling
iser-target: Rename sense_buf_[dma,len] to pdu_[dma,len]
iser-target: Add vendor_err debug output
target: Add (obsolete) checking for PMI/LBA fields in READ CAPACITY(10)
target: Return correct sense data for IO past the end of a device
target: Add tracepoints for SCSI commands being processed
iser-target: Fix session reset bug with RDMA_CM_EVENT_DISCONNECTED
iscsi-target: Fix ISCSI_OP_SCSI_TMFUNC handling for iser
iscsi-target: Fix iscsit_sequence_cmd reject handling for iser
iscsi-target: Fix iscsit_add_reject* usage for iser
iser-target: Fix isert_put_reject payload buffer post
iscsi-target: missing kfree() on error path
iscsi-target: Drop left-over iscsi_conn->bad_hdr
target: Make core_scsi3_update_and_write_aptpl return sense_reason_t
...
08 Jul, 2013
9 commits
-
The return value wasn't checked by any of the callers. Assuming this is
correct behaviour, we can simplify some code by not bothering to
generate it.nab: Add srpt_queue_data_in() + srpt_queue_tm_rsp() nops around
srpt_queue_response() void returnSigned-off-by: Joern Engel
Signed-off-by: Nicholas Bellinger -
Three have been checked for but were never set. Remove the dead code.
Also renumbers the remaining ones to a) get rid of the holes after the
removal and b) avoid a collision between TMR_FUNCTION_COMPLETE==0 and
the uninitialized case. If we failed to set a code, we should rather
fall into the default case then return success.Signed-off-by: Joern Engel
Signed-off-by: Nicholas Bellinger -
This patch includes the conversion of iscsi-target configfs
attributes for NetworkPortal, NodeACL, TPG, IQN and Discovery
groups to use kstrtou*() instead of simple_strtou*().It also cleans up new-line usage during iscsi_tpg_param_store_##name
to use isspace().Signed-off-by: Joern Engel
Signed-off-by: Nicholas Bellinger -
This patch fixes another potential buffer overflow while processing
iscsi_node_auth input for configfs attributes in v3.11 for-next
TPG tfc_tpg_auth_cit context.Reported-by: Joern Engel
Signed-off-by: Nicholas Bellinger -
This patch fixes a potential buffer overflow while processing
iscsi_node_auth input for configfs attributes within NodeACL
tfc_tpg_nacl_auth_cit context.Signed-off-by: Joern Engel
Cc: stable@vger.kernel.org
Signed-off-by: Nicholas Bellinger -
The SBC-2 specification of READ CAPACITY(10) has PMI and LOGICAL BLOCK
ADDRESS fields in the CDB; in SBC-3 these fields are simply listed as
obsolete. However, SBC-2 also has the languageIf the PMI bit is set to zero and the LOGICAL BLOCK ADDRESS field
is not set to zero, the device server shall terminate the command
with CHECK CONDITION status with the sense key set to ILLEGAL
REQUEST and the additional sense code set to INVALID FIELD IN CDB.and in fact at least the Windows SCSI compliance test checks this
behavior. Since no one following SBC-3 is going to set these fields,
we might as well include the check from SBC-2 and pass this test.Signed-off-by: Roland Dreier
Signed-off-by: Nicholas Bellinger -
We should use TCM_ADDRESS_OUT_OF_RANGE (-> sense data LOGICAL BLOCK
ADDRESS OUT OF RANGE) for IOs past the end of a device instead of
INVALID FIELD IN CDB.Signed-off-by: Roland Dreier
Signed-off-by: Nicholas Bellinger -
This patch adds tracepoints to the target code for commands being
received and being completed, which is quite useful for debugging
interactions with initiators. For example, one can do something like the
following to watch commands that are completing unsuccessfully:# echo 'scsi_status!=0' > /sys/kernel/debug/tracing/events/target/target_cmd_complete/filter
# echo 1 > /sys/kernel/debug/tracing/events/target/target_cmd_complete/enable# cat /sys/kernel/debug/tracing/trace
iscsi_trx-0-1902 [003] ...1 990185.810385: target_cmd_complete: iqn.1993-08.org.debian:01:e51ede6aacfd
Signed-off-by: Nicholas Bellinger -
This patch addresses a bug where RDMA_CM_EVENT_DISCONNECTED may occur
before the connection shutdown has been completed by rx/tx threads,
that causes isert_free_conn() to wait indefinately on ->conn_wait.This patch allows isert_disconnect_work code to invoke rdma_disconnect
when isert_disconnect_work() process context is started by client
session reset before isert_free_conn() code has been reached.It also adds isert_conn->conn_mutex protection for ->state within
isert_disconnect_work(), isert_cq_comp_err() and isert_free_conn()
code, along with isert_check_state() for wait_event usage.(v2: Add explicit iscsit_cause_connection_reinstatement call
during isert_disconnect_work() to force conn reset)Cc: stable@vger.kernel.org # 3.10+
Cc: Or Gerlitz
Signed-off-by: Nicholas Bellinger
07 Jul, 2013
3 commits
-
This patch adds target_get_sess_cmd reference counting for
iscsit_handle_task_mgt_cmd(), and adds a target_put_sess_cmd()
for the failure case.It also fixes a bug where ISCSI_OP_SCSI_TMFUNC type commands
where leaking iscsi_cmd->i_conn_node and eventually triggering
an OOPs during struct isert_conn shutdown.Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger -
This patch moves ISCSI_OP_REJECT failures into iscsit_sequence_cmd()
in order to avoid external iscsit_reject_cmd() reject usage for all
PDU types.It also updates PDU specific handlers for traditional iscsi-target
code to not reset the session after posting a ISCSI_OP_REJECT during
setup.(v2: Fix CMDSN_LOWER_THAN_EXP for ISCSI_OP_SCSI to call
target_put_sess_cmd() after iscsit_sequence_cmd() failure)Cc: Or Gerlitz
Cc: Mike Christie
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger -
This patch changes iscsit_add_reject() + iscsit_add_reject_from_cmd()
usage to not sleep on iscsi_cmd->reject_comp to address a free-after-use
usage bug in v3.10 with iser-target code.It saves ->reject_reason for use within iscsit_build_reject() so the
correct value for both transport cases. It also drops the legacy
fail_conn parameter usage throughput iscsi-target code and adds
two iscsit_add_reject_cmd() and iscsit_reject_cmd helper functions,
along with various small cleanups.(v2: Re-enable target_put_sess_cmd() to be called from
iscsit_add_reject_from_cmd() for rejects invoked after
target_get_sess_cmd() has been called)Cc: Or Gerlitz
Cc: Mike Christie
Cc: stable@vger.kernel.org # 3.10+
Signed-off-by: Nicholas Bellinger
04 Jul, 2013
8 commits
-
Fix-up breakage in iscsit_build_sendtargets_response() from v3.11
changes, and free "payload" before returning.Signed-off-by: Dan Carpenter
Signed-off-by: Nicholas Bellinger -
All REJECT response setup of the rejected payload is now done using
on-demand cmd->buf_ptr allocations.Go ahead and remove dead iscsi_conn->bad_hdr usage rx_opcode path
Signed-off-by: Nicholas Bellinger
-
Fix up sense_reason_t breakage in core_scsi3_update_and_write_aptpl()
from recent conversion to use local scope memory allocation.Reported as sparse warnings: (new ones prefixed by >>) by Fengguang:
>> drivers/target/target_core_pr.c:2069:57: sparse: incorrect type in
>> return expression (different base types)
drivers/target/target_core_pr.c:2069:57: expected restricted sense_reason_t
drivers/target/target_core_pr.c:2069:57: got int
>> drivers/target/target_core_pr.c:2179:21: sparse: incorrect type in
>> assignment (different base types)
drivers/target/target_core_pr.c:2179:21: expected restricted sense_reason_t [assigned] [usertype] ret
drivers/target/target_core_pr.c:2179:21: got int
>> drivers/target/target_core_pr.c:2197:13: sparse: incorrect type in
>> assignment (different base types)
drivers/target/target_core_pr.c:2197:13: expected restricted sense_reason_t [assigned] [usertype] ret
drivers/target/target_core_pr.c:2197:13: got int
drivers/target/target_core_pr.c:1245:28: sparse: context imbalance in '__core_scsi3_free_registration' - unexpected unlockReported-by: kbuild test robot
Cc: Andy Grover
Signed-off-by: Nicholas Bellinger -
This patch adds a auth configfs group context following existing
explict NodeACL and discovery auth within:/sys/kernel/config/target/iscsi/$TARGETNAME/$TPGT/auth/
This patch allows these attributes to be used for CHAP authentication
an TPG is configured in demo-mode (generate_node_acl=1).Note this authentication information takes precedence over NodeACL
authentication when struct se_node_acl->dynamic_node_acl is present.Cc: Dax Kelson
Signed-off-by: Nicholas Bellinger -
This patch adds an optional /auth/ configfs group to TPG context that
can be used by fabrics like iscsi-target for TPG demo-mode
authentication.Cc: Dax Kelson
Signed-off-by: Nicholas Bellinger -
This patch changes ISCSI_OP_TEXT handling of SendTargets=[iqn.,eui.]
payloads to return explicit discovery information.It adds checks to iscsit_process_text_cmd() and adds the special single
$TARGETNAME discovery case in iscsit_build_sendtargets_response() code.Cc: Or Gerlitz
Cc: Mike Christie
Signed-off-by: Nicholas Bellinger -
This patch moves ISCSI_OP_TEXT PDU buffer sanity checks to
iscsit_process_text_cmd() code, so that it can be shared
with iser-target code.It adds IFC_SENDTARGETS_ALL + iscsi_cmd->text_in_ptr in order
to save text payload for ISCSI_OP_TEXT_RSP, and updates
iscsit_release_cmd() to assigned memory.Cc: Or Gerlitz
Cc: Mike Christie
Signed-off-by: Nicholas Bellinger -
This patch changes iscsi_set_connection_parameters() to allow
conn_ops->MaxXmitDataSegmentLength assignement to occur during
in-band iser send-targets discovery, as this value is required
by TEXT response processing code.Reported-by: Or Gerlitz
Cc: Or Gerlitz
Cc: Mike Christie
Signed-off-by: Nicholas Bellinger
26 Jun, 2013
1 commit
-
This patch refactoring existing iscsit_send_text_rsp() in order
to handle iscsi_text_rsp payloads in a transport specific manner.This includes the addition of iscsit_build_text_rsp() to build
the response payload and initialize ISCSI_OP_TEXT_RSP.v2: Make iscsit_build_text_rsp() determine extra padding bytes, and
drop legacy padding calculation for traditional iSCSI text
responses within iscsit_send_text_rsp()Reported-by: Or Gerlitz
Cc: Or Gerlitz
Cc: Mike Christie
Signed-off-by: Nicholas Bellinger
25 Jun, 2013
2 commits
-
This patch refactors ISCSI_OP_TEXT handling within iscsi-target in
order to handle iscsi_text payloads in a transport specific manner.This includes splitting current iscsit_handle_text_cmd() into
iscsit_setup_text_cmd() and iscsit_process_text_cmd() calls, and
makes iscsit_handle_text_cmd be only used internally by traditional
iscsi socket calls.Cc: Or Gerlitz
Cc: Mike Christie
Signed-off-by: Nicholas Bellinger -
This patch refactors ISCSI_OP_NOOP handling within iscsi-target in
order to handle iscsi_nopout payloads in a transport specific manner.This includes splitting existing iscsit_handle_nop_out() into
iscsit_setup_nop_out() and iscsit_process_nop_out() calls, and
makes iscsit_handle_nop_out() be only used internally by traditional
iscsi socket calls.Next update iser-target code to use new callers and add FIXME for
the handling iscsi_nopout payloads. Also fix reject response handling
in iscsit_setup_nop_out() to use proper iscsit_add_reject_from_cmd().v2: Fix uninitialized iscsit_handle_nop_out() payload_length usage (Fengguang)
v3: Remove left-over dead code in iscsit_setup_nop_out() (DanC)Signed-off-by: Nicholas Bellinger
21 Jun, 2013
9 commits
-
Reported-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
Writing 0 when iser was not previously enabled, so succeed but do
nothing so that user-space code doesn't need a try: catch block
when ib_isert logic is not available.Also, return actual error from add_network_portal using PTR_ERR
during op=enable failure.Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
Now with iscsi-target using modern se_cmd->cmd_kref accounting in
v3.10 code, it's safe to go ahead and drop the legacy release
codepath + se_cmd->check_release bit in transport_release_cmd()Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger -
In modern iscsi-target code, the setup and I/O submission is done within a
single process context, so there is no need to acquire se_cmd->t_state_lock while
checking SCF_SUPPORTED_SAM_OPCODE for determining when unsolicited data-out
should be dumped.Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger -
This patch drops the se_cmd->t_state_lock access around SCF_SUPPORTED_SAM_OPCODE
assignment within target_setup_cmd_from_cdb().Original v4.0 target code required this as fabrics would be checking for
this values in different process contexts for setup and I/O submission.Given that modern v4.1 target code performs setup and I/O submission
from the same process context, this t_state_lock access is no longer
required.Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger -
This patch makes target_execute_cmd() set CMD_T_BUSY|CMD_T_SENT while
holding se_cmd->t_state_lock, in order to avoid the extra aquire/release
in __target_execute_cmd().It also clears these bits in case of a target_handle_task_attr()
failure.Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger -
This patch removes legacy se_cmd->t_fe_count usage in order to avoid
se_cmd->t_state_lock access within transport_put_cmd() during normal
fast path se_cmd descriptor release.Also drop the left-over parameter usage within core_tmr_handle_tas_abort()
Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger -
This patch drops an unnecessary acquire/release of se_cmd->t_state_lock within
transport_lun_remove_cmd() when checking CMD_T_DEV_ACTIVE for invoking
target_remove_from_state_list().For all fast path completion cases, transport_lun_remove_cmd() is always
called ahead of transport_cmd_check_stop(), and since transport_cmd_check_stop()
is calling target_remove_from_state_list() when remove_from_lists=true,
the t_state_lock usage in transport_lun_remove_cmd() can safely be removed.Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger -
This patch adds a new transport_cmd_check_stop() parameter for signaling
when TRANSPORT_WRITE_PENDING needs to be set.This allows transport_generic_new_cmd() to avoid the extra lock acquire/release
of ->t_state_lock in the fast path for DMA_TO_DEVICE operations ahead of
transport_cmd_check_stop() + se_tfo->write_pending().Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Kent Overstreet
Cc: Or Gerlitz
Cc: Moussa Ba
Signed-off-by: Nicholas Bellinger
14 Jun, 2013
7 commits
-
Here is a fun one. Bug seems to have been introduced by commit 140854cb,
almost two years ago. I have no idea why we only started seeing it now,
but we did.Rough callgraph:
core_tpg_set_initiator_node_queue_depth()
`-> spin_lock_irqsave(&tpg->session_lock, flags);
`-> lio_tpg_shutdown_session()
`-> iscsit_stop_time2retain_timer()
`-> spin_unlock_bh(&se_tpg->session_lock);
`-> spin_lock_bh(&se_tpg->session_lock);
`-> spin_unlock_irqrestore(&tpg->session_lock, flags);core_tpg_set_initiator_node_queue_depth() used to call spin_lock_bh(),
but 140854cb changed that to spin_lock_irqsave(). However,
lio_tpg_shutdown_session() still claims to be called with spin_lock_bh()
held, as does iscsit_stop_time2retain_timer():
* Called with spin_lock_bh(&struct se_portal_group->session_lock) heldStale documentation is mostly annoying, but in this case the dropping
the lock with the _bh variant is plain wrong. It is also wrong to drop
locks two functions below the lock-holder, but I will ignore that bit
for now.After some more locking and unlocking we eventually hit this backtrace:
------------[ cut here ]------------
WARNING: at kernel/softirq.c:159 local_bh_enable_ip+0xe8/0x100()
Pid: 24645, comm: lio_helper.py Tainted: G O 3.6.11+
Call Trace:
[] warn_slowpath_common+0x7f/0xc0
[] ? iscsit_inc_conn_usage_count+0x37/0x50 [iscsi_target_mod]
[] warn_slowpath_null+0x1a/0x20
[] local_bh_enable_ip+0xe8/0x100
[] _raw_spin_unlock_bh+0x15/0x20
[] iscsit_inc_conn_usage_count+0x37/0x50 [iscsi_target_mod]
[] iscsit_stop_session+0xfa/0x1c0 [iscsi_target_mod]
[] lio_tpg_shutdown_session+0x7b/0x90 [iscsi_target_mod]
[] core_tpg_set_initiator_node_queue_depth+0xe4/0x290 [target_core_mod]
[] iscsit_tpg_set_initiator_node_queue_depth+0x12/0x20 [iscsi_target_mod]
[] lio_target_nacl_store_cmdsn_depth+0xa9/0x180 [iscsi_target_mod]
[] target_fabric_nacl_base_attr_store+0x39/0x40 [target_core_mod]
[] configfs_write_file+0xbd/0x120
[] vfs_write+0xc6/0x180
[] sys_write+0x51/0x90
[] system_call_fastpath+0x16/0x1b
---[ end trace 3747632b9b164652 ]---As a pure band-aid, this patch drops the _bh.
Signed-off-by: Joern Engel
Cc: stable
Signed-off-by: Nicholas Bellinger -
Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
Nobody should be expecting to read or write virtual_lun0.
Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
Use bool params when appropriate.
Eliminate unneeded pr_reg_e and type variables.
Just one goto label, so rename to 'out' from 'out_put_pr_reg'.
Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
Don't need goto, we can just do an if/else for sa_res_key behavior. Move
shorter case first. Slightly shorter b/c both cases can share a call to
update_and_write_aptpl() now.Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
Instead of taking the buffer and length, update_and_write_aptpl() will
allocate the buffer as needed, and then free it. Instead, the function
takes an 'aptpl' boolean parameter.This enables us to remove memory alloc/frees from struct
t10_pr_registration and other spots.There is a slight loss of functionality because each callsite doesn't get
its own pr_debug any more, but this info can be cleaned via ftrace if
necessary and I think the shorter code is worth it.Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger -
aptpl_file_mutex seems to no longer exist.
Signed-off-by: Andy Grover
Signed-off-by: Nicholas Bellinger