On 07/08/2011 01:13 PM, Christoph Hellwig wrote: >> @@ -1723,10 +1707,6 @@ transport_generic_get_task(struct se_cmd *cmd, >> task->se_dev = dev; >> task->task_data_direction = data_direction; >> >> - spin_lock_irqsave(&cmd->t_state_lock, flags); >> - list_add_tail(&task->t_list, &cmd->t_task_list); >> - spin_unlock_irqrestore(&cmd->t_state_lock, flags); >> - > > How is moving the task <-> cmd linkage related to the rest of the patch? > It probably should be a separate preparatory patch, but at least needs > some good documentation. There's no clean (easy) way for me to extract this from the rest of the patch, so I'm just going to add some text to the changelog. BTW I've cleaned up alloc_task some more in a follow-on patch, will post shortly. >> + if (cmd->t_bidi_data_sg && >> + (dev->transport->transport_type != TRANSPORT_PLUGIN_PHBA_PDEV)) { > > no need for braces around the comparism. True, but they're not unusual, so I left instances like this. >> + ret = transport_allocate_control_task(cmd); >> + if (ret < 0) >> + return ret; >> + else >> + return 1; > > Any reason you don't directly return the expected value from > transport_allocate_control_task? See patch 16, it fixes this. Regards -- Andy -- 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