Re: [PATCH 1/2] target: Add transport_handle_cdb_direct optimization

[Andy Grover <agrover@redhat.com>]

On 06/04/2011 07:03 AM, Christoph Hellwig wrote:
>> + */
>> +int transport_handle_cdb_direct(
>> +	struct se_cmd *cmd)
>> +{
>> +	if (!cmd->se_lun) {
>> +		dump_stack();
>> +		printk(KERN_ERR "cmd->se_lun is NULL\n");
>> +		return -EINVAL;
>> +	}
>> +	if (in_interrupt()) {
>> +		dump_stack();
>> +		printk(KERN_ERR "transport_generic_handle_cdb cannot be called"
>> +				" from interrupt context\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	return transport_generic_new_cmd(cmd);
> 
> I can't really see any reason to add this helper.  It just adds rather
> pointless debug checks for cases that already will blow up "properly" with
> the current code.  Let's keep the callchain lean and just leave it out.

I agree. Also, I don't think there should be a handle_cdb_direct because
I think handle_cdb should call transport_generic_new_cmd, we don't need
a "direct" version. transport_generic_new_cmd should be safe (or made
safe) for calling from interrupt context. There's nothing in it that
demands it be executed in the backstore thread's context, and doing so
just incurs a two context switch latency penalty.

Regards -- Andy