Re: [PATCH] RDMA/srpt: Fix TPG creation

[Bart Van Assche <bvanassche@acm.org>]

On 2019-10-23 20:37, Honggang LI wrote:
> On Wed, Oct 23, 2019 at 01:41:06PM -0700, Bart Van Assche wrote:
>> +	mutex_lock(&sport->port_gid_id.mutex);
>> +	list_for_each_entry(stpg, &sport->port_gid_id.tpg_list, entry) {
>> +		if (!IS_ERR_OR_NULL(ch->sess))
>                 ^^^^^^^
>> +			break;
>> +		ch->sess = target_setup_session(&stpg->tpg, tag_num,
>                            ^^^^^^^^^^^^^^^^^^^^
>>  					tag_size, TARGET_PROT_NORMAL, i_port_id,
>>  					ch, NULL);
>> -	/* Retry without leading "0x" */
>> -	if (sport->port_gid_id.tpg.se_tpg_wwn && IS_ERR_OR_NULL(ch->sess))
>> -		ch->sess = target_setup_session(&sport->port_gid_id.tpg, tag_num,
>> +		if (!IS_ERR_OR_NULL(ch->sess))
>                     ^
>> +			break;
> 
> I'm confused about this 'if' statement. In case you repeated the
> validation as previous 'if' statement, it is redundance.
> 
> In case you check the return of the first target_setup_session,
> it seems wrong, we only need to retry in case first target_setup_session
> was failed. But you break out, and skip the second target_setup_session.
> 
>> +		/* Retry without leading "0x" */
>> +		ch->sess = target_setup_session(&stpg->tpg, tag_num,
>                            ^^^^^^^^^^^^^^^^^^^^
>>  						tag_size, TARGET_PROT_NORMAL,
>>  						i_port_id + 2, ch, NULL);

Hi Honggang,

The purpose of this code is to keep iterating until a session has been
created. The "if (!IS_ERR_OR_NULL(ch->sess)) break" code prevents
further target_setup_session() calls after a session has been created
successfully.

Bart.