On Fri, 2011-11-04 at 17:55 +0100, Fubo Chen wrote: > On Thu, Nov 3, 2011 at 8:52 PM, Nicholas A. Bellinger > <nab@xxxxxxxxxxxxxxx> wrote: > > 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. > > Why modify each driver ? Why not invoke target_get_sess_cmd() from > transport_handle_cdb, target_put_sess_cmd() from > transport_generic_free and target_wait_for_sess_cmds() from > transport_deregister_session ? > 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. 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. 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 do think that target_wait_for_sess_cmds() can be moved into transport_deregister_session_configfs(), for which the fabric module callers will need to ensure sleeping is allowed. I think tcm_qla2xxx is the only one that does not assume this atm, but that is easy enough to address. Thanks, --nab -- 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
