On Wed, 2011-11-09 at 03:30 -0500, Christoph Hellwig wrote: > On Wed, Nov 09, 2011 at 01:09:03AM +0000, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx> > > > > This patch pushes the main process context I/O execution into qla_target.c > > qla_tgt_do_work() and splits up interrupt context se_cmd initialization > > into tgt_ops->init_cmd() from I/O dispatch in tgt_ops->handle_cmd(). > > The latter will run in process context in qla_tgt_do_work() for > > qla_tgt->qla_tgt_wq without reacquire of qla_hw_data->hardware_lock > > Why do you care about initializing the se_cmd in interrupt context? It currently does the minimal amount of setup necessary in interrupt context while hardware_lock is held to fit into existing code. I'm slowly removing the remaining setup in interrupt context, and will drop ->init_cmd() after testing some more.. > > This has various major downsides: > > - we keep calling target core code from interrupt context, which means > we need to keep all the interrupt safe locking there for the long run It's single session context lock, and everything including and below transport_lookup_cmd_lun() is done in workqueue context. > - we still have the double workqueue offload when needing to allocate > a session from user context I know. The next step is to take a look at the mess of qla_tgt_sess locking + session creation and see how split that out to use a single offload path. > - you keep the command submission sequence for the drivers messy - I > really want to see the driver having to do a single call into the > target core one day, and not the messy sequence of: > > - transport_init_se_cmd > - target_get_sess_cmd > - (set bidi flag in the cmd) > - transport_lookup_cmd_lun > - transport_generic_allocate_tasks > - transport_handle_cdb_direct > > All with error handling, sending sense and other little details that > driver writers are guaranteed to get wrong inbetween. > > I really want a: > > int target_submit_cmd(struct se_session *sess, struct se_cmd *cmd, > struct scsi_lun *lun, u8 *cdb, u8 *sense, int task_attr, > u8 *sense, int data_length, enum dma_data_direction data_dir, > unsigned int flags /* for bidi */); > > and not have to have the driver worry about any of the nitty-gritty > details (and yes, we'd need a variant for drivers that already pass > in a SGL list, this is just a high-level concept) > Yes, and yes. We're getting to the point where all of this can be invoked from a single caller, > And btw, it's fairly annoying that you keep pushing these patches to the > git tree ASAP while they still are under heavy discussion. > Sorry, this should be going into a different branch. --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
