04 Nov, 2011
1 commit
-
This patch adds the initial pieces of generic active I/O shutdown logic.
This is intended to be a 'opt-in' feature for fabric modules that
includes the following functions to provide a mechinism for fabric
modules to track se_cmd via se_session->sess_cmd_list:*) target_get_sess_cmd() - Add se_cmd to sess->sess_cmd_list, called
from fabric module incoming I/O path.
*) target_put_sess_cmd() - Check for completion or drop se_cmd from
->sess_cmd_list
*) target_splice_sess_cmd_list() - Splice active I/O list from
->sess_cmd_list to ->sess_wait_list, can called with HW fabric
lock held.
*) target_wait_for_sess_cmds() - Walk ->sess_wait_list waiting on
individual ->cmd_wait_comp. Optional transport_wait_for_tasks()
call.target_splice_sess_cmd_list() is allowed to be called under HW fabric
lock, and performs the splice into se_sess->sess_wait_list and set
se_cmd->cmd_wait_set. Then target_wait_for_sess_cmds() walks the list
waiting for individual target_put_sess_cmd() fabric callbacks to
complete.It also adds TFO->check_release_cmd() to split the completion and memory
release calls, where a fabric module uses target_put_sess_cmd() to check
for I/O completion during session shutdown. This is currently pushed out
into fabric modules as current fabric code may sleep here waiting for
TFO->check_stop_free() to complete in main response path, and because
target_wait_for_sess_cmds() calling TFO->release_cmd() to free fabric
descriptor memory directly.Cc: Christoph Hellwig
Cc: Roland Dreier
Signed-off-by: Nicholas A. Bellinger
03 Nov, 2011
1 commit
-
The commit
target: use a workqueue for I/O completions
accidentally removed setting t_tasks_failed in transport_complete_task.
Add it back in a slightly cleaner way; now it is set for every failed task
instead of special casing the last one completing by using the success
argument directly for it.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas A. Bellinger
02 Nov, 2011
6 commits
-
The check is wrong here because blk_make_request() returns an
ERR_PTR() and it doesn't return NULL.Signed-off-by: Dan Carpenter
Signed-off-by: Nicholas A. Bellinger -
This patch drops TRANSPORT_FREE_CMD_INTR usage from target core, which
includes the removal of transport_generic_free_cmd_intr() symbol,
TRANSPORT_FREE_CMD_INTR usage in transport_processing_thread(), and
special case LUN_RESET handling to skip TRANSPORT_FREE_CMD_INTR processing
in core_tmr_drain_cmd_list(). We now expect that fabric modules will
use an internal workqueue to provide process context when releasing
se_cmd descriptor resources via transport_generic_free_cmd().Reported-by: Christoph Hellwig
Cc: Christoph Hellwig
Cc: Roland Dreier
Cc: Madhuranath Iyengar
Signed-off-by: Nicholas Bellinger -
This patch converts target_core_fabric_ops->check_stop_free() usage in
transport_cmd_check_stop() and associated fabric module usage to
return '1' when the passed se_cmd has been released directly within
->check_stop_free(), or return '0' when the passed se_cmd has not
been released.This addresses an issue where transport_cmd_finish_abort() ->
transport_cmd_check_stop_to_fabric() was leaking descriptors during
LUN_RESET for modules using ->check_stop_free(), but not directly
releasing se_cmd in all cases.Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
This patch addresses two issues with non immediate TMR handling in
iscsit_handle_task_mgt_cmd(). The first involves breakage due to
v3.1-rc conversion of iscsit_sequence_cmd(), which upon good status
would hit the iscsit_add_reject_from_cmd() block of code. This patch
adds an explict check for CMDSN_ERROR_CANNOT_RECOVER.The second adds a check to return when non immediate TMR operation is
detected after iscsit_ack_from_expstatsn(), as iscsit_sequence_cmd()
-> iscsit_execute_cmd() will have called transport_generic_handle_tmr()
for the non immediate TMR case already.Cc: Andy Grover
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
This patch adds a missing CMDSN_LOWER_THAN_EXP return check for
iscsit_sequence_cmd() in iscsit_handle_scsi_cmd() that was incorrectly
dropped during the v3.1-rc cleanups to use iscsit_sequence_cmd().Cc: Andy Grover
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
After the list_del() in core_tmr_drain_tmr_list(),
core_tmr_release_req() would list_del() the same object again.Call graph:
core_tmr_drain_tmr_list
transport_cmd_finish_abort_tmr
transport_generic_remove
transport_free_se_cmd
core_tmr_release_reqSo use list_del_init(), as list_del() of an initialized list_head is
safe and essentially a nop. In the CONFIG_DEBUG_LIST case, list_del()
actually poisons the list_head, but that is fine as we free the object
directly afterwards.Signed-off-by: Joern Engel
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger
27 Oct, 2011
6 commits
-
This patch adds a handful minor cleanups to core_tmr_drain_tmr_list() that
remove an unnecessary NULL check, use list_for_each_entry_safe() instead of
list_entry(), and makes the drain_tmr_list walk use *tmr_p instead of
directly referencing the passed *tmr function parameter.Signed-off-by: Joern Engel
Cc: Joern Engel
Reviewed-by: Roland Dreier
Cc: Roland Dreier
Signed-off-by: Nicholas Bellinger -
This patch fixes another bug from LUN_RESET re-org fallout in
core_tmr_drain_tmr_list() that was adding the wrong se_tmr_req
into the local drain_tmr_list to be walked + released.Signed-off-by: Joern Engel
Cc: Joern Engel
Reviewed-by: Roland Dreier
Cc: Roland Dreier
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
This patch fixes a bug in core_tmr_drain_tmr_list() where drain_tmr_list
was using the wrong se_tmr_req for cmd assignment due to a typo during the
LUN_RESET re-org. This was resulting in general protection faults while
using the leftover bogus *tmr_p pointer from list_for_each_entry_safe().Signed-off-by: Joern Engel
Cc: Joern Engel
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
This patch changes target core to also check for -ENOMEM from fabric callbacks
to signal QUEUE_FULL status, instead of just -EAGAIN in order to catch a
larger set of fabric failure cases that want to trigger QUEUE_FULL logic.
This includes the callbacks for ->write_pending(), ->queue_data_in() and
->queue_status().It also makes transport_generic_write_pending() return zero upon QUEUE_FULL,
and removes two unnecessary -EAGAIN checks to catch write pending QUEUE_FULL
cases from transport_generic_new_cmd() failures in transport_handle_cdb_direct()
and transport_processing_thread():TRANSPORT_NEW_CMD_MAP state.Reported-by: Bart Van Assche
Cc: Bart Van Assche
Cc: Christoph Hellwig
Cc: Roland Dreier
Signed-off-by: Nicholas A. Bellinger -
This patch addresses an issue with buggy userspace code sending I/O
via scsi-generic that does not explictly clear their associated read
buffers. It adds an explict memset of the first SGL entry within
tcm_loop_new_cmd_map() for SCF_SCSI_CONTROL_SG_IO_CDB payloads that
are currently guaranteed to be a single SGL by target-core code.This issue is a side effect of the v3.1-rc1 merge to remove the
extra memcpy between certain control CDB types using a contigious
+ cleared buffer in target-core, and performing a memcpy into the
SGL list within tcm_loop.It was originally mainfesting itself by udev + scsi_id + scsi-generic
not properly setting up the expected /dev/disk/by-id/ symlinks because
the INQUIRY payload was containing extra bogus data preventing the
proper NAA IEEE WWN from being parsed by userspace.Cc: Christoph Hellwig
Cc: Andy Grover
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
This patch fixes the following compile warning in target_core_cdb.c in
recent linux-next code due to the new use of EXPORT_SYMBOL() for
target_get_task_cdb().drivers/target/target_core_cdb.c:1316: warning: data definition has no type or storage class
drivers/target/target_core_cdb.c:1316: warning: type defaults to ‘int’ in declaration of ‘EXPORT_SYMBOL’
drivers/target/target_core_cdb.c:1316: warning: parameter names (without types) in function declarationSigned-off-by: Nicholas Bellinger
24 Oct, 2011
26 commits
-
This patch removes the legacy usage of se_task->task_timer and associated
infrastructure that originally was used as a way to help manage buggy backend
SCSI LLDs that in certain cases would never return back an outstanding task.This includes the removal of target_complete_timeout_work(), timeout logic
from transport_complete_task(), transport_task_timeout_handler(),
transport_start_task_timer(), the per device task_timeout configfs attribute,
and all task_timeout associated structure members and defines in
target_core_base.hThis is being removed in preparation to make transport_complete_task() run
in lock-less mode.Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch converts target-core to use se_cmd->t_transport_sent instead of
a duplicated se_cmd->transport_sent member in a handful of locations.
It also updates iscsi_target to properly use ->t_transport_sent instead of
it's own iscsi_cmd_t->transport_sent value that was not being assigned.Reported-by: Christoph Hellwig
Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
If we only have a single task per command (which at least in my testing
is the by far most common case) we do not have to allocate a new per-task
S/G list but can reuse the one from the command.(nab: Fix BIDI handling in transport_free_dev_tasks)
Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch fixes a bug for BIDI handling in transport_generic_new_cmd() where
cmd->t_task_cdbs_left and Co. where not taking into account the extra
task count generated during the first call to transport_allocate_data_tasks().Cc: Christoph Hellwig
Cc: Boaz Harrosh
Signed-off-by: Nicholas Bellinger -
There were only two callers, and one of them always wants the call
to transport_allocate_data_tasks anyway. Also drop the constant
lba argument to transport_allocate_data_tasks and move the variables
inside it into the minimum required scope.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
These are two fairly small functions, and merging them gives a much
more readable control flow, and opportunities for more useful comments.It also moves all code related to resources allocation closer together
and allows to remove a forward declaration for transport_allocate_tasks.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This field is never used given that BIDI handling happens at the
command and not the task level. Remove it and the dead code in
pscsi that tries to work on it.It also prevents pSCSI passthrough for the two currently enabled BIDI
commands now that task->task_sg_bidi support has been removed.Signed-off-by: Christoph Hellwig
Cc: Boaz Harrosh
Signed-off-by: Nicholas Bellinger -
Remove the now unnecessary extra call to transport_subsystem_check_init() in
target_core_register_fabric(), and also merge transport_subsystem_reqmods()
directly into transport_subsystem_check_init().Reported-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Instead of abusing the target processing thread for offloading I/O
completion in the backends to user context add a new workqueue. This means
completions can be processed as fast as available CPU time allows it,
including in parallel with other completions and more importantly I/O
submission or QUEUE FULL retries. This should give much better performance
especially on loaded systems.As a fallout we can merge all the completed states into a single
one.On the downside this change complicates lun reset handling a bit by
requiring us to cancel a work item only for those states that have it
initialized. The alternative would be to either always initialize the work
item to a dummy handler, or always use the same handler and do a switch on
the state. The long term solution will be a flag that says that the command
has an initialized work item, but that's only going to be useful once we
have more users.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
We never check for this state, and it makes testing for a completed
state much harder given that it overrides the existing state.Also remove the unused deferred_t_state which is related to it.
Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
We never queue an command with this state, and only set it in a completely
bogus place in tcm_fc.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
We only need to decrement dev->depth_left if failing a command from
__transport_execute_tasks. Instead of doing it first thing in
transport_generic_request_failure and requiring a pseudo-flag argument
for it just opencode the decrement in the two callers (which should
be factored into a single one anyway)Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Currently we stop the timers for all tasks in a command fairly late during
I/O completion, which is fairly pointless and requires all kinds of safety
checks.Instead delete pending timers early on in transport_complete_task, thus
ensuring no new timers firest after that. We take t_state_lock a bit later
in that function thus making sure currenly running timers are out of the
criticial section. To be completely sure the timer has finished we also
add another del_timer_sync call when freeing the task.This also allows removing TF_TIMER_RUNNING as it would be equivalent
to TF_ACTIVE now.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
TF_TIMER_STOP is useless as it only helps to mitigate a tiny race during
deleting the timer. But given that we have cleared TF_ACTIVE at this point
we already have another mitigation a few lines down the function.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
list_for_each_entry_safe only protects against deletions from the list,
but not against any concurrent modifications. Given that we drop
t_state_lock inside the loop it is not safe in transport_free_dev_tasks.Instead of use a local dispose_list that we move all tasks that are
to be deleted to. This is safe because we never do list_emptry checks
on t_list to check if a command is on the list anywhere.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Change one remaining user of transport_cmd_check_stop(cmd, 2, 0) to the
transport_cmd_check_stop_to_fabric wrapper.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
We always operated on the same queue, so move finding it into the function,
just like we do for all other helpers operating on it.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Add a new boolean at_head parameter to transport_add_cmd_to_queue and thus
obsolete the SCF_EMULATE_QUEUE_FULL flag.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Remove the need for the transport_qf_callback callback by making
sure we have specific states with specific handlers for the two
queue full cases.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Remove the dpo_emulated, fua_write_emulated, fua_read_emulated and
write_cache_emulated methods, and replace them with a simple bitfields in
se_subsystem_api in those cases where they ever returned one.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Do not block the submitting thread when handling a SYNCHRONIZE CACHE command,
but implement it asynchronously by sending the FLUSH command ourself and
calling transport_complete_sync_cache from the completion handler.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch fixes a bug with the handling of REPORT TARGET PORT GROUPS
containing a smaller allocation length than the payload requires causing
memory writes beyond the end of the buffer. This patch checks for the
minimum 4 byte length for the response payload length, and also checks
upon each loop of T10_ALUA(su_dev)->tg_pt_gps_list to ensure the Target
port group and Target port descriptor list is able to fit into the
remaining allocation length.If the response payload exceeds the allocation length length, then rd_len
is still increments to indicate to the initiator that the payload has
been truncated.Reported-by: Roland Dreier
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger -
Add a switch statement implementing the CDB LBA/len update directly
in target_get_task_cdb and remove the old ->transport_split_cdb
callback and all its implementations.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Instead of calling out to the backends from the core to get a per-task
CDB and then modify it for the LBA/len pair used for this CDB provide
a helper that writes the adjusted CDB into a provided buffer and call
this method from ->do_task in pscsi.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger
