public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Andy Grover <agrover@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: target-devel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [PATCH 06/20] target: Eliminate usage of struct se_mem
Date: Fri, 08 Jul 2011 17:38:17 -0700	[thread overview]
Message-ID: <4E17A2F9.8090309@redhat.com> (raw)
In-Reply-To: <20110708201331.GD12288@infradead.org>

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

  reply	other threads:[~2011-07-09  0:38 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-07 22:33 [0/20] Patches from June 28 v2 Andy Grover
2011-07-07 22:33 ` [PATCH 01/20] target: Remove ifdeffed code in t_g_process_write Andy Grover
2011-07-07 22:33 ` [PATCH 02/20] target: Pass 2nd param of transport_split_cdb by value Andy Grover
2011-07-07 22:33 ` [PATCH 03/20] target: Make all control CDBs scatter-gather Andy Grover
2011-07-08 19:57   ` Christoph Hellwig
2011-07-09  0:02     ` Andy Grover
2011-07-09  0:35     ` Andy Grover
2011-07-07 22:33 ` [PATCH 04/20] target: Enforce 1 page max for control cdb buffer sizes Andy Grover
2011-07-08 19:58   ` Christoph Hellwig
2011-07-07 22:33 ` [PATCH 05/20] target: Remove direct ramdisk code Andy Grover
2011-07-08 19:59   ` Christoph Hellwig
2011-07-07 22:33 ` [PATCH 06/20] target: Eliminate usage of struct se_mem Andy Grover
2011-07-08 20:13   ` Christoph Hellwig
2011-07-09  0:38     ` Andy Grover [this message]
2011-07-07 22:33 ` [PATCH 07/20] target: Rename task_sg_num to task_sg_nents Andy Grover
2011-07-07 22:33 ` [PATCH 08/20] target: Remove custom debug macros for pr_debug. Use pr_err() Andy Grover
2011-07-07 22:33 ` [PATCH 09/20] target: Remove custom debug macros in non-iscsi fabrics Andy Grover
2011-07-07 22:33 ` [PATCH 10/20] target/iscsi: Remove iscsi_target_debug.h and usage of TRACE() & printk() Andy Grover
2011-07-07 22:33 ` [PATCH 11/20] target/iscsi: Remove SE_CMD macro Andy Grover
2011-07-07 22:33 ` [PATCH 12/20] target: Set WSNZ=1 in block limits VPD. Abort if WRITE_SAME sectors = 0 Andy Grover
2011-07-07 22:33 ` [PATCH 13/20] target: Remove transport do_se_mem_map callback Andy Grover
2011-07-07 22:33 ` [PATCH 14/20] target: When using kunmap_first_data_page, use goto-style err handling Andy Grover
2011-07-07 22:33 ` [PATCH 15/20] target: Further simplify transport_free_pages Andy Grover
2011-07-07 22:33 ` [PATCH 16/20] target: Redo task allocation return value handling Andy Grover
2011-07-07 22:33 ` [PATCH 17/20] target/core: Remove extra parentheses Andy Grover
2011-07-07 22:33 ` [PATCH 18/20] target: Extraneous paren removal in tcm_vhost and tcm_qla2xxx Andy Grover
2011-07-07 22:33 ` [PATCH 19/20] target/iscsi: Remove unneeded parens in conditional statements Andy Grover
2011-07-07 22:33 ` [PATCH 20/20] target/iscsi: Simplify two small bits Andy Grover
2011-07-09  1:18 ` [0/20] target: Patches for June 28 v3 Andy Grover
2011-07-17 19:32   ` Nicholas A. Bellinger
2011-07-09  1:18 ` [PATCH 01/20] target: Remove ifdeffed code in t_g_process_write Andy Grover
2011-07-09  1:18 ` [PATCH 02/20] target: Pass 2nd param of transport_split_cdb by value Andy Grover
2011-07-09  1:18 ` [PATCH 03/20] target: Make all control CDBs scatter-gather Andy Grover
2011-07-17 19:40   ` Nicholas A. Bellinger
2011-07-09  1:18 ` [PATCH 04/20] target: Enforce 1 page max for control cdb buffer sizes Andy Grover
2011-07-09  1:18 ` [PATCH 05/20] target: Remove direct ramdisk code Andy Grover
2011-07-09  1:18 ` [PATCH 06/20] target: Eliminate usage of struct se_mem Andy Grover
2011-07-17 19:57   ` Nicholas A. Bellinger
2011-07-18 17:16     ` Andy Grover
2011-07-18 18:44       ` Nicholas A. Bellinger
2011-07-19  4:56     ` Christoph Hellwig
2011-07-19  5:40       ` Nicholas A. Bellinger
2011-07-09  1:18 ` [PATCH 07/20] target: Rename task_sg_num to task_sg_nents Andy Grover
2011-07-09  1:18 ` [PATCH 08/20] target: Remove custom debug macros for pr_debug. Use pr_err() Andy Grover
2011-07-09  1:18 ` [PATCH 09/20] target: Remove custom debug macros in non-iscsi fabrics Andy Grover
2011-07-17 20:02   ` Nicholas A. Bellinger
2011-07-09  1:18 ` [PATCH 10/20] target/iscsi: Remove iscsi_target_debug.h and usage of TRACE() & printk() Andy Grover
2011-07-09  1:18 ` [PATCH 11/20] target/iscsi: Remove SE_CMD macro Andy Grover
2011-07-09  1:18 ` [PATCH 12/20] target: Set WSNZ=1 in block limits VPD. Abort if WRITE_SAME sectors = 0 Andy Grover
2011-07-09  1:18 ` [PATCH 13/20] target: Remove transport do_se_mem_map callback Andy Grover
2011-07-09  1:18 ` [PATCH 14/20] target: Further simplify transport_free_pages Andy Grover
2011-07-09  1:18 ` [PATCH 15/20] target: Redo task allocation return value handling Andy Grover
2011-07-09  1:18 ` [PATCH 16/20] target/core: Remove extra parentheses Andy Grover
2011-07-09  1:18 ` [PATCH 17/20] target: Extraneous paren removal in tcm_vhost and tcm_qla2xxx Andy Grover
2011-07-09  1:18 ` [PATCH 18/20] target/iscsi: Remove unneeded parens in conditional statements Andy Grover
2011-07-09  1:18 ` [PATCH 19/20] target/iscsi: Simplify two small bits Andy Grover
2011-07-09  1:18 ` [PATCH 20/20] target: change alloc_task call to take *cdb, not *cmd Andy Grover
2011-07-09 10:11   ` Christoph Hellwig
2011-07-10 22:41     ` Andy Grover
2011-07-11 15:28       ` Christoph Hellwig
2011-07-17 20:04   ` Nicholas A. Bellinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E17A2F9.8090309@redhat.com \
    --to=agrover@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox