On Fri, Nov 04, 2011 at 02:07:09PM -0700, Nicholas A. Bellinger wrote: > One of the two modules (tcm_qla2xxx) in lio-core.git that have been > converted to use this does not call transport_handle_cdb(), and instead > uses process context provided by TFO->new_cmd_map() after being queued > up from interrupt context. Having this as an external caller atm gives > me a little more flexibility when debugging active I/O shutdown issues > with ib_srpt and tcm_qla2xxx, but will eventually need to be included > into core callers as you have observed. We really need to get rid of new_cmd_map in its current from anyway - it's one of those abuses of the I/O processing threads that is rather in the way of parallelizing it. > Also, I wanted this code to be 'opt-in' as iscsi-target still does a > number of special things wrt to shutdown that are on a per iscsi_conn > basis, for which a conversion currently to per session tracking does not > really make sense. IMHO the maintainance of the list should be done uncdontionally. There's nothing worse but data structure linkages that are different depending on users. Actually making use of them is a different thing. > The release path in target_put_sess_cmd() was broken up into a seperate > caller as tcm_qla2xxx currently may sleep between > transport_generic_free_cmd() and TFO->check_release_cmd() for module > dependent reasons, which is why this became an external function to > start with. Also, there is a special case with ib_srpt where this is > called individually by the srpt_abort_cmd() callback as well. I think this is a major design issue in the qla2xxx driver and really shouldn't creep into the core. I'd really prefer not to add more hacks until qla2xxx is whacked into a sane model - that is make sure target core code is always called form user context by using workqueues inside qla2xxx (we're getting there for the command freeing, but even the current version is a bit half-assed), make sure the whole cmd->locked_rsp crap goes away - which should be a fairly easy fallout from calling qla_tgt_handle_cmd_for_atio from a workqueue without qla2xxx-specific locks held, and doing some major cleanp of qla_tgt_sess_work_fn and it's subfunctions, that is: - use one struct work item per work item, instead of keeping a list that gets run from one work item and thus kill the whole struct qla_tgt_sess_work_param crapola and sess_work_lock mess. - de-multiplex qla_tgt_exec_sess_work into one function per different functionality - audit hardware_lock usage in these, and cut them down to the actual required critical sections -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
