On Sat, Jun 08, 2013 at 12:00:36AM -0400, Jörn Engel wrote: > > -static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists) > > +static int transport_cmd_check_stop(struct se_cmd *cmd, bool remove_from_lists, > > + bool write_pending) > ... > > > - return transport_cmd_check_stop(cmd, true); > > + return transport_cmd_check_stop(cmd, true, false); > > At this point I would prefer to pass in a flags. > transport_cmd_check_stop(cmd, SC_REMOVE_FROM_LISTS) seems more > readable to me. The whole area needs a major cleanup. The list removal is mostly unrelated to the rest of the function, so it really should be split out. Tje all to target_remove_from_state_list actually happens unconditionaly, just that in the CMD_T_LUN_STOP case it is done after clearing CMD_T_ACTIVE. I can't see a reason for that delay, so unless proven otherwise the call to it should be lifted from transport_cmd_check_stop to transport_cmd_check_stop_to_fabric. The same probably applies to the clearing of cmd->se_lun, and the call to ->check_stop_free can already be done in the caller just based on the return value from transport_cmd_check_stop. Then we can replace the irqsave locking with just _irq locking because static int transport_cmd_check_stop should never be called with irqs disabled and finally add a variant that has the lock already taken instead of adding the new flag. Longer term down the road we should get rid of the _irq locking in the target core entirely. As we moved all major work to workqueues a while ago nothing should be nessecary although a small audit is needed first. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
