24 Oct, 2011
40 commits
-
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 -
The most commonly used file, iblock and rd backends have no use for
a per-task CDB and thus don't need a method to copy it into their
otherwise unused CDB fields.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Rearrange the fields in se_task to avoid holes. Also increase the
flags field to 16 bits as we have the space for it, and this makes
adding new flags safer.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This is a squashed version of the following unnecessary se_task structure
member removal patches:target: remove the task_execute_queue field in se_task
Instead of using a separate flag we can simply do list_emptry checks
on t_execute_list if we make sure to always use list_del_init to remove
a task from the list. Also factor some duplicate code into a new
__transport_remove_task_from_execute_queue helper.target: remove the read-only task_no field in se_task
The task_no field never was initialized and only used in debug printks,
so kill it.target: remove the task_padded_sg field in se_task
This field is only check in one place and not actually needed there.
Rationale:
- transport_do_task_sg_chain asserts that we have task_sg_chaining
set early on
- we only make use of the sg_prev_nents field we calculate based on it
if there is another sg list that gets chained onto this one, which
never happens for the last (or only) task.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Replace various atomic_t variables that were mostly under t_state_lock
with new flags in task_flags. Note that the execution error path
didn't take t_state_lock before, so add it there.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This is a squashed version of the following se_task cleanup patches:
target: remove the unused task_state_flags field in se_task
target: remove the unused se_obj_ptr field in se_task
target: remove the se_dev field in se_taskSigned-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This is a squashed version of the following target_core_base.h
cleanup patches:target: remove the unused SHUTDOWN_SIGS defintion
target: remove unused se_mem leftovers
target: remove the unused map_func_t typedef
target: move TRANSPORT_IOV_DATA_BUFFER to the iscsi-specific codeSigned-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch fixes a bug with tcm_loop where performing a scsi_host rescan was
causing an oops due to a received scsi_cmnd->device->id value not matching a
previously configured tcm_loop_tpg entry in tcm_loop_hba->tl_hba_tpgs[]
obtained from within tcm_loop_queuecommand() code.This fix adds an explict check for tcm_loop_tpg->tl_hba in order to ensure
tcm_loop_make_naa_tpg() has already been invoked to initialize a given
tcm_loop_tpg entry, and also adds an explict clear of tcm_loop_tpg->tl_hba
from within the tcm_loop_drop_naa_tpg() release path.This bug was manifesting itself with the following OOPs:
[176289.430909] BUG: unable to handle kernel NULL pointer dereference at 0000000000000090
[176289.431337] IP: [] transport_processing_thread+0x1e3/0x794 [target_core_mod]
[176289.431399] PGD 22e9b067 PUD 23375067 PMD 0
[176289.431399] Oops: 0000 [#1] SMP
[176289.431815] CPU 1
[176289.431815] Modules linked in: tcm_loop target_core_stgt target_core_pscsi target_core_file target_core_iblock target_core_mod crc32c ib_cm ib_sa ib_mad ib_core qla2xxx scsi_tgt configfs fcoe libfcoe libfc scsi_transport_fc ipv6 iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi sr_mod cdrom sd_mod ata_piix libata e1000 mptspi mptscsih mptbase [last unloaded: target_core_mod]
[176289.431815]
[176289.431815] Pid: 12339, comm: LIO_iblock Tainted: G W 3.1.0-rc8+
[176289.431815] RIP: 0010:[] [] transport_processing_thread+0x1e3/0x794 [target_core_mod]
[176289.431815] RSP: 0018:ffff880023bfbe10 EFLAGS: 00010283
[176289.431815] RAX: 0000000000000000 RBX: ffff88002d600040 RCX: ffff88002d600108
[176289.431815] RDX: ffff88000c9e50bc RSI: 0000000000000246 RDI: 0000000000000246
[176289.431815] RBP: ffff880023bfbee0 R08: ffff88002d600108 R09: 0000000000000000
[176289.431815] R10: ffff88002fc8cc80 R11: ffffffff81671b60 R12: ffff88002d600108
[176289.431815] R13: ffff88000c9e4f38 R14: ffff88000c9e50b8 R15: 0000000000000000
[176289.431815] FS: 0000000000000000(0000) GS:ffff88002fc80000(0000) knlGS:0000000000000000
[176289.431815] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[176289.431815] CR2: 0000000000000090 CR3: 000000002a33f000 CR4: 00000000000006e0
[176289.431815] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[176289.431815] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[176289.431815] Process LIO_iblock (pid: 12339, threadinfo ffff880023bfa000, task ffff88002a2e0000)
[176289.431815] Stack:
[176289.431815] 0000000000011280 0000000000000246 ffff88002a2e0000 ffff880023a58900
[176289.431815] ffff880023bfbed0 ffff880023bfa000 ffff880023bfa000 ffff88000c9e50d0
[176289.431815] ffff88000c9e50c0 ffff88000c9e50bc ffff880023bfa000 ffff880023bfbfd8
[176289.431815] Call Trace:
[176289.431815] [] ? wake_up_bit+0x25/0x25
[176289.431815] [] ? transport_handle_cdb_direct+0x92/0x92 [target_core_mod]
[176289.431815] [] kthread+0x7d/0x85
[176289.431815] [] kernel_thread_helper+0x4/0x10
[176289.431815] [] ? kthread_worker_fn+0x16d/0x16d
[176289.431815] [] ? gs_change+0x13/0x13
[176289.431815] Code: 67 05 00 00 41 8b 84 24 4c ff ff ff ff c8 83 f8 11 0f 87 f0 04 00 00 89 c0 ff 24 c5 b0 c6 39 a0 0f 0b eb fe 48 8b 83 d8 00 00 00
[176289.431815] RIP [] transport_processing_thread+0x1e3/0x794 [target_core_mod]
[176289.431815] RSP
[176289.431815] CR2: 0000000000000090
[176295.041004] ---[ end trace 85dc6865b23b8f3e ]---Reported-by: Paolo Bonzini
Cc: Paolo Bonzini
Signed-off-by: Nicholas Bellinger -
This patch removes the legacy device active I/O shutdown code that was
originally called from transport_processing_thread() context during shutdown
including transport_processing_shutdown() and transport_release_all_cmds().This is due to the fact that in modern configfs control plane code by the
time shutdown of an se_device instance in transport_processing_thread()
is allowed to occur via:rmdir /sys/kernel/config/target/core/$HBA/$DEV
all active I/O will already have been ceased while removing active configfs
fabric Port/LUN symlinks. Eg: the removal of an active se_device is protected
by inter-module VFS references from active Port/LUN symlinks.Two WARN_ON() checks have been added in their place before exiting
transport_processing_thread() to watch out for any leaked descriptors.Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch merges transport_cmd_finish_abort_tmr() logic into a single
transport_cmd_finish_abort() function by adding a cmd->se_tmr_req check
around transport_lun_remove_cmd(), and updates the single caller within
core_tmr_drain_tmr_list().Reported-by: Christoph Hellwig
Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch removes a number of SCF_SE_LUN_CMD flag abuses within iscsi-target
code to determine when iscsit_release_cmd() or transport_generic_free_cmd()
should be called while releasing an individual iscsi_cmd descriptor.In the place of SCF_SE_LUN_CMD checks, this patch converts existing code to
use a new iscsit_free_cmd() that inspects iscsi_cmd->iscsi_opcode types to
determine which of the above functions should be invoked. It also removes the
now unnecessary special case checking in iscsit_release_commands_from_conn().(hch: Use iscsit_free_cmd instead of open-coded alternative)
Reported-by: Christoph Hellwig
Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch converts se_cmd->transport_wait_for_tasks(se_cmd, 1) usage to use
transport_generic_free_cmd() directly in target-core and iscsi-target fabric
usage. The includes:*) Removal of the optional transport_generic_free_cmd() call from within
transport_generic_wait_for_tasks()
*) Usage of existing SCF_SUPPORTED_SAM_OPCODE to determine when
transport_generic_wait_for_tasks() processing may occur instead of
checking se_cmd->transport_wait_for_tasks()
*) Move transport_generic_wait_for_tasks() call ahead of core_dec_lacl_count()
and transport_lun_remove_cmd() in transport_generic_free_cmd() to follow
existing logic for iscsi-target w/ se_cmd->transport_wait_for_tasks(se_cmd, 1)
*) Removal of se_cmd->transport_wait_for_tasks() function pointer
*) Rename transport_generic_wait_for_tasks() -> transport_wait_for_tasks(), and
add docbook comment.
*) Add EXPORT_SYMBOL for transport_wait_for_tasks()For the case in iscsi_target_erl2.c:iscsit_prepare_cmds_for_realligance()
where se_cmd->transport_wait_for_tasks(se_cmd, 0) is called, this patch
adds a direct call to transport_wait_for_tasks().(hch: Fix transport_generic_free_cmd() usage in iscsit_release_commands_from_conn)
(nab: Add patch: Ensure that TMRs hit wait_for_tasks logic during release)Reported-by: Christoph Hellwig
Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Testing in_interrupt() to know when sleeping is allowed is not really
reliable (since eg it won't be true if the caller is holding a spinlock).
Instead have the caller tell core_tmr_alloc_req() what GFP_xxx to use;
every caller except tcm_qla2xxx can use GFP_KERNEL.Signed-off-by: Roland Dreier
Signed-off-by: Nicholas Bellinger -
This patch changes pscsi_create_virtdevice() to properly return ERR_CAST
instead of a raw pointer upon scsi_host_lookup() failure.Signed-off-by: Dan Carpenter
Signed-off-by: Nicholas Bellinger -
This patch converts ft_parse_wwn() to use hex_to_bin() instead of custom
conversion code.(Andy: Re-add missing strict && isupper(c) check)
Signed-off-by: Andy Shevchenko
Cc: "Nicholas A. Bellinger"
Signed-off-by: Nicholas Bellinger -
This patch converts chap_string_to_hex() to use hex2bin() instead of
the internal chap_asciihex_to_binaryhex().(nab: Fix up minor compile breakage + typo)
Signed-off-by: Andy Shevchenko
Cc: "Nicholas A. Bellinger"
Signed-off-by: Nicholas Bellinger -
The cdb_none, map_data_SG and map_control_SG methods have no callers left
and can be removed now.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Move the entirely request allocation, mapping and submission into ->do_task.
Thisa) avoids blocking the I/O submission thread unessecarily, and
b) simplifies the code greatlyNote that the code seems to have various error handling issues, mostly
related to bidi handling in the current form. I've added comments about
those but not tried to fix them in this commit.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Move the entirely bio allocation, mapping and submission into ->do_task.
Thisa) avoids blocking the I/O submission thread unessecarily, and
b) simplifies the code greatlySigned-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
This patch adds a minor simplfication in target_parse_naa_6h_vendor_specific()
to remove direct isxdigit() + ctype.h usage.(nab: Fix next assignment breakage in for loop)
Signed-off-by: Andy Shevchenko
Cc: Nicholas Bellinger
Signed-off-by: Nicholas Bellinger -
This patch removes the unnecessary session_reinstatement parameter from
se_cmd->transport_wait_for_tasks(), logic in transport_generic_wait_for_tasks,
and usage within iscsi-target code.This also includes the removal of the 'bool' return from transport_put_cmd() +
transport_generic_free_cmd() that is no longer necessary.Cc: Christoph Hellwig
Signed-off-by: Nicholas Bellinger -
Push session reinstatement out of transport_generic_free_cmd into the only
caller that actually needs it. Clean up transport_generic_free_cmd a bit,
and remove the useless comment. I'd love to add a more useful kerneldoc
comment for it, but as this point I'm still a bit confused in where it
stands in the command release stack.Signed-off-by: Christoph Hellwig
Signed-off-by: Nicholas Bellinger
